13427: Fix replication stats reporting for multiple-mounted devices.

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 5aad8e635cb15d95971a6d8473e93e13c519050a..328e6234221f157f8ac18be525b5460d3a0ada93 100644 (file)
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -528,10 +528,14 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
        for _, class := range bal.classes {
                desired := blk.Desired[class]
 
+               countedDev := map[string]bool{}
                have := 0
                for _, slot := range slots {
-                       if slot.repl != nil && bal.mountsByClass[class][slot.mnt] {
+                       if slot.repl != nil && bal.mountsByClass[class][slot.mnt] && !countedDev[slot.mnt.DeviceID] {
                                have++
+                               if slot.mnt.DeviceID != "" {
+                                       countedDev[slot.mnt.DeviceID] = true
+                               }
                        }
                }
                classState[class] = balancedBlockState{
@@ -670,13 +674,17 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
        // replica that doesn't have a timestamp collision with
        // others.
 
+       countedDev := map[string]bool{}
        var have, want int
        for _, slot := range slots {
                if slot.want {
                        want++
                }
-               if slot.repl != nil {
+               if slot.repl != nil && !countedDev[slot.mnt.DeviceID] {
                        have++
+                       if slot.mnt.DeviceID != "" {
+                               countedDev[slot.mnt.DeviceID] = true
+                       }
                }
        }
 

diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index 602c42e840bd9d93bb02e00878686bd727d4c919..8650de141cd245239754e98c474e9783acf6e8f2 100644 (file)
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -49,6 +49,8 @@ type tester struct {
 
        shouldPullMounts  []string
        shouldTrashMounts []string
+
+       expectResult balanceResult
 }
 
 func (bal *balancerSuite) SetUpSuite(c *check.C) {
@@ -292,14 +294,26 @@ func (bal *balancerSuite) TestDeviceRWMountedByMultipleServers(c *check.C) {
                known:      0,
                desired:    map[string]int{"default": 2},
                current:    slots{1, 9},
-               shouldPull: slots{0}})
+               shouldPull: slots{0},
+               expectResult: balanceResult{
+                       have: 1,
+                       classState: map[string]balancedBlockState{"default": {
+                               desired:      2,
+                               surplus:      -1,
+                               unachievable: false}}}})
        // block 0 is overreplicated, but the second and third
        // replicas are the same replica according to DeviceID
        // (despite different Mtimes). Don't trash the third replica.
        bal.try(c, tester{
                known:   0,
                desired: map[string]int{"default": 2},
-               current: slots{0, 1, 9}})
+               current: slots{0, 1, 9},
+               expectResult: balanceResult{
+                       have: 2,
+                       classState: map[string]balancedBlockState{"default": {
+                               desired:      2,
+                               surplus:      0,
+                               unachievable: false}}}})
        // block 0 is overreplicated; the third and fifth replicas are
        // extra, but the fourth is another view of the second and
        // shouldn't be trashed.
@@ -307,7 +321,13 @@ func (bal *balancerSuite) TestDeviceRWMountedByMultipleServers(c *check.C) {
                known:       0,
                desired:     map[string]int{"default": 2},
                current:     slots{0, 1, 5, 9, 12},
-               shouldTrash: slots{5, 12}})
+               shouldTrash: slots{5, 12},
+               expectResult: balanceResult{
+                       have: 4,
+                       classState: map[string]balancedBlockState{"default": {
+                               desired:      2,
+                               surplus:      2,
+                               unachievable: false}}}})
 }
 
 func (bal *balancerSuite) TestChangeStorageClasses(c *check.C) {
@@ -438,7 +458,7 @@ func (bal *balancerSuite) try(c *check.C, t tester) {
        for _, srv := range bal.srvs {
                srv.ChangeSet = &ChangeSet{}
        }
-       bal.balanceBlock(knownBlkid(t.known), blk)
+       result := bal.balanceBlock(knownBlkid(t.known), blk)
 
        var didPull, didTrash slots
        var didPullMounts, didTrashMounts []string
@@ -474,6 +494,12 @@ func (bal *balancerSuite) try(c *check.C, t tester) {
                sort.Strings(didTrashMounts)
                c.Check(didTrashMounts, check.DeepEquals, t.shouldTrashMounts)
        }
+       if t.expectResult.have > 0 {
+               c.Check(result.have, check.Equals, t.expectResult.have)
+       }
+       if t.expectResult.classState != nil {
+               c.Check(result.classState, check.DeepEquals, t.expectResult.classState)
+       }
 }
 
 // srvList returns the KeepServices, sorted in rendezvous order and