X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/863570108a2c901a8eff22dc8a9bc72635ba7b95..b405f0f487f35f62d8362dc06981b83176b77d44:/services/keepstore/azure_blob_volume.go diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go index b21f68d683..75344890ab 100644 --- a/services/keepstore/azure_blob_volume.go +++ b/services/keepstore/azure_blob_volume.go @@ -8,7 +8,6 @@ import ( "fmt" "io" "io/ioutil" - "log" "net/http" "os" "regexp" @@ -18,6 +17,7 @@ import ( "time" "git.curoverse.com/arvados.git/sdk/go/arvados" + log "github.com/Sirupsen/logrus" "github.com/curoverse/azure-sdk-for-go/storage" ) @@ -187,7 +187,7 @@ func (v *AzureBlobVolume) Get(ctx context.Context, loc string, buf []byte) (int, } var deadline time.Time haveDeadline := false - size, err := v.get(loc, buf) + size, err := v.get(ctx, loc, buf) for err == nil && size == 0 && loc != "d41d8cd98f00b204e9800998ecf8427e" { // Seeing a brand new empty block probably means we're // in a race with CreateBlob, which under the hood @@ -208,8 +208,12 @@ func (v *AzureBlobVolume) Get(ctx context.Context, loc string, buf []byte) (int, } else if time.Now().After(deadline) { break } - time.Sleep(azureWriteRacePollTime) - size, err = v.get(loc, buf) + select { + case <-ctx.Done(): + return 0, ctx.Err() + case <-time.After(azureWriteRacePollTime): + } + size, err = v.get(ctx, loc, buf) } if haveDeadline { log.Printf("Race ended with size==%d", size) @@ -217,7 +221,9 @@ func (v *AzureBlobVolume) Get(ctx context.Context, loc string, buf []byte) (int, return size, err } -func (v *AzureBlobVolume) get(loc string, buf []byte) (int, error) { +func (v *AzureBlobVolume) get(ctx context.Context, loc string, buf []byte) (int, error) { + ctx, cancel := context.WithCancel(ctx) + defer cancel() expectSize := len(buf) if azureMaxGetBytes < BlockSize { // Unfortunately the handler doesn't tell us how long the blob @@ -239,10 +245,18 @@ func (v *AzureBlobVolume) get(loc string, buf []byte) (int, error) { // We'll update this actualSize if/when we get the last piece. actualSize := -1 pieces := (expectSize + azureMaxGetBytes - 1) / azureMaxGetBytes - errors := make([]error, pieces) + errors := make(chan error, pieces) var wg sync.WaitGroup wg.Add(pieces) for p := 0; p < pieces; p++ { + // Each goroutine retrieves one piece. If we hit an + // error, it is sent to the errors chan so get() can + // return it -- but only if the error happens before + // ctx is done. This way, if ctx is done before we hit + // any other error (e.g., requesting client has hung + // up), we return the original ctx.Err() instead of + // the secondary errors from the transfers that got + // interrupted as a result. go func(p int) { defer wg.Done() startPos := p * azureMaxGetBytes @@ -252,23 +266,51 @@ func (v *AzureBlobVolume) get(loc string, buf []byte) (int, error) { } var rdr io.ReadCloser var err error - if startPos == 0 && endPos == expectSize { - rdr, err = v.bsClient.GetBlob(v.ContainerName, loc) - } else { - rdr, err = v.bsClient.GetBlobRange(v.ContainerName, loc, fmt.Sprintf("%d-%d", startPos, endPos-1), nil) + gotRdr := make(chan struct{}) + go func() { + defer close(gotRdr) + if startPos == 0 && endPos == expectSize { + rdr, err = v.bsClient.GetBlob(v.ContainerName, loc) + } else { + rdr, err = v.bsClient.GetBlobRange(v.ContainerName, loc, fmt.Sprintf("%d-%d", startPos, endPos-1), nil) + } + }() + select { + case <-ctx.Done(): + go func() { + <-gotRdr + if err == nil { + rdr.Close() + } + }() + return + case <-gotRdr: } if err != nil { - errors[p] = err + errors <- err + cancel() return } - defer rdr.Close() + go func() { + // Close the reader when the client + // hangs up or another piece fails + // (possibly interrupting ReadFull()) + // or when all pieces succeed and + // get() returns. + <-ctx.Done() + rdr.Close() + }() n, err := io.ReadFull(rdr, buf[startPos:endPos]) if pieces == 1 && (err == io.ErrUnexpectedEOF || err == io.EOF) { // If we don't know the actual size, // and just tried reading 64 MiB, it's // normal to encounter EOF. } else if err != nil { - errors[p] = err + if ctx.Err() == nil { + errors <- err + } + cancel() + return } if p == pieces-1 { actualSize = startPos + n @@ -276,16 +318,18 @@ func (v *AzureBlobVolume) get(loc string, buf []byte) (int, error) { }(p) } wg.Wait() - for _, err := range errors { - if err != nil { - return 0, v.translateError(err) - } + close(errors) + if len(errors) > 0 { + return 0, v.translateError(<-errors) + } + if ctx.Err() != nil { + return 0, ctx.Err() } return actualSize, nil } // Compare the given data with existing stored data. -func (v *AzureBlobVolume) Compare(loc string, expect []byte) error { +func (v *AzureBlobVolume) Compare(ctx context.Context, loc string, expect []byte) error { trashed, _, err := v.checkTrashed(loc) if err != nil { return err @@ -293,20 +337,65 @@ func (v *AzureBlobVolume) Compare(loc string, expect []byte) error { if trashed { return os.ErrNotExist } - rdr, err := v.bsClient.GetBlob(v.ContainerName, loc) + var rdr io.ReadCloser + gotRdr := make(chan struct{}) + go func() { + defer close(gotRdr) + rdr, err = v.bsClient.GetBlob(v.ContainerName, loc) + }() + select { + case <-ctx.Done(): + go func() { + <-gotRdr + if err == nil { + rdr.Close() + } + }() + return ctx.Err() + case <-gotRdr: + } if err != nil { return v.translateError(err) } defer rdr.Close() - return compareReaderWithBuf(rdr, expect, loc[:32]) + return compareReaderWithBuf(ctx, rdr, expect, loc[:32]) } // Put stores a Keep block as a block blob in the container. -func (v *AzureBlobVolume) Put(loc string, block []byte) error { +func (v *AzureBlobVolume) Put(ctx context.Context, loc string, block []byte) error { if v.ReadOnly { return MethodDisabledError } - return v.bsClient.CreateBlockBlobFromReader(v.ContainerName, loc, uint64(len(block)), bytes.NewReader(block), nil) + // Send the block data through a pipe, so that (if we need to) + // we can close the pipe early and abandon our + // CreateBlockBlobFromReader() goroutine, without worrying + // about CreateBlockBlobFromReader() accessing our block + // buffer after we release it. + bufr, bufw := io.Pipe() + go func() { + io.Copy(bufw, bytes.NewReader(block)) + bufw.Close() + }() + errChan := make(chan error) + go func() { + errChan <- v.bsClient.CreateBlockBlobFromReader(v.ContainerName, loc, uint64(len(block)), bufr, nil) + }() + select { + case <-ctx.Done(): + theConfig.debugLogf("%s: taking CreateBlockBlobFromReader's input away: %s", v, ctx.Err()) + // Our pipe might be stuck in Write(), waiting for + // io.Copy() to read. If so, un-stick it. This means + // CreateBlockBlobFromReader will get corrupt data, + // but that's OK: the size won't match, so the write + // will fail. + go io.Copy(ioutil.Discard, bufr) + // CloseWithError() will return once pending I/O is done. + bufw.CloseWithError(ctx.Err()) + theConfig.debugLogf("%s: abandoning CreateBlockBlobFromReader goroutine", v) + return ctx.Err() + case err := <-errChan: + return err + } } // Touch updates the last-modified property of a block blob.