From 6921328b9395092bfb01cf7498484d13f34c59ac Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 29 Jun 2015 10:59:27 -0700 Subject: [PATCH] Avoid frivolous GetObjectMetadata() calls at driver level, return back all the information in donut ListObjects() --- pkg/storage/donut/bucket.go | 76 ++++++++++++++++-------------- pkg/storage/donut/definitions.go | 6 +-- pkg/storage/donut/donut_test.go | 10 ++-- pkg/storage/drivers/donut/donut.go | 31 ++++++------ 4 files changed, 66 insertions(+), 57 deletions(-) diff --git a/pkg/storage/donut/bucket.go b/pkg/storage/donut/bucket.go index 9c5d54225..944f3eb85 100644 --- a/pkg/storage/donut/bucket.go +++ b/pkg/storage/donut/bucket.go @@ -84,28 +84,6 @@ func newBucket(bucketName, aclType, donutName string, nodes map[string]node) (bu func (b bucket) getBucketName() string { return b.name } - -func (b bucket) GetObjectMetadata(objectName string) (ObjectMetadata, error) { - b.lock.RLock() - defer b.lock.RUnlock() - metadataReaders, err := b.getDiskReaders(normalizeObjectName(objectName), objectMetadataConfig) - if err != nil { - return ObjectMetadata{}, iodine.New(err, nil) - } - for _, metadataReader := range metadataReaders { - defer metadataReader.Close() - } - objMetadata := ObjectMetadata{} - for _, metadataReader := range metadataReaders { - jdec := json.NewDecoder(metadataReader) - if err := jdec.Decode(&objMetadata); err != nil { - return ObjectMetadata{}, iodine.New(err, nil) - } - return objMetadata, nil - } - return ObjectMetadata{}, iodine.New(InvalidArgument{}, nil) -} - func (b bucket) getBucketMetadataReaders() ([]io.ReadCloser, error) { var readers []io.ReadCloser for _, node := range b.nodes { @@ -144,6 +122,13 @@ func (b bucket) getBucketMetadata() (*AllBuckets, error) { return nil, iodine.New(InvalidArgument{}, nil) } +// GetObjectMetadata - get metadata for an object +func (b bucket) GetObjectMetadata(objectName string) (ObjectMetadata, error) { + b.lock.RLock() + defer b.lock.RUnlock() + return b.readObjectMetadata(objectName) +} + // ListObjects - list all objects func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) (ListObjects, error) { b.lock.RLock() @@ -191,13 +176,20 @@ func (b bucket) ListObjects(prefix, marker, delimiter string, maxkeys int) (List } results = AppendU(results, prefix+objectName) } - sort.Strings(results) sort.Strings(commonPrefixes) listObjects := ListObjects{} - listObjects.Objects = results + listObjects.Objects = make(map[string]ObjectMetadata) listObjects.CommonPrefixes = commonPrefixes listObjects.IsTruncated = isTruncated + + for _, objectName := range results { + objMetadata, err := b.readObjectMetadata(normalizeObjectName(objectName)) + if err != nil { + return ListObjects{}, iodine.New(err, nil) + } + listObjects.Objects[objectName] = objMetadata + } return listObjects, nil } @@ -215,21 +207,10 @@ func (b bucket) ReadObject(objectName string) (reader io.ReadCloser, size int64, if _, ok := bucketMetadata.Buckets[b.getBucketName()].BucketObjects[objectName]; !ok { return nil, 0, iodine.New(ObjectNotFound{Object: objectName}, nil) } - objMetadata := ObjectMetadata{} - metadataReaders, err := b.getDiskReaders(normalizeObjectName(objectName), objectMetadataConfig) + objMetadata, err := b.readObjectMetadata(normalizeObjectName(objectName)) if err != nil { return nil, 0, iodine.New(err, nil) } - for _, metadataReader := range metadataReaders { - defer metadataReader.Close() - } - for _, metadataReader := range metadataReaders { - jdec := json.NewDecoder(metadataReader) - if err := jdec.Decode(&objMetadata); err != nil { - return nil, 0, iodine.New(err, nil) - } - break - } // read and reply back to GetObject() request in a go-routine go b.readEncodedData(normalizeObjectName(objectName), writer, objMetadata) return reader, objMetadata.Size, nil @@ -346,6 +327,29 @@ func (b bucket) writeObjectMetadata(objectName string, objMetadata *ObjectMetada return nil } +// readObjectMetadata - read object metadata +func (b bucket) readObjectMetadata(objectName string) (ObjectMetadata, error) { + objMetadata := ObjectMetadata{} + if objectName == "" { + return ObjectMetadata{}, iodine.New(InvalidArgument{}, nil) + } + objMetadataReaders, err := b.getDiskReaders(objectName, objectMetadataConfig) + if err != nil { + return ObjectMetadata{}, iodine.New(err, nil) + } + for _, objMetadataReader := range objMetadataReaders { + defer objMetadataReader.Close() + } + for _, objMetadataReader := range objMetadataReaders { + jdec := json.NewDecoder(objMetadataReader) + if err := jdec.Decode(&objMetadata); err != nil { + return ObjectMetadata{}, iodine.New(err, nil) + } + break + } + return objMetadata, nil +} + // TODO - This a temporary normalization of objectNames, need to find a better way // // normalizedObjectName - all objectNames with "/" get normalized to a simple objectName diff --git a/pkg/storage/donut/definitions.go b/pkg/storage/donut/definitions.go index d1212c943..f94a54be8 100644 --- a/pkg/storage/donut/definitions.go +++ b/pkg/storage/donut/definitions.go @@ -67,7 +67,7 @@ type BucketMetadata struct { // ListObjects container for list objects response type ListObjects struct { - Objects []string `json:"objects"` - CommonPrefixes []string `json:"commonPrefixes"` - IsTruncated bool `json:"isTruncated"` + Objects map[string]ObjectMetadata `json:"objects"` + CommonPrefixes []string `json:"commonPrefixes"` + IsTruncated bool `json:"isTruncated"` } diff --git a/pkg/storage/donut/donut_test.go b/pkg/storage/donut/donut_test.go index 2993f8de6..67a8972a1 100644 --- a/pkg/storage/donut/donut_test.go +++ b/pkg/storage/donut/donut_test.go @@ -93,7 +93,7 @@ func (s *MySuite) TestEmptyBucket(c *C) { // check if bucket is empty listObjects, err := donut.ListObjects("foo", "", "", "", 1) c.Assert(err, IsNil) - c.Assert(listObjects.Objects, IsNil) + c.Assert(len(listObjects.Objects), Equals, 0) c.Assert(listObjects.CommonPrefixes, IsNil) c.Assert(listObjects.IsTruncated, Equals, false) } @@ -312,7 +312,8 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { // test list objects with only delimiter listObjects, err = donut.ListObjects("foo", "", "", "1", 10) c.Assert(err, IsNil) - c.Assert(listObjects.Objects[0], Equals, "obj2") + _, ok := listObjects.Objects["obj2"] + c.Assert(ok, Equals, true) c.Assert(listObjects.IsTruncated, Equals, false) c.Assert(listObjects.CommonPrefixes[0], Equals, "obj1") @@ -320,7 +321,10 @@ func (s *MySuite) TestMultipleNewObjects(c *C) { listObjects, err = donut.ListObjects("foo", "o", "", "", 10) c.Assert(err, IsNil) c.Assert(listObjects.IsTruncated, Equals, false) - c.Assert(listObjects.Objects, DeepEquals, []string{"obj1", "obj2"}) + _, ok1 := listObjects.Objects["obj1"] + _, ok2 := listObjects.Objects["obj2"] + c.Assert(ok1, Equals, true) + c.Assert(ok2, Equals, true) three := ioutil.NopCloser(bytes.NewReader([]byte("three"))) metadata["contentLength"] = strconv.Itoa(len("three")) diff --git a/pkg/storage/drivers/donut/donut.go b/pkg/storage/drivers/donut/donut.go index 094ccf0a8..3b89d27ac 100644 --- a/pkg/storage/drivers/donut/donut.go +++ b/pkg/storage/drivers/donut/donut.go @@ -448,6 +448,12 @@ func (d donutDriver) GetObjectMetadata(bucketName, objectName string) (drivers.O return objectMetadata, nil } +type byObjectName []drivers.ObjectMetadata + +func (b byObjectName) Len() int { return len(b) } +func (b byObjectName) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byObjectName) Less(i, j int) bool { return b[i].Key < b[j].Key } + // ListObjects - returns list of objects func (d donutDriver) ListObjects(bucketName string, resources drivers.BucketResourcesMetadata) ([]drivers.ObjectMetadata, drivers.BucketResourcesMetadata, error) { d.lock.RLock() @@ -470,23 +476,18 @@ func (d donutDriver) ListObjects(bucketName string, resources drivers.BucketReso } resources.CommonPrefixes = listObjects.CommonPrefixes resources.IsTruncated = listObjects.IsTruncated - if resources.IsTruncated && resources.IsDelimiterSet() { - resources.NextMarker = listObjects.Objects[len(listObjects.Objects)-1] - } - // make sure to keep the lexical order same as returned by donut - // we do not have to sort here again - results := make([]drivers.ObjectMetadata, len(listObjects.Objects)) - for i, objectName := range listObjects.Objects { - objectMetadata, err := d.donut.GetObjectMetadata(bucketName, objectName) - if err != nil { - return nil, drivers.BucketResourcesMetadata{}, iodine.New(err, errParams) - } + var results []drivers.ObjectMetadata + for _, objMetadata := range listObjects.Objects { metadata := drivers.ObjectMetadata{ - Key: objectMetadata.Object, - Created: objectMetadata.Created, - Size: objectMetadata.Size, + Key: objMetadata.Object, + Created: objMetadata.Created, + Size: objMetadata.Size, } - results[i] = metadata + results = append(results, metadata) + } + sort.Sort(byObjectName(results)) + if resources.IsTruncated && resources.IsDelimiterSet() { + resources.NextMarker = results[len(results)-1].Key } return results, resources, nil }