From 1f03d4e77d3c102acf013a7fc359714a5b1f25d8 Mon Sep 17 00:00:00 2001 From: Stefan Majewsky Date: Fri, 6 May 2016 13:14:21 +0200 Subject: [PATCH] [Swift] add simple heuristic to detect incomplete DLOs during read ops This is similar to waitForSegmentsToShowUp which is called during Close/Commit. Intuitively, you wouldn't expect missing segments to be a problem during read operations, since the previous Close/Commit confirmed that all segments are there. But due to the distributed nature of Swift, the read request could be hitting a different storage node of the Swift cluster, where the segments are still missing. Load tests on my team's staging Swift cluster have shown this to occur about once every 100-200 layer uploads when the Swift proxies are under high load. The retry logic, borrowed from waitForSegmentsToShowUp, fixes this temporary inconsistency. Signed-off-by: Stefan Majewsky --- registry/storage/driver/swift/swift.go | 77 ++++++++++++++++++++------ 1 file changed, 61 insertions(+), 16 deletions(-) diff --git a/registry/storage/driver/swift/swift.go b/registry/storage/driver/swift/swift.go index 4191b8ba..b0ab34f4 100644 --- a/registry/storage/driver/swift/swift.go +++ b/registry/storage/driver/swift/swift.go @@ -302,14 +302,40 @@ func (d *driver) Reader(ctx context.Context, path string, offset int64) (io.Read headers := make(swift.Headers) headers["Range"] = "bytes=" + strconv.FormatInt(offset, 10) + "-" - file, _, err := d.Conn.ObjectOpen(d.Container, d.swiftPath(path), false, headers) - if err == swift.ObjectNotFound { - return nil, storagedriver.PathNotFoundError{Path: path} + waitingTime := readAfterWriteWait + endTime := time.Now().Add(readAfterWriteTimeout) + + for { + file, headers, err := d.Conn.ObjectOpen(d.Container, d.swiftPath(path), false, headers) + if err != nil { + if err == swift.ObjectNotFound { + return nil, storagedriver.PathNotFoundError{Path: path} + } + if swiftErr, ok := err.(*swift.Error); ok && swiftErr.StatusCode == http.StatusRequestedRangeNotSatisfiable { + return ioutil.NopCloser(bytes.NewReader(nil)), nil + } + return file, err + } + + //if this is a DLO and it is clear that segments are still missing, + //wait until they show up + _, isDLO := headers["X-Object-Manifest"] + size, err := file.Length() + if err != nil { + return file, err + } + if isDLO && size == 0 { + if time.Now().Add(waitingTime).After(endTime) { + return nil, fmt.Errorf("Timeout expired while waiting for segments of %s to show up", path) + } + time.Sleep(waitingTime) + waitingTime *= 2 + continue + } + + //if not, then this reader will be fine + return file, nil } - if swiftErr, ok := err.(*swift.Error); ok && swiftErr.StatusCode == http.StatusRequestedRangeNotSatisfiable { - return ioutil.NopCloser(bytes.NewReader(nil)), nil - } - return file, err } // Writer returns a FileWriter which will store the content written to it @@ -389,17 +415,36 @@ func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo, //Don't trust an empty `objects` slice. A container listing can be //outdated. For files, we can make a HEAD request on the object which //reports existence (at least) much more reliably. - info, _, err := d.Conn.Object(d.Container, swiftPath) - if err != nil { - if err == swift.ObjectNotFound { - return nil, storagedriver.PathNotFoundError{Path: path} + waitingTime := readAfterWriteWait + endTime := time.Now().Add(readAfterWriteTimeout) + + for { + info, headers, err := d.Conn.Object(d.Container, swiftPath) + if err != nil { + if err == swift.ObjectNotFound { + return nil, storagedriver.PathNotFoundError{Path: path} + } + return nil, err } - return nil, err + + //if this is a DLO and it is clear that segments are still missing, + //wait until they show up + _, isDLO := headers["X-Object-Manifest"] + if isDLO && info.Bytes == 0 { + if time.Now().Add(waitingTime).After(endTime) { + return nil, fmt.Errorf("Timeout expired while waiting for segments of %s to show up", path) + } + time.Sleep(waitingTime) + waitingTime *= 2 + continue + } + + //otherwise, accept the result + fi.IsDir = false + fi.Size = info.Bytes + fi.ModTime = info.LastModified + return storagedriver.FileInfoInternal{FileInfoFields: fi}, nil } - fi.IsDir = false - fi.Size = info.Bytes - fi.ModTime = info.LastModified - return storagedriver.FileInfoInternal{FileInfoFields: fi}, nil } // List returns a list of the objects that are direct descendants of the given path.