From 9caebb9b3a8d50e950553b2ae81477a8b761356c Mon Sep 17 00:00:00 2001 From: PJ Date: Mon, 20 Nov 2023 11:51:23 +0100 Subject: [PATCH 1/3] stores: add sortBy and sortDir, allow sorting by health --- api/object.go | 20 ++- bus/bus.go | 21 +++- stores/metadata.go | 267 ++++++++++++++++++++++++++++++++-------- stores/metadata_test.go | 139 +++++++++++++-------- 4 files changed, 337 insertions(+), 110 deletions(-) diff --git a/api/object.go b/api/object.go index 014ec4215..6b7167525 100644 --- a/api/object.go +++ b/api/object.go @@ -17,6 +17,12 @@ import ( const ( ObjectsRenameModeSingle = "single" ObjectsRenameModeMulti = "multi" + + ObjectSortByHealth = "health" + ObjectSortByName = "name" + + ObjectSortDirAsc = "asc" + ObjectSortDirDesc = "desc" ) var ( @@ -27,6 +33,10 @@ var ( // ErrObjectCorrupted is returned if we were unable to retrieve the object // from the database. ErrObjectCorrupted = errors.New("object corrupted") + + // ErrInvalidObjectSortParameters is returned when invalid sort parameters + // were provided + ErrInvalidObjectSortParameters = errors.New("invalid sort parameters") ) type ( @@ -76,10 +86,12 @@ type ( // ObjectsDeleteRequest is the request type for the /bus/objects/list endpoint. ObjectsListRequest struct { - Bucket string `json:"bucket"` - Limit int `json:"limit"` - Prefix string `json:"prefix"` - Marker string `json:"marker"` + Bucket string `json:"bucket"` + Limit int `json:"limit"` + SortBy string `json:"sortBy"` + SortDir string `json:"sortDir"` + Prefix string `json:"prefix"` + Marker string `json:"marker"` } // ObjectsListResponse is the response type for the /bus/objects/list endpoint. diff --git a/bus/bus.go b/bus/bus.go index fccafff5f..a3bb4bbbe 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -137,9 +137,9 @@ type ( UpdateBucketPolicy(ctx context.Context, bucketName string, policy api.BucketPolicy) error CopyObject(ctx context.Context, srcBucket, dstBucket, srcPath, dstPath, mimeType string) (api.ObjectMetadata, error) - ListObjects(ctx context.Context, bucketName, prefix, marker string, limit int) (api.ObjectsListResponse, error) + ListObjects(ctx context.Context, bucketName, prefix, sortBy, sortDir, marker string, limit int) (api.ObjectsListResponse, error) Object(ctx context.Context, bucketName, path string) (api.Object, error) - ObjectEntries(ctx context.Context, bucketName, path, prefix, marker string, offset, limit int) ([]api.ObjectMetadata, bool, error) + ObjectEntries(ctx context.Context, bucketName, path, prefix, sortBy, sortDir, marker string, offset, limit int) ([]api.ObjectMetadata, bool, error) ObjectsBySlabKey(ctx context.Context, bucketName string, slabKey object.EncryptionKey) ([]api.ObjectMetadata, error) ObjectsStats(ctx context.Context) (api.ObjectsStatsResponse, error) RemoveObject(ctx context.Context, bucketName, path string) error @@ -1203,6 +1203,16 @@ func (b *bus) objectEntriesHandlerGET(jc jape.Context, path string) { return } + var sortBy string + if jc.DecodeForm("sortBy", &sortBy) != nil { + return + } + + var sortDir string + if jc.DecodeForm("sortDir", &sortDir) != nil { + return + } + var marker string if jc.DecodeForm("marker", &marker) != nil { return @@ -1218,7 +1228,7 @@ func (b *bus) objectEntriesHandlerGET(jc jape.Context, path string) { } // look for object entries - entries, hasMore, err := b.ms.ObjectEntries(jc.Request.Context(), bucket, path, prefix, marker, offset, limit) + entries, hasMore, err := b.ms.ObjectEntries(jc.Request.Context(), bucket, path, prefix, sortBy, sortDir, marker, offset, limit) if jc.Check("couldn't list object entries", err) != nil { return } @@ -1256,10 +1266,11 @@ func (b *bus) objectsListHandlerPOST(jc jape.Context) { var req api.ObjectsListRequest if jc.Decode(&req) != nil { return - } else if req.Bucket == "" { + } + if req.Bucket == "" { req.Bucket = api.DefaultBucketName } - resp, err := b.ms.ListObjects(jc.Request.Context(), req.Bucket, req.Prefix, req.Marker, req.Limit) + resp, err := b.ms.ListObjects(jc.Request.Context(), req.Bucket, req.Prefix, req.SortBy, req.SortDir, req.Marker, req.Limit) if jc.Check("couldn't list objects", err) != nil { return } diff --git a/stores/metadata.go b/stores/metadata.go index 5a1858bff..1bdbd13c5 100644 --- a/stores/metadata.go +++ b/stores/metadata.go @@ -1078,74 +1078,82 @@ func (s *SQLStore) SearchObjects(ctx context.Context, bucket, substring string, return objects, nil } -func (s *SQLStore) ObjectEntries(ctx context.Context, bucket, path, prefix, marker string, offset, limit int) (metadata []api.ObjectMetadata, hasMore bool, err error) { - // convenience variables - usingMarker := marker != "" - usingOffset := offset > 0 - +func (s *SQLStore) ObjectEntries(ctx context.Context, bucket, path, prefix, sortBy, sortDir, marker string, offset, limit int) (metadata []api.ObjectMetadata, hasMore bool, err error) { // sanity check we are passing a directory if !strings.HasSuffix(path, "/") { panic("path must end in /") } + // convenience variables + usingMarker := marker != "" + usingOffset := offset > 0 + // sanity check we are passing sane paging parameters if usingMarker && usingOffset { return nil, false, errors.New("fetching entries using a marker and an offset is not supported at the same time") } + // sanity check we are passing sane sorting parameters + if err := validateSort(sortBy, sortDir); err != nil { + return nil, false, err + } + + // sanitize sorting parameters + if sortBy == "" { + sortBy = api.ObjectSortByName + } + if sortDir == "" { + sortDir = api.ObjectSortDirAsc + } else { + sortDir = strings.ToLower(sortDir) + } + // ensure marker is '/' prefixed if usingMarker && !strings.HasPrefix(marker, "/") { marker = fmt.Sprintf("/%s", marker) } - // ensure limits are out of play + // ensure limit is out of play if limit <= -1 { limit = math.MaxInt } - // figure out the HAVING CLAUSE and its parameters - havingClause := "1 = 1" - var havingParams []interface{} - if usingMarker { - havingClause = "name > ?" - havingParams = append(havingParams, marker) - - offset = 0 // disable offset - } - // fetch one more to see if there are more entries if limit != math.MaxInt { limit += 1 } - var rows []rawObjectMetadata - query := fmt.Sprintf(` - SELECT - MAX(etag) AS ETag, - MAX(created_at) AS ModTime, - CASE slashindex WHEN 0 THEN %s ELSE %s END AS name, - SUM(size) AS size, - MIN(health) as health, - MAX(mimeType) as MimeType - FROM ( - SELECT MAX(etag) AS etag, MAX(objects.created_at) AS created_at, MAX(size) AS size, MIN(slabs.health) as health, MAX(objects.mime_type) as mimeType, SUBSTR(object_id, ?) AS trimmed , INSTR(SUBSTR(object_id, ?), "/") AS slashindex - FROM objects - INNER JOIN buckets b ON objects.db_bucket_id = b.id AND b.name = ? - LEFT JOIN slices ON objects.id = slices.db_object_id - LEFT JOIN slabs ON slices.db_slab_id = slabs.id - WHERE SUBSTR(object_id, 1, ?) = ? AND ? - GROUP BY object_id - ) AS m - GROUP BY name - HAVING SUBSTR(name, 1, ?) = ? AND name != ? AND %s - ORDER BY name ASC - LIMIT ? - OFFSET ?`, + // ensure offset is out of play + if usingMarker { + offset = 0 + } + + // build objects query & parameters + objectsQuery := fmt.Sprintf(` +SELECT + MAX(etag) AS ETag, + MAX(created_at) AS ModTime, + CASE slashindex WHEN 0 THEN %s ELSE %s END AS name, + SUM(size) AS size, + MIN(health) as health, + MAX(mimeType) as MimeType +FROM ( + SELECT MAX(etag) AS etag, MAX(objects.created_at) AS created_at, MAX(size) AS size, MIN(slabs.health) as health, MAX(objects.mime_type) as mimeType, SUBSTR(object_id, ?) AS trimmed , INSTR(SUBSTR(object_id, ?), "/") AS slashindex + FROM objects + INNER JOIN buckets b ON objects.db_bucket_id = b.id AND b.name = ? + LEFT JOIN slices ON objects.id = slices.db_object_id + LEFT JOIN slabs ON slices.db_slab_id = slabs.id + WHERE SUBSTR(object_id, 1, ?) = ? AND ? + GROUP BY object_id +) AS m +GROUP BY name +HAVING SUBSTR(name, 1, ?) = ? AND name != ? +`, sqlConcat(s.db, "?", "trimmed"), sqlConcat(s.db, "?", "substr(trimmed, 1, slashindex)"), - havingClause) + ) - parameters := append(append([]interface{}{ + objectsQueryParams := []interface{}{ path, // sqlConcat(s.db, "?", "trimmed"), path, // sqlConcat(s.db, "?", "substr(trimmed, 1, slashindex)") @@ -1160,7 +1168,54 @@ func (s *SQLStore) ObjectEntries(ctx context.Context, bucket, path, prefix, mark utf8.RuneCountInString(path + prefix), // HAVING SUBSTR(name, 1, ?) = ? AND name != ? path + prefix, // HAVING SUBSTR(name, 1, ?) = ? AND name != ? path, // HAVING SUBSTR(name, 1, ?) = ? AND name != ? - }, havingParams...), limit, offset) + } + + // build marker expr + markerExpr := "1 = 1" + var markerParams []interface{} + if usingMarker { + switch sortBy { + case api.ObjectSortByHealth: + var markerHealth float64 + if err = s.db. + Raw(fmt.Sprintf(`SELECT health FROM (%s ORDER BY name) as n WHERE name >= ? LIMIT 1`, objectsQuery), append(objectsQueryParams, marker)...). + Scan(&markerHealth). + Error; err != nil { + return + } + + if sortDir == api.ObjectSortDirAsc { + markerExpr = "(health > ? OR (health = ? AND name > ?))" + markerParams = []interface{}{markerHealth, markerHealth, marker} + } else { + markerExpr = "(health = ? AND name > ?) OR health < ?" + markerParams = []interface{}{markerHealth, marker, markerHealth} + } + case api.ObjectSortByName: + if sortDir == api.ObjectSortDirAsc { + markerExpr = "name > ?" + } else { + markerExpr = "name < ?" + } + markerParams = []interface{}{marker} + default: + panic("unhandled sortBy") // developer error + } + } + + // build order clause + orderByClause := fmt.Sprintf("%s %s", sortBy, sortDir) + if sortBy == api.ObjectSortByHealth { + orderByClause += ", name" + } + + var rows []rawObjectMetadata + query := fmt.Sprintf(`SELECT * FROM (%s ORDER BY %s) AS n WHERE %s LIMIT ? OFFSET ?`, + objectsQuery, + orderByClause, + markerExpr, + ) + parameters := append(append(objectsQueryParams, markerParams...), limit, offset) if err = s.db. Raw(query, parameters...). @@ -2354,7 +2409,7 @@ func sqlWhereBucket(objTable string, bucket string) clause.Expr { // TODO: we can use ObjectEntries instead of ListObject if we want to use '/' as // a delimiter for now (see backend.go) but it would be interesting to have // arbitrary 'delim' support in ListObjects. -func (s *SQLStore) ListObjects(ctx context.Context, bucket, prefix, marker string, limit int) (api.ObjectsListResponse, error) { +func (s *SQLStore) ListObjects(ctx context.Context, bucket, prefix, sortBy, sortDir, marker string, limit int) (api.ObjectsListResponse, error) { // fetch one more to see if there are more entries if limit <= -1 { limit = math.MaxInt @@ -2362,17 +2417,23 @@ func (s *SQLStore) ListObjects(ctx context.Context, bucket, prefix, marker strin limit++ } - prefixExpr := exprTRUE - if prefix != "" { - prefixExpr = gorm.Expr("SUBSTR(o.object_id, 1, ?) = ?", utf8.RuneCountInString(prefix), prefix) + // build prefix expr + prefixExpr := buildPrefixExpr(prefix) + + // build order clause + orderBy, err := buildOrderClause(sortBy, sortDir) + if err != nil { + return api.ObjectsListResponse{}, err } - markerExpr := exprTRUE - if marker != "" { - markerExpr = gorm.Expr("o.object_id > ?", marker) + + // build marker expr + markerExpr, markerOrderBy, err := buildMarkerExpr(s.db, bucket, prefix, marker, sortBy, sortDir) + if err != nil { + return api.ObjectsListResponse{}, err } var rows []rawObjectMetadata - err := s.db. + if err := s.db. Select("o.object_id as Name, MAX(o.size) as Size, MIN(sla.health) as Health, MAX(o.mime_type) as mimeType, MAX(o.created_at) as ModTime"). Model(&dbObject{}). Table("objects o"). @@ -2381,10 +2442,10 @@ func (s *SQLStore) ListObjects(ctx context.Context, bucket, prefix, marker strin Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). Where("? AND ? AND ?", sqlWhereBucket("o", bucket), prefixExpr, markerExpr). Group("o.object_id"). - Order("o.object_id"). + Order(orderBy). + Order(markerOrderBy). Limit(int(limit)). - Scan(&rows).Error - if err != nil { + Scan(&rows).Error; err != nil { return api.ObjectsListResponse{}, err } @@ -2496,3 +2557,103 @@ func (ss *SQLStore) processConsensusChangeContracts(cc modules.ConsensusChange) height++ } } + +func buildMarkerExpr(db *gorm.DB, bucket, prefix, marker, sortBy, sortDir string) (markerExpr clause.Expr, orderBy clause.OrderBy, err error) { + // no marker + if marker == "" { + return exprTRUE, clause.OrderBy{}, nil + } + + // for markers to work we need to order by object_id + orderBy = clause.OrderBy{ + Columns: []clause.OrderByColumn{ + { + Column: clause.Column{Name: "object_id"}, + Desc: false, + }, + }, + } + + desc := strings.EqualFold(sortDir, api.ObjectSortDirDesc) + switch sortBy { + case "", api.ObjectSortByName: + if desc { + markerExpr = gorm.Expr("object_id < ?", marker) + } else { + markerExpr = gorm.Expr("object_id > ?", marker) + } + case api.ObjectSortByHealth: + // fetch marker health + var markerHealth float64 + if marker != "" && sortBy == api.ObjectSortByHealth { + if err := db. + Select("MIN(sla.health)"). + Model(&dbObject{}). + Table("objects o"). + Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id AND b.name = ?", bucket). + Joins("LEFT JOIN slices sli ON o.id = sli.`db_object_id`"). + Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). + Where("? AND ? AND ?", sqlWhereBucket("o", bucket), buildPrefixExpr(prefix), gorm.Expr("o.object_id >= ?", marker)). + Group("o.object_id"). + Limit(1). + Scan(&markerHealth). + Error; err != nil { + return exprTRUE, clause.OrderBy{}, err + } + } + + if desc { + markerExpr = gorm.Expr("(Health <= ? AND object_id > ?) OR Health < ?", markerHealth, marker, markerHealth) + } else { + markerExpr = gorm.Expr("Health > ? OR (Health >= ? AND object_id > ?)", markerHealth, markerHealth, marker) + } + default: + err = fmt.Errorf("unhandled sortBy parameter '%s'", sortBy) + } + return +} + +func buildOrderClause(sortBy, sortDir string) (clause.OrderByColumn, error) { + if err := validateSort(sortBy, sortDir); err != nil { + return clause.OrderByColumn{}, err + } + + orderByColumns := map[string]string{ + "": "object_id", + api.ObjectSortByName: "object_id", + api.ObjectSortByHealth: "Health", + } + + return clause.OrderByColumn{ + Column: clause.Column{Name: orderByColumns[sortBy]}, + Desc: strings.EqualFold(sortDir, api.ObjectSortDirDesc), + }, nil +} + +func buildPrefixExpr(prefix string) clause.Expr { + if prefix != "" { + return gorm.Expr("SUBSTR(o.object_id, 1, ?) = ?", utf8.RuneCountInString(prefix), prefix) + } else { + return exprTRUE + } +} + +func validateSort(sortBy, sortDir string) error { + allowed := func(s string, allowed ...string) bool { + for _, a := range allowed { + if strings.EqualFold(s, a) { + return true + } + } + return false + } + + if !allowed(sortDir, "", api.ObjectSortDirAsc, api.ObjectSortDirDesc) { + return fmt.Errorf("invalid dir '%v', allowed values are '%v' and '%v'; %w", sortDir, api.ObjectSortDirAsc, api.ObjectSortDirDesc, api.ErrInvalidObjectSortParameters) + } + + if !allowed(sortBy, "", api.ObjectSortByHealth, api.ObjectSortByName) { + return fmt.Errorf("invalid sort by '%v', allowed values are '%v' and '%v'; %w", sortBy, api.ObjectSortByHealth, api.ObjectSortByName, api.ErrInvalidObjectSortParameters) + } + return nil +} diff --git a/stores/metadata_test.go b/stores/metadata_test.go index b677d3ebf..302d4bb60 100644 --- a/stores/metadata_test.go +++ b/stores/metadata_test.go @@ -1344,7 +1344,7 @@ func TestObjectHealth(t *testing.T) { } // assert health is returned correctly by ObjectEntries - entries, _, err := ss.ObjectEntries(context.Background(), api.DefaultBucketName, "/", "", "", 0, -1) + entries, _, err := ss.ObjectEntries(context.Background(), api.DefaultBucketName, "/", "", "", "", "", 0, -1) if err != nil { t.Fatal(err) } else if len(entries) != 1 { @@ -1458,47 +1458,64 @@ func TestObjectEntries(t *testing.T) { } } + // override health of some slabs + if err := ss.overrideSlabHealth("/foo/baz/quuz", 0.5); err != nil { + t.Fatal(err) + } + if err := ss.overrideSlabHealth("/foo/baz/quux", 0.75); err != nil { + t.Fatal(err) + } + tests := []struct { - path string - prefix string - want []api.ObjectMetadata + path string + prefix string + sortBy string + sortDir string + want []api.ObjectMetadata }{ - {"/", "", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/foo/", Size: 10, Health: 1}, {Name: "/gab/", Size: 5, Health: 1}}}, - {"/foo/", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/", Size: 7, Health: 1}}}, - {"/foo/baz/", "", []api.ObjectMetadata{{Name: "/foo/baz/quux", Size: 3, Health: 1}, {Name: "/foo/baz/quuz", Size: 4, Health: 1}}}, - {"/gab/", "", []api.ObjectMetadata{{Name: "/gab/guub", Size: 5, Health: 1}}}, - {"/fileś/", "", []api.ObjectMetadata{{Name: "/fileś/śpecial", Size: 6, Health: 1}}}, - - {"/", "f", []api.ObjectMetadata{{Name: "/fileś/", Size: 6, Health: 1}, {Name: "/foo/", Size: 10, Health: 1}}}, - {"/", "F", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}}}, - {"/foo/", "fo", []api.ObjectMetadata{}}, - {"/foo/baz/", "quux", []api.ObjectMetadata{{Name: "/foo/baz/quux", Size: 3, Health: 1}}}, - {"/gab/", "/guub", []api.ObjectMetadata{}}, + {"/", "", "", "", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/foo/", Size: 10, Health: .5}, {Name: "/gab/", Size: 5, Health: 1}}}, + {"/foo/", "", "", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/", Size: 7, Health: .5}}}, + {"/foo/baz/", "", "", "", []api.ObjectMetadata{{Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}}}, + {"/gab/", "", "", "", []api.ObjectMetadata{{Name: "/gab/guub", Size: 5, Health: 1}}}, + {"/fileś/", "", "", "", []api.ObjectMetadata{{Name: "/fileś/śpecial", Size: 6, Health: 1}}}, + + {"/", "f", "", "", []api.ObjectMetadata{{Name: "/fileś/", Size: 6, Health: 1}, {Name: "/foo/", Size: 10, Health: .5}}}, + {"/", "F", "", "", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}}}, + {"/foo/", "fo", "", "", []api.ObjectMetadata{}}, + {"/foo/baz/", "quux", "", "", []api.ObjectMetadata{{Name: "/foo/baz/quux", Size: 3, Health: .75}}}, + {"/gab/", "/guub", "", "", []api.ObjectMetadata{}}, + + {"/", "", "name", "ASC", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/foo/", Size: 10, Health: .5}, {Name: "/gab/", Size: 5, Health: 1}}}, + {"/", "", "name", "DESC", []api.ObjectMetadata{{Name: "/gab/", Size: 5, Health: 1}, {Name: "/foo/", Size: 10, Health: .5}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/FOO/", Size: 7, Health: 1}}}, + + {"/", "", "health", "ASC", []api.ObjectMetadata{{Name: "/foo/", Size: 10, Health: .5}, {Name: "/FOO/", Size: 7, Health: 1}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/gab/", Size: 5, Health: 1}}}, + {"/", "", "health", "DESC", []api.ObjectMetadata{{Name: "/FOO/", Size: 7, Health: 1}, {Name: "/fileś/", Size: 6, Health: 1}, {Name: "/gab/", Size: 5, Health: 1}, {Name: "/foo/", Size: 10, Health: .5}}}, } for _, test := range tests { - got, _, err := ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, "", 0, -1) + got, _, err := ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, test.sortBy, test.sortDir, "", 0, -1) if err != nil { t.Fatal(err) } assertMetadata(got) if !(len(got) == 0 && len(test.want) == 0) && !reflect.DeepEqual(got, test.want) { - t.Errorf("\nlist: %v\nprefix: %v\ngot: %v\nwant: %v", test.path, test.prefix, got, test.want) + t.Fatalf("\nlist: %v\nprefix: %v\ngot: %v\nwant: %v", test.path, test.prefix, got, test.want) } + for offset := 0; offset < len(test.want); offset++ { - got, hasMore, err := ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, "", offset, 1) + got, hasMore, err := ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, test.sortBy, test.sortDir, "", offset, 1) if err != nil { t.Fatal(err) } assertMetadata(got) if len(got) != 1 || got[0] != test.want[offset] { - t.Errorf("\nlist: %v\nprefix: %v\ngot: %v\nwant: %v", test.path, test.prefix, got, test.want[offset]) + t.Fatalf("\noffset: %v\nlist: %v\nprefix: %v\ngot: %v\nwant: %v", offset, test.path, test.prefix, got, test.want[offset]) } moreRemaining := len(test.want)-offset-1 > 0 if hasMore != moreRemaining { - t.Errorf("invalid value for hasMore (%t) at offset (%d) test (%+v)", hasMore, offset, test) + t.Fatalf("invalid value for hasMore (%t) at offset (%d) test (%+v)", hasMore, offset, test) } // make sure we stay within bounds @@ -1506,19 +1523,19 @@ func TestObjectEntries(t *testing.T) { continue } - got, hasMore, err = ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, test.want[offset].Name, 0, 1) + got, hasMore, err = ss.ObjectEntries(ctx, api.DefaultBucketName, test.path, test.prefix, test.sortBy, test.sortDir, test.want[offset].Name, 0, 1) if err != nil { t.Fatal(err) } assertMetadata(got) if len(got) != 1 || got[0] != test.want[offset+1] { - t.Errorf("\nlist: %v\nprefix: %v\nmarker: %v\ngot: %v\nwant: %v", test.path, test.prefix, test.want[offset].Name, got, test.want[offset+1]) + t.Fatalf("\noffset: %v\nlist: %v\nprefix: %v\nmarker: %v\ngot: %v\nwant: %v", offset+1, test.path, test.prefix, test.want[offset].Name, got, test.want[offset+1]) } moreRemaining = len(test.want)-offset-2 > 0 if hasMore != moreRemaining { - t.Errorf("invalid value for hasMore (%t) at marker (%s) test (%+v)", hasMore, test.want[offset].Name, test) + t.Fatalf("invalid value for hasMore (%t) at marker (%s) test (%+v)", hasMore, test.want[offset].Name, test) } } } @@ -3280,13 +3297,13 @@ func TestBucketObjects(t *testing.T) { } // List the objects in the buckets. - if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", 0, -1); err != nil { + if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 1 entry", len(entries)) } else if entries[0].Size != 1 { t.Fatal("unexpected size", entries[0].Size) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 1 entry", len(entries)) @@ -3312,13 +3329,13 @@ func TestBucketObjects(t *testing.T) { // Rename object foo/bar in bucket 1 to foo/baz but not in bucket 2. if err := ss.RenameObject(context.Background(), b1, "/foo/bar", "/foo/baz"); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 2 entries", len(entries)) } else if entries[0].Name != "/foo/baz" { t.Fatal("unexpected name", entries[0].Name) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 2 entries", len(entries)) @@ -3329,13 +3346,13 @@ func TestBucketObjects(t *testing.T) { // Rename foo/bar in bucket 2 using the batch rename. if err := ss.RenameObjects(context.Background(), b2, "/foo/bar", "/foo/bam"); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 2 entries", len(entries)) } else if entries[0].Name != "/foo/baz" { t.Fatal("unexpected name", entries[0].Name) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 2 entries", len(entries)) @@ -3348,28 +3365,28 @@ func TestBucketObjects(t *testing.T) { t.Fatal(err) } else if err := ss.RemoveObject(context.Background(), b1, "/foo/baz"); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) > 0 { t.Fatal("expected 0 entries", len(entries)) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/foo/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 1 entry", len(entries)) } // Delete all files in bucket 2. - if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/", "", "", 0, -1); err != nil { + if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 2 { t.Fatal("expected 2 entries", len(entries)) } else if err := ss.RemoveObjects(context.Background(), b2, "/"); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b2, "/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 0 { t.Fatal("expected 0 entries", len(entries)) - } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(context.Background(), b1, "/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 1 entry", len(entries)) @@ -3423,7 +3440,7 @@ func TestCopyObject(t *testing.T) { // Copy it within the same bucket. if om, err := ss.CopyObject(ctx, "src", "src", "/foo", "/bar", ""); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(ctx, "src", "/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(ctx, "src", "/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 2 { t.Fatal("expected 2 entries", len(entries)) @@ -3436,7 +3453,7 @@ func TestCopyObject(t *testing.T) { // Copy it cross buckets. if om, err := ss.CopyObject(ctx, "src", "dst", "/foo", "/bar", ""); err != nil { t.Fatal(err) - } else if entries, _, err := ss.ObjectEntries(ctx, "dst", "/", "", "", 0, -1); err != nil { + } else if entries, _, err := ss.ObjectEntries(ctx, "dst", "/", "", "", "", "", 0, -1); err != nil { t.Fatal(err) } else if len(entries) != 1 { t.Fatal("expected 1 entry", len(entries)) @@ -3559,18 +3576,33 @@ func TestListObjects(t *testing.T) { t.Fatal(err) } } + + // override health of some slabs + if err := ss.overrideSlabHealth("/foo/baz/quuz", 0.5); err != nil { + t.Fatal(err) + } + if err := ss.overrideSlabHealth("/foo/baz/quux", 0.75); err != nil { + t.Fatal(err) + } + tests := []struct { - prefix string - marker string - want []api.ObjectMetadata + prefix string + sortBy string + sortDir string + marker string + want []api.ObjectMetadata }{ - {"/", "", []api.ObjectMetadata{{Name: "/FOO/bar", Size: 6, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: 1}, {Name: "/foo/baz/quuz", Size: 4, Health: 1}, {Name: "/gab/guub", Size: 5, Health: 1}}}, - {"/foo/b", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: 1}, {Name: "/foo/baz/quuz", Size: 4, Health: 1}}}, - {"o/baz/quu", "", []api.ObjectMetadata{}}, - {"/foo", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: 1}, {Name: "/foo/baz/quuz", Size: 4, Health: 1}}}, + {"/", "", "", "", []api.ObjectMetadata{{Name: "/FOO/bar", Size: 6, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}, {Name: "/gab/guub", Size: 5, Health: 1}}}, + {"/", "", "ASC", "", []api.ObjectMetadata{{Name: "/FOO/bar", Size: 6, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}, {Name: "/gab/guub", Size: 5, Health: 1}}}, + {"/", "", "DESC", "", []api.ObjectMetadata{{Name: "/gab/guub", Size: 5, Health: 1}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/FOO/bar", Size: 6, Health: 1}}}, + {"/", "health", "ASC", "", []api.ObjectMetadata{{Name: "/foo/baz/quuz", Size: 4, Health: .5}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/FOO/bar", Size: 6, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/gab/guub", Size: 5, Health: 1}}}, + {"/", "health", "DESC", "", []api.ObjectMetadata{{Name: "/FOO/bar", Size: 6, Health: 1}, {Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/gab/guub", Size: 5, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}}}, + {"/foo/b", "", "", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}}}, + {"o/baz/quu", "", "", "", []api.ObjectMetadata{}}, + {"/foo", "", "", "", []api.ObjectMetadata{{Name: "/foo/bar", Size: 1, Health: 1}, {Name: "/foo/bat", Size: 2, Health: 1}, {Name: "/foo/baz/quux", Size: 3, Health: .75}, {Name: "/foo/baz/quuz", Size: 4, Health: .5}}}, } for _, test := range tests { - res, err := ss.ListObjects(ctx, api.DefaultBucketName, test.prefix, "", -1) + res, err := ss.ListObjects(ctx, api.DefaultBucketName, test.prefix, test.sortBy, test.sortDir, "", -1) if err != nil { t.Fatal(err) } @@ -3580,12 +3612,12 @@ func TestListObjects(t *testing.T) { got := res.Objects if !(len(got) == 0 && len(test.want) == 0) && !reflect.DeepEqual(got, test.want) { - t.Errorf("\nkey: %v\ngot: %v\nwant: %v", test.prefix, got, test.want) + t.Fatalf("\nkey: %v\ngot: %v\nwant: %v", test.prefix, got, test.want) } if len(res.Objects) > 0 { marker := "" for offset := 0; offset < len(test.want); offset++ { - res, err := ss.ListObjects(ctx, api.DefaultBucketName, test.prefix, marker, 1) + res, err := ss.ListObjects(ctx, api.DefaultBucketName, test.prefix, test.sortBy, test.sortDir, marker, 1) if err != nil { t.Fatal(err) } @@ -3595,9 +3627,9 @@ func TestListObjects(t *testing.T) { got := res.Objects if len(got) != 1 { - t.Errorf("expected 1 object, got %v", len(got)) + t.Fatalf("expected 1 object, got %v", len(got)) } else if got[0].Name != test.want[offset].Name { - t.Errorf("expected %v, got %v", test.want[offset].Name, got[0].Name) + t.Fatalf("expected %v, got %v, offset %v, marker %v", test.want[offset].Name, got[0].Name, offset, marker) } marker = res.NextMarker } @@ -3945,3 +3977,14 @@ func TestSlabHealthInvalidation(t *testing.T) { } } } +func (s *SQLStore) overrideSlabHealth(objectID string, health float64) (err error) { + err = s.db.Exec(fmt.Sprintf(` + UPDATE slabs SET health = %v WHERE id IN ( + SELECT sla.id + FROM objects o + INNER JOIN slices sli ON o.id = sli.db_object_id + INNER JOIN slabs sla ON sli.db_slab_id = sla.id + WHERE o.object_id = "%s" + )`, health, objectID)).Error + return +} From dad8f6ecef0644779c9b4bfaccd54c7c5b014564 Mon Sep 17 00:00:00 2001 From: PJ Date: Mon, 20 Nov 2023 15:38:44 +0100 Subject: [PATCH 2/3] stores: fix TestSlabHealthInvalidation NDF --- stores/metadata_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stores/metadata_test.go b/stores/metadata_test.go index 302d4bb60..58a8f664d 100644 --- a/stores/metadata_test.go +++ b/stores/metadata_test.go @@ -3955,7 +3955,7 @@ func TestSlabHealthInvalidation(t *testing.T) { } // refresh health - now := time.Now() + now := time.Now().Round(time.Second) if err := ss.RefreshHealth(context.Background()); err != nil { t.Fatal(err) } From 04ee171979c4b16bef6e42eb108b2c75a8b7d84d Mon Sep 17 00:00:00 2001 From: PJ Date: Wed, 22 Nov 2023 11:36:54 +0100 Subject: [PATCH 3/3] stores: optimise objects query --- stores/metadata.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/stores/metadata.go b/stores/metadata.go index 1bdbd13c5..36d47f523 100644 --- a/stores/metadata.go +++ b/stores/metadata.go @@ -1063,10 +1063,10 @@ func (s *SQLStore) SearchObjects(ctx context.Context, bucket, substring string, Select("o.object_id as name, MAX(o.size) as size, MIN(sla.health) as health"). Model(&dbObject{}). Table("objects o"). - Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id AND b.name = ?", bucket). + Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id"). Joins("LEFT JOIN slices sli ON o.id = sli.`db_object_id`"). Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). - Where("INSTR(o.object_id, ?) > 0 AND ?", substring, sqlWhereBucket("o", bucket)). + Where("INSTR(o.object_id, ?) > 0 AND b.name = ?", substring, bucket). Group("o.object_id"). Offset(offset). Limit(limit). @@ -1140,10 +1140,10 @@ SELECT FROM ( SELECT MAX(etag) AS etag, MAX(objects.created_at) AS created_at, MAX(size) AS size, MIN(slabs.health) as health, MAX(objects.mime_type) as mimeType, SUBSTR(object_id, ?) AS trimmed , INSTR(SUBSTR(object_id, ?), "/") AS slashindex FROM objects - INNER JOIN buckets b ON objects.db_bucket_id = b.id AND b.name = ? + INNER JOIN buckets b ON objects.db_bucket_id = b.id LEFT JOIN slices ON objects.id = slices.db_object_id LEFT JOIN slabs ON slices.db_slab_id = slabs.id - WHERE SUBSTR(object_id, 1, ?) = ? AND ? + WHERE SUBSTR(object_id, 1, ?) = ? AND b.name = ? GROUP BY object_id ) AS m GROUP BY name @@ -1159,11 +1159,10 @@ HAVING SUBSTR(name, 1, ?) = ? AND name != ? utf8.RuneCountInString(path) + 1, // SUBSTR(object_id, ?) utf8.RuneCountInString(path) + 1, // INSTR(SUBSTR(object_id, ?), "/") - bucket, // b.name = ? - utf8.RuneCountInString(path), // WHERE SUBSTR(object_id, 1, ?) = ? AND ? - path, // WHERE SUBSTR(object_id, 1, ?) = ? AND ? - sqlWhereBucket("objects", bucket), // WHERE SUBSTR(object_id, 1, ?) = ? AND ? + utf8.RuneCountInString(path), // WHERE SUBSTR(object_id, 1, ?) = ? AND b.name = ? + path, // WHERE SUBSTR(object_id, 1, ?) = ? AND b.name = ? + bucket, // WHERE SUBSTR(object_id, 1, ?) = ? AND b.name = ? utf8.RuneCountInString(path + prefix), // HAVING SUBSTR(name, 1, ?) = ? AND name != ? path + prefix, // HAVING SUBSTR(name, 1, ?) = ? AND name != ? @@ -1178,7 +1177,7 @@ HAVING SUBSTR(name, 1, ?) = ? AND name != ? case api.ObjectSortByHealth: var markerHealth float64 if err = s.db. - Raw(fmt.Sprintf(`SELECT health FROM (%s ORDER BY name) as n WHERE name >= ? LIMIT 1`, objectsQuery), append(objectsQueryParams, marker)...). + Raw(fmt.Sprintf(`SELECT health FROM (%s AND name >= ? ORDER BY name LIMIT 1) as n`, objectsQuery), append(objectsQueryParams, marker)...). Scan(&markerHealth). Error; err != nil { return @@ -2003,12 +2002,12 @@ func (s *SQLStore) object(ctx context.Context, txn *gorm.DB, bucket string, path Select("o.id as ObjectID, o.key as ObjectKey, o.object_id as ObjectName, o.size as ObjectSize, o.mime_type as ObjectMimeType, o.created_at as ObjectModTime, o.etag as ObjectETag, sli.id as SliceID, sli.offset as SliceOffset, sli.length as SliceLength, sla.id as SlabID, sla.health as SlabHealth, sla.key as SlabKey, sla.min_shards as SlabMinShards, bs.id IS NOT NULL AS SlabBuffered, sec.slab_index as SectorIndex, sec.root as SectorRoot, sec.latest_host as SectorHost"). Model(&dbObject{}). Table("objects o"). - Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id AND b.name = ?", bucket). + Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id"). Joins("LEFT JOIN slices sli ON o.id = sli.`db_object_id`"). Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). Joins("LEFT JOIN sectors sec ON sla.id = sec.`db_slab_id`"). Joins("LEFT JOIN buffered_slabs bs ON sla.db_buffered_slab_id = bs.`id`"). - Where("o.object_id = ? AND ?", path, sqlWhereBucket("o", bucket)). + Where("o.object_id = ? AND b.name = ?", path, bucket). Order("sli.id ASC"). Order("sec.slab_index ASC"). Scan(&rows) @@ -2437,10 +2436,10 @@ func (s *SQLStore) ListObjects(ctx context.Context, bucket, prefix, sortBy, sort Select("o.object_id as Name, MAX(o.size) as Size, MIN(sla.health) as Health, MAX(o.mime_type) as mimeType, MAX(o.created_at) as ModTime"). Model(&dbObject{}). Table("objects o"). - Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id AND b.name = ?", bucket). + Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id"). Joins("LEFT JOIN slices sli ON o.id = sli.`db_object_id`"). Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). - Where("? AND ? AND ?", sqlWhereBucket("o", bucket), prefixExpr, markerExpr). + Where("b.name = ? AND ? AND ?", bucket, prefixExpr, markerExpr). Group("o.object_id"). Order(orderBy). Order(markerOrderBy). @@ -2590,10 +2589,10 @@ func buildMarkerExpr(db *gorm.DB, bucket, prefix, marker, sortBy, sortDir string Select("MIN(sla.health)"). Model(&dbObject{}). Table("objects o"). - Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id AND b.name = ?", bucket). + Joins("INNER JOIN buckets b ON o.db_bucket_id = b.id"). Joins("LEFT JOIN slices sli ON o.id = sli.`db_object_id`"). Joins("LEFT JOIN slabs sla ON sli.db_slab_id = sla.`id`"). - Where("? AND ? AND ?", sqlWhereBucket("o", bucket), buildPrefixExpr(prefix), gorm.Expr("o.object_id >= ?", marker)). + Where("b.name = ? AND ? AND ?", bucket, buildPrefixExpr(prefix), gorm.Expr("o.object_id >= ?", marker)). Group("o.object_id"). Limit(1). Scan(&markerHealth).