X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/6f461f4d0a996da85140982846a5d5c10ccfaae4..cd0e7f06116925d66cc370a03cef1b8c19d0002d:/services/keepstore/s3_volume.go

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index ca5b1a2eb9..e6a53d06c6 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 (
@@ -14,7 +18,6 @@ import (
 	"regexp"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
@@ -130,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.
@@ -240,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() {
@@ -332,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
@@ -378,7 +420,7 @@ func (v *S3Volume) Put(ctx context.Context, loc string, block []byte) error {
 		if err != nil {
 			return
 		}
-		err = v.bucket.Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
+		err = v.bucket.PutReader("recent/"+loc, nil, 0, "application/octet-stream", s3ACL, s3.Options{})
 	}()
 	select {
 	case <-ctx.Done():
@@ -410,7 +452,7 @@ func (v *S3Volume) Touch(loc string) error {
 	} else if err != nil {
 		return err
 	}
-	err = v.bucket.Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
+	err = v.bucket.PutReader("recent/"+loc, nil, 0, "application/octet-stream", s3ACL, s3.Options{})
 	return v.translateError(err)
 }
 
@@ -424,7 +466,7 @@ func (v *S3Volume) Mtime(loc string) (time.Time, error) {
 	err = v.translateError(err)
 	if os.IsNotExist(err) {
 		// The data object X exists, but recent/X is missing.
-		err = v.bucket.Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
+		err = v.bucket.PutReader("recent/"+loc, nil, 0, "application/octet-stream", s3ACL, s3.Options{})
 		if err != nil {
 			log.Printf("error: creating %q: %s", "recent/"+loc, err)
 			return zeroTime, v.translateError(err)
@@ -456,10 +498,10 @@ func (v *S3Volume) IndexTo(prefix string, writer io.Writer) error {
 		Prefix:   "recent/" + prefix,
 		PageSize: v.IndexPageSize,
 	}
-	v.bucket.stats.tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
-	v.bucket.stats.tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
+	v.bucket.stats.Tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
+	v.bucket.stats.Tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
 	for data, recent := dataL.First(), recentL.First(); data != nil; data = dataL.Next() {
-		v.bucket.stats.tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
+		v.bucket.stats.Tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
 		if data.Key >= "g" {
 			// Conveniently, "recent/*" and "trash/*" are
 			// lexically greater than all hex-encoded data
@@ -481,12 +523,12 @@ func (v *S3Volume) IndexTo(prefix string, writer io.Writer) error {
 		for recent != nil {
 			if cmp := strings.Compare(recent.Key[7:], data.Key); cmp < 0 {
 				recent = recentL.Next()
-				v.bucket.stats.tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
+				v.bucket.stats.Tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
 				continue
 			} else if cmp == 0 {
 				stamp = recent
 				recent = recentL.Next()
-				v.bucket.stats.tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
+				v.bucket.stats.Tick(&v.bucket.stats.Ops, &v.bucket.stats.ListOps)
 				break
 			} else {
 				// recent/X marker is missing: we'll
@@ -606,7 +648,7 @@ func (v *S3Volume) Untrash(loc string) error {
 	if err != nil {
 		return err
 	}
-	err = v.bucket.Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
+	err = v.bucket.PutReader("recent/"+loc, nil, 0, "application/octet-stream", s3ACL, s3.Options{})
 	return v.translateError(err)
 }
 
@@ -872,82 +914,60 @@ type s3bucket struct {
 
 func (b *s3bucket) GetReader(path string) (io.ReadCloser, error) {
 	rdr, err := b.Bucket.GetReader(path)
-	b.stats.tick(&b.stats.Ops, &b.stats.GetOps)
-	b.stats.tickErr(err)
-	return NewCountingReader(rdr, b.stats.tickInBytes), err
+	b.stats.Tick(&b.stats.Ops, &b.stats.GetOps)
+	b.stats.TickErr(err)
+	return NewCountingReader(rdr, b.stats.TickInBytes), err
 }
 
 func (b *s3bucket) Head(path string, headers map[string][]string) (*http.Response, error) {
 	resp, err := b.Bucket.Head(path, headers)
-	b.stats.tick(&b.stats.Ops, &b.stats.HeadOps)
-	b.stats.tickErr(err)
+	b.stats.Tick(&b.stats.Ops, &b.stats.HeadOps)
+	b.stats.TickErr(err)
 	return resp, err
 }
 
 func (b *s3bucket) PutReader(path string, r io.Reader, length int64, contType string, perm s3.ACL, options s3.Options) error {
-	err := b.Bucket.PutReader(path, NewCountingReader(r, b.stats.tickOutBytes), length, contType, perm, options)
-	b.stats.tick(&b.stats.Ops, &b.stats.PutOps)
-	b.stats.tickErr(err)
-	return err
-}
-
-func (b *s3bucket) Put(path string, data []byte, contType string, perm s3.ACL, options s3.Options) error {
-	err := b.Bucket.PutReader(path, NewCountingReader(bytes.NewBuffer(data), b.stats.tickOutBytes), int64(len(data)), contType, perm, options)
-	b.stats.tick(&b.stats.Ops, &b.stats.PutOps)
-	b.stats.tickErr(err)
+	if length == 0 {
+		// goamz will only send Content-Length: 0 when reader
+		// is nil due to net.http.Request.ContentLength
+		// behavior.  Otherwise, Content-Length header is
+		// omitted which will cause some S3 services
+		// (including AWS and Ceph RadosGW) to fail to create
+		// empty objects.
+		r = nil
+	} else {
+		r = NewCountingReader(r, b.stats.TickOutBytes)
+	}
+	err := b.Bucket.PutReader(path, r, length, contType, perm, options)
+	b.stats.Tick(&b.stats.Ops, &b.stats.PutOps)
+	b.stats.TickErr(err)
 	return err
 }
 
 func (b *s3bucket) Del(path string) error {
 	err := b.Bucket.Del(path)
-	b.stats.tick(&b.stats.Ops, &b.stats.DelOps)
-	b.stats.tickErr(err)
+	b.stats.Tick(&b.stats.Ops, &b.stats.DelOps)
+	b.stats.TickErr(err)
 	return err
 }
 
 type s3bucketStats struct {
-	Errors   uint64
-	Ops      uint64
-	GetOps   uint64
-	PutOps   uint64
-	HeadOps  uint64
-	DelOps   uint64
-	ListOps  uint64
-	InBytes  uint64
-	OutBytes uint64
-
-	ErrorCodes map[string]uint64 `json:",omitempty"`
-
-	lock sync.Mutex
-}
-
-func (s *s3bucketStats) tickInBytes(n uint64) {
-	atomic.AddUint64(&s.InBytes, n)
-}
-
-func (s *s3bucketStats) tickOutBytes(n uint64) {
-	atomic.AddUint64(&s.OutBytes, n)
+	statsTicker
+	Ops     uint64
+	GetOps  uint64
+	PutOps  uint64
+	HeadOps uint64
+	DelOps  uint64
+	ListOps uint64
 }
 
-func (s *s3bucketStats) tick(counters ...*uint64) {
-	for _, counter := range counters {
-		atomic.AddUint64(counter, 1)
-	}
-}
-
-func (s *s3bucketStats) tickErr(err error) {
+func (s *s3bucketStats) TickErr(err error) {
 	if err == nil {
 		return
 	}
-	atomic.AddUint64(&s.Errors, 1)
-	errStr := fmt.Sprintf("%T", err)
+	errType := fmt.Sprintf("%T", err)
 	if err, ok := err.(*s3.Error); ok {
-		errStr = errStr + fmt.Sprintf(" %d %s", err.StatusCode, err.Code)
-	}
-	s.lock.Lock()
-	if s.ErrorCodes == nil {
-		s.ErrorCodes = make(map[string]uint64)
+		errType = errType + fmt.Sprintf(" %d %s", err.StatusCode, err.Code)
 	}
-	s.ErrorCodes[errStr]++
-	s.lock.Unlock()
+	s.statsTicker.TickErr(err, errType)
 }