14388: Don't prefer positions that are only temporarily untrashable.

If old copies exist in the preferred positions, and a new copy is
written to a worse position, keep the old copies and wait for the new
copy to become eligible for trash.

Previously, an old copy would be deleted. This would bring replication
down to the desired level in the short term, but would also cause
extra pulling/copying in the future to restore the optimal copy, and
introduce extra probes for all clients in the meantime.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 6bfd17573709b82adb7231b81dfdd340a599914c..7683835dd9c3c73e675c9aca347c5a42cd878c7a 100644 (file)
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -520,7 +520,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
                        slots = append(slots, slot{
                                mnt:  mnt,
                                repl: repl,
-                               want: repl != nil && (mnt.ReadOnly || repl.Mtime >= bal.MinMtime),
+                               want: repl != nil && mnt.ReadOnly,
                        })
                }
        }
@@ -716,7 +716,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
                // TODO: request a Touch if Mtime is duplicated.
                var change int
                switch {
-               case !underreplicated && slot.repl != nil && !slot.want && !unsafeToDelete[slot.repl.Mtime]:
+               case !underreplicated && !slot.want && slot.repl != nil && slot.repl.Mtime < bal.MinMtime && !unsafeToDelete[slot.repl.Mtime]:
                        slot.mnt.KeepService.AddTrash(Trash{
                                SizedDigest: blkid,
                                Mtime:       slot.repl.Mtime,

diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index 2e664bedfb19fe8054d39083e6ee4f5cf6e477c6..37be185dcc1af9fbc4ebf9f83cd8302ca15b8e6d 100644 (file)
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -238,7 +238,14 @@ func (bal *balancerSuite) TestDecreaseReplBlockTooNew(c *check.C) {
        bal.try(c, tester{
                desired:    map[string]int{"default": 2},
                current:    slots{0, 1, 2},
-               timestamps: []int64{oldTime, newTime, newTime + 1}})
+               timestamps: []int64{oldTime, newTime, newTime + 1},
+               expectResult: balanceResult{
+                       have: 3,
+                       want: 2,
+                       classState: map[string]balancedBlockState{"default": {
+                               desired:      2,
+                               surplus:      1,
+                               unachievable: false}}}})
        // The best replicas are too new to delete, but the excess
        // replica is old enough.
        bal.try(c, tester{