[arvados] created: 2.7.0-5094-gda83807d6b

git repository hosting git at public.arvados.org
Thu Oct 26 20:34:18 UTC 2023


        at  da83807d6bcef1c1f0bb78479c5ec17f150f5eda (commit)


commit da83807d6bcef1c1f0bb78479c5ec17f150f5eda
Author: Tom Clegg <tom at curii.com>
Date:   Thu Oct 26 16:15:16 2023 -0400

    21126: Add AllowTrashWhenReadOnly flag.
    
    Split KeepMount's ReadOnly flag into AllowWrite and AllowTrash flags.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 32727b1bce..593abad854 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1684,6 +1684,7 @@ Clusters:
             ReadOnly: false
           "http://host1.example:25107": {}
         ReadOnly: false
+        AllowTrashWhenReadOnly: false
         Replication: 1
         StorageClasses:
           # If you have configured storage classes (see StorageClasses
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index a3e54952da..5db0222e27 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -316,12 +316,13 @@ type StorageClassConfig struct {
 }
 
 type Volume struct {
-	AccessViaHosts   map[URL]VolumeAccess
-	ReadOnly         bool
-	Replication      int
-	StorageClasses   map[string]bool
-	Driver           string
-	DriverParameters json.RawMessage
+	AccessViaHosts         map[URL]VolumeAccess
+	ReadOnly               bool
+	AllowTrashWhenReadOnly bool
+	Replication            int
+	StorageClasses         map[string]bool
+	Driver                 string
+	DriverParameters       json.RawMessage
 }
 
 type S3VolumeDriverParameters struct {
diff --git a/sdk/go/arvados/keep_service.go b/sdk/go/arvados/keep_service.go
index 5b6d71a4fb..85750d8cfc 100644
--- a/sdk/go/arvados/keep_service.go
+++ b/sdk/go/arvados/keep_service.go
@@ -33,7 +33,8 @@ type KeepService struct {
 type KeepMount struct {
 	UUID           string          `json:"uuid"`
 	DeviceID       string          `json:"device_id"`
-	ReadOnly       bool            `json:"read_only"`
+	AllowWrite     bool            `json:"allow_write"`
+	AllowTrash     bool            `json:"allow_trash"`
 	Replication    int             `json:"replication"`
 	StorageClasses map[string]bool `json:"storage_classes"`
 }
diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 215c5e1b5b..e44dfeda87 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -227,7 +227,7 @@ func (bal *Balancer) cleanupMounts() {
 	rwdev := map[string]*KeepService{}
 	for _, srv := range bal.KeepServices {
 		for _, mnt := range srv.mounts {
-			if !mnt.ReadOnly {
+			if mnt.AllowWrite {
 				rwdev[mnt.UUID] = srv
 			}
 		}
@@ -237,7 +237,7 @@ func (bal *Balancer) cleanupMounts() {
 	for _, srv := range bal.KeepServices {
 		var dedup []*KeepMount
 		for _, mnt := range srv.mounts {
-			if mnt.ReadOnly && rwdev[mnt.UUID] != nil {
+			if !mnt.AllowWrite && rwdev[mnt.UUID] != nil {
 				bal.logf("skipping srv %s readonly mount %q because same volume is mounted read-write on srv %s", srv, mnt.UUID, rwdev[mnt.UUID])
 			} else {
 				dedup = append(dedup, mnt)
@@ -587,9 +587,11 @@ func (bal *Balancer) setupLookupTables() {
 		for _, mnt := range srv.mounts {
 			bal.mounts++
 
-			// All mounts on a read-only service are
-			// effectively read-only.
-			mnt.ReadOnly = mnt.ReadOnly || srv.ReadOnly
+			if srv.ReadOnly {
+				// All mounts on a read-only service
+				// are effectively read-only.
+				mnt.AllowWrite = false
+			}
 
 			for class := range mnt.StorageClasses {
 				if mbc := bal.mountsByClass[class]; mbc == nil {
@@ -667,7 +669,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
 			slots = append(slots, slot{
 				mnt:  mnt,
 				repl: repl,
-				want: repl != nil && mnt.ReadOnly,
+				want: repl != nil && !mnt.AllowTrash,
 			})
 		}
 	}
@@ -756,7 +758,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
 				protMnt[slot.mnt] = true
 				replProt += slot.mnt.Replication
 			}
-			if replWant < desired && (slot.repl != nil || !slot.mnt.ReadOnly) {
+			if replWant < desired && (slot.repl != nil || slot.mnt.AllowWrite) {
 				slots[i].want = true
 				wantSrv[slot.mnt.KeepService] = true
 				wantMnt[slot.mnt] = true
@@ -875,7 +877,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
 		case slot.repl == nil && slot.want && len(blk.Replicas) == 0:
 			lost = true
 			change = changeNone
-		case slot.repl == nil && slot.want && !slot.mnt.ReadOnly:
+		case slot.repl == nil && slot.want && slot.mnt.AllowWrite:
 			slot.mnt.KeepService.AddPull(Pull{
 				SizedDigest: blkid,
 				From:        blk.Replicas[0].KeepMount.KeepService,
diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go
index db9a668481..962bd40ade 100644
--- a/services/keep-balance/balance_run_test.go
+++ b/services/keep-balance/balance_run_test.go
@@ -89,21 +89,29 @@ var stubMounts = map[string][]arvados.KeepMount{
 		UUID:           "zzzzz-ivpuk-000000000000000",
 		DeviceID:       "keep0-vol0",
 		StorageClasses: map[string]bool{"default": true},
+		AllowWrite:     true,
+		AllowTrash:     true,
 	}},
 	"keep1.zzzzz.arvadosapi.com:25107": {{
 		UUID:           "zzzzz-ivpuk-100000000000000",
 		DeviceID:       "keep1-vol0",
 		StorageClasses: map[string]bool{"default": true},
+		AllowWrite:     true,
+		AllowTrash:     true,
 	}},
 	"keep2.zzzzz.arvadosapi.com:25107": {{
 		UUID:           "zzzzz-ivpuk-200000000000000",
 		DeviceID:       "keep2-vol0",
 		StorageClasses: map[string]bool{"default": true},
+		AllowWrite:     true,
+		AllowTrash:     true,
 	}},
 	"keep3.zzzzz.arvadosapi.com:25107": {{
 		UUID:           "zzzzz-ivpuk-300000000000000",
 		DeviceID:       "keep3-vol0",
 		StorageClasses: map[string]bool{"default": true},
+		AllowWrite:     true,
+		AllowTrash:     true,
 	}},
 }
 
diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index f9fca1431b..cb61ea58cc 100644
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -87,6 +87,8 @@ func (bal *balancerSuite) SetUpTest(c *check.C) {
 			KeepMount: arvados.KeepMount{
 				UUID:           fmt.Sprintf("zzzzz-mount-%015x", i),
 				StorageClasses: map[string]bool{"default": true},
+				AllowWrite:     true,
+				AllowTrash:     true,
 			},
 			KeepService: srv,
 		}}
@@ -169,7 +171,8 @@ func (bal *balancerSuite) testMultipleViews(c *check.C, readonly bool) {
 			KeepMount: arvados.KeepMount{
 				DeviceID:       bal.srvs[(i+1)%len(bal.srvs)].mounts[0].KeepMount.DeviceID,
 				UUID:           bal.srvs[(i+1)%len(bal.srvs)].mounts[0].KeepMount.UUID,
-				ReadOnly:       readonly,
+				AllowWrite:     !readonly,
+				AllowTrash:     !readonly,
 				Replication:    1,
 				StorageClasses: map[string]bool{"default": true},
 			},
@@ -374,7 +377,7 @@ func (bal *balancerSuite) TestDecreaseReplBlockTooNew(c *check.C) {
 }
 
 func (bal *balancerSuite) TestCleanupMounts(c *check.C) {
-	bal.srvs[3].mounts[0].KeepMount.ReadOnly = true
+	bal.srvs[3].mounts[0].KeepMount.AllowWrite = false
 	bal.srvs[3].mounts[0].KeepMount.DeviceID = "abcdef"
 	bal.srvs[14].mounts[0].KeepMount.UUID = bal.srvs[3].mounts[0].KeepMount.UUID
 	bal.srvs[14].mounts[0].KeepMount.DeviceID = "abcdef"
@@ -583,6 +586,8 @@ func (bal *balancerSuite) TestChangeStorageClasses(c *check.C) {
 	// classes=[special,special2].
 	bal.srvs[9].mounts = []*KeepMount{{
 		KeepMount: arvados.KeepMount{
+			AllowWrite:     true,
+			AllowTrash:     true,
 			Replication:    1,
 			StorageClasses: map[string]bool{"special": true},
 			UUID:           "zzzzz-mount-special00000009",
@@ -591,6 +596,8 @@ func (bal *balancerSuite) TestChangeStorageClasses(c *check.C) {
 		KeepService: bal.srvs[9],
 	}, {
 		KeepMount: arvados.KeepMount{
+			AllowWrite:     true,
+			AllowTrash:     true,
 			Replication:    1,
 			StorageClasses: map[string]bool{"special": true, "special2": true},
 			UUID:           "zzzzz-mount-special20000009",
@@ -603,6 +610,8 @@ func (bal *balancerSuite) TestChangeStorageClasses(c *check.C) {
 	// classes=[special3], one with classes=[default].
 	bal.srvs[13].mounts = []*KeepMount{{
 		KeepMount: arvados.KeepMount{
+			AllowWrite:     true,
+			AllowTrash:     true,
 			Replication:    1,
 			StorageClasses: map[string]bool{"special2": true},
 			UUID:           "zzzzz-mount-special2000000d",
@@ -611,6 +620,8 @@ func (bal *balancerSuite) TestChangeStorageClasses(c *check.C) {
 		KeepService: bal.srvs[13],
 	}, {
 		KeepMount: arvados.KeepMount{
+			AllowWrite:     true,
+			AllowTrash:     true,
 			Replication:    1,
 			StorageClasses: map[string]bool{"default": true},
 			UUID:           "zzzzz-mount-00000000000000d",
diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 4fb469bdcb..56a52c913a 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -480,10 +480,9 @@ func (v *AzureBlobVolume) listBlobs(page int, params storage.ListBlobsParameters
 
 // Trash a Keep block.
 func (v *AzureBlobVolume) Trash(loc string) error {
-	if v.volume.ReadOnly {
+	if v.volume.ReadOnly && !v.volume.AllowTrashWhenReadOnly {
 		return MethodDisabledError
 	}
-
 	// Ideally we would use If-Unmodified-Since, but that
 	// particular condition seems to be ignored by Azure. Instead,
 	// we get the Etag before checking Mtime, and use If-Match to
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index 9761680fb5..db387f494b 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -211,7 +211,7 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
 	if d := h.Cluster.Collections.BlobTrashCheckInterval.Duration(); d > 0 &&
 		h.Cluster.Collections.BlobTrash &&
 		h.Cluster.Collections.BlobDeleteConcurrency > 0 {
-		go emptyTrash(h.volmgr.writables, d)
+		go emptyTrash(h.volmgr.mounts, d)
 	}
 
 	return nil
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 60fdde89c7..abeb20fe86 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -475,8 +475,10 @@ func (rtr *router) handleDELETE(resp http.ResponseWriter, req *http.Request) {
 		Deleted int `json:"copies_deleted"`
 		Failed  int `json:"copies_failed"`
 	}
-	for _, vol := range rtr.volmgr.AllWritable() {
-		if err := vol.Trash(hash); err == nil {
+	for _, vol := range rtr.volmgr.Mounts() {
+		if !vol.KeepMount.AllowTrash {
+			continue
+		} else if err := vol.Trash(hash); err == nil {
 			result.Deleted++
 		} else if os.IsNotExist(err) {
 			continue
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index f83ba603d2..953aa047cb 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -49,7 +49,9 @@ func (e *KeepError) Error() string {
 func emptyTrash(mounts []*VolumeMount, interval time.Duration) {
 	for range time.NewTicker(interval).C {
 		for _, v := range mounts {
-			v.EmptyTrash()
+			if v.KeepMount.AllowTrash {
+				v.EmptyTrash()
+			}
 		}
 	}
 }
diff --git a/services/keepstore/s3aws_volume.go b/services/keepstore/s3aws_volume.go
index aaec02721b..18b30f4638 100644
--- a/services/keepstore/s3aws_volume.go
+++ b/services/keepstore/s3aws_volume.go
@@ -846,7 +846,7 @@ func (b *s3AWSbucket) Del(path string) error {
 
 // Trash a Keep block.
 func (v *S3AWSVolume) Trash(loc string) error {
-	if v.volume.ReadOnly {
+	if v.volume.ReadOnly && !v.volume.AllowTrashWhenReadOnly {
 		return MethodDisabledError
 	}
 	if t, err := v.Mtime(loc); err != nil {
diff --git a/services/keepstore/trash_worker.go b/services/keepstore/trash_worker.go
index d7c73127eb..5e8a5a963c 100644
--- a/services/keepstore/trash_worker.go
+++ b/services/keepstore/trash_worker.go
@@ -36,10 +36,12 @@ func TrashItem(volmgr *RRVolumeManager, logger logrus.FieldLogger, cluster *arva
 
 	var volumes []*VolumeMount
 	if uuid := trashRequest.MountUUID; uuid == "" {
-		volumes = volmgr.AllWritable()
-	} else if mnt := volmgr.Lookup(uuid, true); mnt == nil {
+		volumes = volmgr.Mounts()
+	} else if mnt := volmgr.Lookup(uuid, false); mnt == nil {
 		logger.Warnf("trash request for nonexistent mount: %v", trashRequest)
 		return
+	} else if !mnt.KeepMount.AllowTrash {
+		logger.Warnf("trash request for mount with ReadOnly=true, AllowTrashWhenReadOnly=false: %v", trashRequest)
 	} else {
 		volumes = []*VolumeMount{mnt}
 	}
diff --git a/services/keepstore/unix_volume.go b/services/keepstore/unix_volume.go
index 8c935dcddb..dee4bdc1c1 100644
--- a/services/keepstore/unix_volume.go
+++ b/services/keepstore/unix_volume.go
@@ -442,8 +442,7 @@ func (v *UnixVolume) Trash(loc string) error {
 	// be re-written), or (b) Touch() will update the file's timestamp and
 	// Trash() will read the correct up-to-date timestamp and choose not to
 	// trash the file.
-
-	if v.volume.ReadOnly || !v.cluster.Collections.BlobTrash {
+	if v.volume.ReadOnly && !v.volume.AllowTrashWhenReadOnly {
 		return MethodDisabledError
 	}
 	if err := v.lock(context.TODO()); err != nil {
diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index c3b8cd6283..f597ff5781 100644
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -316,8 +316,6 @@ func makeRRVolumeManager(logger logrus.FieldLogger, cluster *arvados.Cluster, my
 		if err != nil {
 			return nil, fmt.Errorf("error initializing volume %s: %s", uuid, err)
 		}
-		logger.Printf("started volume %s (%s), ReadOnly=%v", uuid, vol, cfgvol.ReadOnly || va.ReadOnly)
-
 		sc := cfgvol.StorageClasses
 		if len(sc) == 0 {
 			sc = map[string]bool{"default": true}
@@ -330,7 +328,8 @@ func makeRRVolumeManager(logger logrus.FieldLogger, cluster *arvados.Cluster, my
 			KeepMount: arvados.KeepMount{
 				UUID:           uuid,
 				DeviceID:       vol.GetDeviceID(),
-				ReadOnly:       cfgvol.ReadOnly || va.ReadOnly,
+				AllowWrite:     !va.ReadOnly && !cfgvol.ReadOnly,
+				AllowTrash:     !va.ReadOnly && (!cfgvol.ReadOnly || cfgvol.AllowTrashWhenReadOnly),
 				Replication:    repl,
 				StorageClasses: sc,
 			},
@@ -340,9 +339,10 @@ func makeRRVolumeManager(logger logrus.FieldLogger, cluster *arvados.Cluster, my
 		vm.mounts = append(vm.mounts, mnt)
 		vm.mountMap[uuid] = mnt
 		vm.readables = append(vm.readables, mnt)
-		if !mnt.KeepMount.ReadOnly {
+		if mnt.KeepMount.AllowWrite {
 			vm.writables = append(vm.writables, mnt)
 		}
+		logger.Printf("started volume %s (%s), AllowWrite=%v, AllowTrash=%v", uuid, vol, mnt.AllowWrite, mnt.AllowTrash)
 	}
 	// pri(mnt): return highest priority of any storage class
 	// offered by mnt
@@ -382,7 +382,7 @@ func (vm *RRVolumeManager) Mounts() []*VolumeMount {
 }
 
 func (vm *RRVolumeManager) Lookup(uuid string, needWrite bool) *VolumeMount {
-	if mnt, ok := vm.mountMap[uuid]; ok && (!needWrite || !mnt.ReadOnly) {
+	if mnt, ok := vm.mountMap[uuid]; ok && (!needWrite || mnt.AllowWrite) {
 		return mnt
 	}
 	return nil

commit 01c6fe633e9e535dc95b62114744033acaedb4ee
Author: Tom Clegg <tom at curii.com>
Date:   Thu Oct 26 16:09:21 2023 -0400

    21126: Disable trash if BlobTrashConcurrency: 0, as documented.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index 9d220022ab..9761680fb5 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -186,7 +186,7 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
 
 	// Initialize the trashq and workers
 	h.trashq = NewWorkQueue()
-	for i := 0; i < 1 || i < h.Cluster.Collections.BlobTrashConcurrency; i++ {
+	for i := 0; i < h.Cluster.Collections.BlobTrashConcurrency; i++ {
 		go RunTrashWorker(h.volmgr, h.Logger, h.Cluster, h.trashq)
 	}
 

commit 818611f4561ec927935f2cc1e71c767800899c8a
Author: Tom Clegg <tom at curii.com>
Date:   Thu Oct 26 16:08:36 2023 -0400

    21126: Move BlobDeleteConcurrency==0 check up in the stack.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index f9b383e70e..4fb469bdcb 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -575,10 +575,6 @@ func (v *AzureBlobVolume) isKeepBlock(s string) bool {
 // EmptyTrash looks for trashed blocks that exceeded BlobTrashLifetime
 // and deletes them from the volume.
 func (v *AzureBlobVolume) EmptyTrash() {
-	if v.cluster.Collections.BlobDeleteConcurrency < 1 {
-		return
-	}
-
 	var bytesDeleted, bytesInTrash int64
 	var blocksDeleted, blocksInTrash int64
 
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index 555f16dfe1..9d220022ab 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -208,7 +208,9 @@ func (h *handler) setup(ctx context.Context, cluster *arvados.Cluster, token str
 	}
 	h.keepClient.Arvados.ApiToken = fmt.Sprintf("%x", rand.Int63())
 
-	if d := h.Cluster.Collections.BlobTrashCheckInterval.Duration(); d > 0 {
+	if d := h.Cluster.Collections.BlobTrashCheckInterval.Duration(); d > 0 &&
+		h.Cluster.Collections.BlobTrash &&
+		h.Cluster.Collections.BlobDeleteConcurrency > 0 {
 		go emptyTrash(h.volmgr.writables, d)
 	}
 
diff --git a/services/keepstore/s3aws_volume.go b/services/keepstore/s3aws_volume.go
index 8f2c275391..aaec02721b 100644
--- a/services/keepstore/s3aws_volume.go
+++ b/services/keepstore/s3aws_volume.go
@@ -295,10 +295,6 @@ func (v *S3AWSVolume) Compare(ctx context.Context, loc string, expect []byte) er
 // EmptyTrash looks for trashed blocks that exceeded BlobTrashLifetime
 // and deletes them from the volume.
 func (v *S3AWSVolume) EmptyTrash() {
-	if v.cluster.Collections.BlobDeleteConcurrency < 1 {
-		return
-	}
-
 	var bytesInTrash, blocksInTrash, bytesDeleted, blocksDeleted int64
 
 	// Define "ready to delete" as "...when EmptyTrash started".
diff --git a/services/keepstore/unix_volume.go b/services/keepstore/unix_volume.go
index a08b7de01a..8c935dcddb 100644
--- a/services/keepstore/unix_volume.go
+++ b/services/keepstore/unix_volume.go
@@ -647,10 +647,6 @@ var unixTrashLocRegexp = regexp.MustCompile(`/([0-9a-f]{32})\.trash\.(\d+)$`)
 // EmptyTrash walks hierarchy looking for {hash}.trash.*
 // and deletes those with deadline < now.
 func (v *UnixVolume) EmptyTrash() {
-	if v.cluster.Collections.BlobDeleteConcurrency < 1 {
-		return
-	}
-
 	var bytesDeleted, bytesInTrash int64
 	var blocksDeleted, blocksInTrash int64
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list