X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/6599088b45103087b4be743fd51a8330e694e57f..d9e495b553de6d76aaf6c4735977315e6fb0e51f:/services/keepstore/s3_volume.go

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index d34b8772c5..3f08f6e1b8 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -1,3 +1,7 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
 package main
 
 import (
@@ -129,7 +133,7 @@ func init() {
 		&s3UnsafeDelete,
 		"s3-unsafe-delete",
 		false,
-		"EXPERIMENTAL. Enable deletion (garbage collection), even though there are known race conditions that can cause data loss.")
+		"EXPERIMENTAL. Enable deletion (garbage collection) even when trash lifetime is zero, even though there are known race conditions that can cause data loss.")
 }
 
 // S3Volume implements Volume using an S3 bucket.
@@ -239,6 +243,11 @@ func (v *S3Volume) Start() error {
 	return nil
 }
 
+// DeviceID returns a globally unique ID for the storage bucket.
+func (v *S3Volume) DeviceID() string {
+	return "s3://" + v.Endpoint + "/" + v.Bucket
+}
+
 func (v *S3Volume) getReaderWithContext(ctx context.Context, loc string) (rdr io.ReadCloser, err error) {
 	ready := make(chan bool)
 	go func() {
@@ -331,6 +340,40 @@ func (v *S3Volume) Get(ctx context.Context, loc string, buf []byte) (int, error)
 
 // Compare the given data with the stored data.
 func (v *S3Volume) Compare(ctx context.Context, loc string, expect []byte) error {
+	errChan := make(chan error, 1)
+	go func() {
+		_, err := v.bucket.Head("recent/"+loc, nil)
+		errChan <- err
+	}()
+	var err error
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case err = <-errChan:
+	}
+	if err != nil {
+		// Checking for "loc" itself here would interfere with
+		// future GET requests.
+		//
+		// On AWS, if X doesn't exist, a HEAD or GET request
+		// for X causes X's non-existence to be cached. Thus,
+		// if we test for X, then create X and return a
+		// signature to our client, the client might still get
+		// 404 from all keepstores when trying to read it.
+		//
+		// To avoid this, we avoid doing HEAD X or GET X until
+		// we know X has been written.
+		//
+		// Note that X might exist even though recent/X
+		// doesn't: for example, the response to HEAD recent/X
+		// might itself come from a stale cache. In such
+		// cases, we will return a false negative and
+		// PutHandler might needlessly create another replica
+		// on a different volume. That's not ideal, but it's
+		// better than passing the eventually-consistent
+		// problem on to our clients.
+		return v.translateError(err)
+	}
 	rdr, err := v.getReaderWithContext(ctx, loc)
 	if err != nil {
 		return err