[ARVADOS] updated: 1.3.0-776-g0210746ac

Git user git at public.curoverse.com
Mon Apr 22 14:58:25 UTC 2019


Summary of changes:
 services/api/test/unit/container_request_test.rb |  1 +
 services/keep-balance/balance.go                 | 41 +++++++++-----
 services/keep-balance/balance_run_test.go        |  2 +
 services/keep-balance/balance_test.go            | 72 +++++++++++++++++++++++-
 services/keep-balance/collection.go              | 12 ++++
 services/keep-balance/integration_test.go        |  6 +-
 services/keep-balance/main.go                    |  7 ++-
 services/keep-balance/server.go                  |  8 +--
 8 files changed, 125 insertions(+), 24 deletions(-)

       via  0210746ac1fef5a588279b208313ed06e51b19de (commit)
       via  62b9c297c3336b2bc605f70a756c56b69285e730 (commit)
       via  0ceef0fb54f4ce953393a9091a55698521b4b714 (commit)
       via  b9ba2b1441d16f0bbdd77598636e57282d62bbf7 (commit)
       via  44666b0c9e670858cc9959a5fb7e7345f27cdb7a (commit)
       via  e18ef10d5dac6b5a051cbc45fcf1c2b7f8054759 (commit)
       via  a9ccdf931e0655b47ee4b40cf235aaaf1c6fd27a (commit)
       via  ed030abf02acfb2ef7a826f27bc44f4310f497f6 (commit)
       via  aae2802ab7e3ecb35ff86998e79927fe7de26e49 (commit)
       via  bb14c34e321ee553a9279408320ab382f586cd59 (commit)
       via  6c4396152d590afc2f5fa826b9c6a8527f156452 (commit)
       via  ab5d1b7a7d6a144a7daa3051431c9c59ba0ac339 (commit)
       via  8c07074bc91e805e1e98771d7b71e0ecf7f9b896 (commit)
      from  556da32e121396cb5f332f44c028df6fa169957c (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 0210746ac1fef5a588279b208313ed06e51b19de
Merge: 556da32e1 62b9c297c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Apr 22 10:55:28 2019 -0400

    Merge branch '15112-dont-trash-needed-replica'
    
    refs #15112
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>


commit 62b9c297c3336b2bc605f70a756c56b69285e730
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Apr 22 10:50:59 2019 -0400

    15112: Note rationale for non-obvious paging approach.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/collection.go b/services/keep-balance/collection.go
index 1e5fa5797..534928bc8 100644
--- a/services/keep-balance/collection.go
+++ b/services/keep-balance/collection.go
@@ -43,6 +43,18 @@ func EachCollection(c *arvados.Client, pageSize int, f func(arvados.Collection)
 		return err
 	}
 
+	// Note the obvious way to get all collections (sorting by
+	// UUID) would be much easier, but would lose data: If a
+	// client were to move files from collection with uuid="zzz"
+	// to a collection with uuid="aaa" around the time when we
+	// were fetching the "mmm" page, we would never see those
+	// files' block IDs at all -- even if the client is careful to
+	// save "aaa" before saving "zzz".
+	//
+	// Instead, we get pages in modified_at order. Collections
+	// that are modified during the run will be re-fetched in a
+	// subsequent page.
+
 	limit := pageSize
 	if limit <= 0 {
 		// Use the maximum page size the server allows

commit 0ceef0fb54f4ce953393a9091a55698521b4b714
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Apr 22 09:30:29 2019 -0400

    15112: Add test.
    
    This tests for a bug that was already fixed in #14595, but without
    noticing.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 5174e6cf4..0dad6ee75 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -252,6 +252,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     output = Collection.find_by_uuid cr.output_uuid
     assert_equal output_pdh, output.portable_data_hash
     assert_equal output.owner_uuid, project.uuid, "Container output should be copied to #{project.uuid}"
+    assert_not_nil output.modified_at
 
     log = Collection.find_by_uuid cr.log_uuid
     assert_equal log.manifest_text, ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar

commit b9ba2b1441d16f0bbdd77598636e57282d62bbf7
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Apr 22 03:06:18 2019 -0400

    15112: Refuse to run if modified_at column has any nulls.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index eff99b733..fd39ee693 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -206,6 +206,24 @@ func (bal *Balancer) CheckSanityEarly(c *arvados.Client) error {
 			return fmt.Errorf("config error: %s: proxy servers cannot be balanced", srv)
 		}
 	}
+
+	var checkPage arvados.CollectionList
+	if err = c.RequestAndDecode(&checkPage, "GET", "arvados/v1/collections", nil, arvados.ResourceListParams{
+		Limit:              new(int),
+		Count:              "exact",
+		IncludeTrash:       true,
+		IncludeOldVersions: true,
+		Filters: []arvados.Filter{{
+			Attr:     "modified_at",
+			Operator: "=",
+			Operand:  nil,
+		}},
+	}); err != nil {
+		return err
+	} else if n := checkPage.ItemsAvailable; n > 0 {
+		return fmt.Errorf("%d collections exist with null modified_at; cannot fetch reliably", n)
+	}
+
 	return nil
 }
 
diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go
index 7e2adcfed..3b7e2db9a 100644
--- a/services/keep-balance/balance_run_test.go
+++ b/services/keep-balance/balance_run_test.go
@@ -203,6 +203,8 @@ func (s *stubServer) serveCollectionsButSkipOne() *reqTracker {
 			io.WriteString(w, `{"items_available":0,"items":[]}`)
 		} else if strings.Contains(r.Form.Get("filters"), `"modified_at","="`) && strings.Contains(r.Form.Get("filters"), `"uuid","\u003e"`) {
 			io.WriteString(w, `{"items_available":0,"items":[]}`)
+		} else if strings.Contains(r.Form.Get("filters"), `"modified_at","=",null`) {
+			io.WriteString(w, `{"items_available":0,"items":[]}`)
 		} else {
 			io.WriteString(w, `{"items_available":2,"items":[
 				{"uuid":"zzzzz-4zz18-ehbhgtheo8909or","portable_data_hash":"fa7aeb5140e2848d39b416daeef4ffc5+45","manifest_text":". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n","modified_at":"2014-02-03T17:22:54Z"},

commit 44666b0c9e670858cc9959a5fb7e7345f27cdb7a
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Apr 21 20:25:28 2019 -0400

    15112: Stop processing collections on first error.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index ab500d2c6..eff99b733 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -332,7 +332,7 @@ func (bal *Balancer) GetCurrentState(c *arvados.Client, pageSize, bufs int) erro
 		defer wg.Done()
 		for coll := range collQ {
 			err := bal.addCollection(coll)
-			if err != nil {
+			if err != nil || len(errs) > 0 {
 				select {
 				case errs <- err:
 				default:

commit e18ef10d5dac6b5a051cbc45fcf1c2b7f8054759
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Apr 21 20:24:40 2019 -0400

    15112: Fix duplicate default.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 8031ff214..ab500d2c6 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -441,7 +441,7 @@ func (bal *Balancer) ComputeChangeSets() {
 
 func (bal *Balancer) setupLookupTables() {
 	bal.serviceRoots = make(map[string]string)
-	bal.classes = []string{"default"}
+	bal.classes = defaultClasses
 	bal.mountsByClass = map[string]map[*KeepMount]bool{"default": {}}
 	bal.mounts = 0
 	for _, srv := range bal.KeepServices {

commit a9ccdf931e0655b47ee4b40cf235aaaf1c6fd27a
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Apr 21 20:24:12 2019 -0400

    15112: Error out right away on unusable manifest.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 346de30b6..8031ff214 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -383,10 +383,7 @@ func (bal *Balancer) GetCurrentState(c *arvados.Client, pageSize, bufs int) erro
 func (bal *Balancer) addCollection(coll arvados.Collection) error {
 	blkids, err := coll.SizedDigests()
 	if err != nil {
-		bal.mutex.Lock()
-		bal.errors = append(bal.errors, fmt.Errorf("%v: %v", coll.UUID, err))
-		bal.mutex.Unlock()
-		return nil
+		return fmt.Errorf("%v: %v", coll.UUID, err)
 	}
 	repl := bal.DefaultReplication
 	if coll.ReplicationDesired != nil {

commit ed030abf02acfb2ef7a826f27bc44f4310f497f6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Apr 21 20:23:16 2019 -0400

    15112: Fix log check in test case.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go
index 8f5d08a19..a79779c7d 100644
--- a/services/keep-balance/integration_test.go
+++ b/services/keep-balance/integration_test.go
@@ -71,11 +71,11 @@ func (s *integrationSuite) SetUpTest(c *check.C) {
 }
 
 func (s *integrationSuite) TestBalanceAPIFixtures(c *check.C) {
-	var logBuf *bytes.Buffer
+	var logBuf bytes.Buffer
 	for iter := 0; iter < 20; iter++ {
-		logBuf := &bytes.Buffer{}
+		logBuf.Reset()
 		logger := logrus.New()
-		logger.Out = logBuf
+		logger.Out = &logBuf
 		opts := RunOptions{
 			CommitPulls: true,
 			CommitTrash: true,

commit aae2802ab7e3ecb35ff86998e79927fe7de26e49
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Apr 21 20:14:16 2019 -0400

    15112: Show #collections and class->#desired map in dump.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index f47a99761..346de30b6 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -754,7 +754,7 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
 		}
 	}
 	if bal.Dumper != nil {
-		bal.Dumper.Printf("%s have=%d want=%v %s", blkid, have, want, strings.Join(changes, " "))
+		bal.Dumper.Printf("%s refs=%d have=%d want=%v %v %v", blkid, blk.RefCount, have, want, blk.Desired, changes)
 	}
 	return balanceResult{
 		blk:        blk,

commit bb14c34e321ee553a9279408320ab382f586cd59
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Apr 20 20:32:54 2019 -0400

    15112: Test multiple volumes with same device IDs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index 1ff18316e..423546c46 100644
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -132,6 +132,75 @@ func (bal *balancerSuite) TestSkipReadonly(c *check.C) {
 		shouldPull: slots{2, 4}})
 }
 
+func (bal *balancerSuite) TestMultipleViewsReadOnly(c *check.C) {
+	bal.testMultipleViews(c, true)
+}
+
+func (bal *balancerSuite) TestMultipleViews(c *check.C) {
+	bal.testMultipleViews(c, false)
+}
+
+func (bal *balancerSuite) testMultipleViews(c *check.C, readonly bool) {
+	for i, srv := range bal.srvs {
+		// Add a mount to each service
+		srv.mounts[0].KeepMount.DeviceID = fmt.Sprintf("writable-by-srv-%x", i)
+		srv.mounts = append(srv.mounts, &KeepMount{
+			KeepMount: arvados.KeepMount{
+				DeviceID:    fmt.Sprintf("writable-by-srv-%x", (i+1)%len(bal.srvs)),
+				UUID:        fmt.Sprintf("zzzzz-mount-%015x", i<<16),
+				ReadOnly:    readonly,
+				Replication: 1,
+			},
+			KeepService: srv,
+		})
+	}
+	for i := 1; i < len(bal.srvs); i++ {
+		c.Logf("i=%d", i)
+		if i == 4 {
+			// Timestamps are all different, but one of
+			// the mounts on srv[4] has the same device ID
+			// where the non-deletable replica is stored
+			// on srv[3], so only one replica is safe to
+			// trash.
+			bal.try(c, tester{
+				desired:     map[string]int{"default": 1},
+				current:     slots{0, i, i},
+				shouldTrash: slots{i}})
+		} else if readonly {
+			// Timestamps are all different, and the third
+			// replica can't be trashed because it's on a
+			// read-only mount, so the first two replicas
+			// should be trashed.
+			bal.try(c, tester{
+				desired:     map[string]int{"default": 1},
+				current:     slots{0, i, i},
+				shouldTrash: slots{0, i}})
+		} else {
+			// Timestamps are all different, so both
+			// replicas on the non-optimal server should
+			// be trashed.
+			bal.try(c, tester{
+				desired:     map[string]int{"default": 1},
+				current:     slots{0, i, i},
+				shouldTrash: slots{i, i}})
+		}
+		// If the three replicas have identical timestamps,
+		// none of them can be trashed safely.
+		bal.try(c, tester{
+			desired:    map[string]int{"default": 1},
+			current:    slots{0, i, i},
+			timestamps: []int64{12345678, 12345678, 12345678}})
+		// If the first and third replicas have identical
+		// timestamps, only the second replica should be
+		// trashed.
+		bal.try(c, tester{
+			desired:     map[string]int{"default": 1},
+			current:     slots{0, i, i},
+			timestamps:  []int64{12345678, 12345679, 12345678},
+			shouldTrash: slots{i}})
+	}
+}
+
 func (bal *balancerSuite) TestFixUnbalanced(c *check.C) {
 	bal.try(c, tester{
 		desired:    map[string]int{"default": 2},

commit 6c4396152d590afc2f5fa826b9c6a8527f156452
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Apr 20 20:29:42 2019 -0400

    15112: Fix mount IDs in test case.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index 37be185dc..1ff18316e 100644
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -162,9 +162,10 @@ func (bal *balancerSuite) TestFixUnbalanced(c *check.C) {
 }
 
 func (bal *balancerSuite) TestMultipleReplicasPerService(c *check.C) {
-	for _, srv := range bal.srvs {
+	for s, srv := range bal.srvs {
 		for i := 0; i < 3; i++ {
 			m := *(srv.mounts[0])
+			m.UUID = fmt.Sprintf("zzzzz-mount-%015x", (s<<10)+i)
 			srv.mounts = append(srv.mounts, &m)
 		}
 	}

commit ab5d1b7a7d6a144a7daa3051431c9c59ba0ac339
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Apr 20 20:26:44 2019 -0400

    15112: Use FieldLogger interface so tests can use ctxlog.TestLogger.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 9e8ca3a67..f47a99761 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -31,8 +31,8 @@ import (
 // BlobSignatureTTL; and all N existing replicas of a given data block
 // are in the N best positions in rendezvous probe order.
 type Balancer struct {
-	Logger  *logrus.Logger
-	Dumper  *logrus.Logger
+	Logger  logrus.FieldLogger
+	Dumper  logrus.FieldLogger
 	Metrics *metrics
 
 	*BlockStateMap
diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index 3316a1724..84516a821 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -76,9 +76,10 @@ func main() {
 		}
 	}
 	if *dumpFlag {
-		runOptions.Dumper = logrus.New()
-		runOptions.Dumper.Out = os.Stdout
-		runOptions.Dumper.Formatter = &logrus.TextFormatter{}
+		dumper := logrus.New()
+		dumper.Out = os.Stdout
+		dumper.Formatter = &logrus.TextFormatter{}
+		runOptions.Dumper = dumper
 	}
 	srv, err := NewServer(cfg, runOptions)
 	if err != nil {
diff --git a/services/keep-balance/server.go b/services/keep-balance/server.go
index 613a2f7d3..c867d7b10 100644
--- a/services/keep-balance/server.go
+++ b/services/keep-balance/server.go
@@ -70,8 +70,8 @@ type RunOptions struct {
 	Once        bool
 	CommitPulls bool
 	CommitTrash bool
-	Logger      *logrus.Logger
-	Dumper      *logrus.Logger
+	Logger      logrus.FieldLogger
+	Dumper      logrus.FieldLogger
 
 	// SafeRendezvousState from the most recent balance operation,
 	// or "" if unknown. If this changes from one run to the next,
@@ -86,8 +86,8 @@ type Server struct {
 	metrics    *metrics
 	listening  string // for tests
 
-	Logger *logrus.Logger
-	Dumper *logrus.Logger
+	Logger logrus.FieldLogger
+	Dumper logrus.FieldLogger
 }
 
 // NewServer returns a new Server that runs Balancers using the given

commit 8c07074bc91e805e1e98771d7b71e0ecf7f9b896
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Apr 20 17:22:41 2019 -0400

    15112: Report volumes without replicas as "none" in -dump output.
    
    ...instead of reporting as "stay" or "none" depending on
    whether *other* volumes have replicas.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 836be2e60..9e8ca3a67 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -732,17 +732,17 @@ func (bal *Balancer) balanceBlock(blkid arvados.SizedDigest, blk *BlockState) ba
 				From:        slot.mnt,
 			})
 			change = changeTrash
-		case len(blk.Replicas) == 0:
-			change = changeNone
-		case slot.repl == nil && slot.want && !slot.mnt.ReadOnly:
+		case len(blk.Replicas) > 0 && slot.repl == nil && slot.want && !slot.mnt.ReadOnly:
 			slot.mnt.KeepService.AddPull(Pull{
 				SizedDigest: blkid,
 				From:        blk.Replicas[0].KeepMount.KeepService,
 				To:          slot.mnt,
 			})
 			change = changePull
-		default:
+		case slot.repl != nil:
 			change = changeStay
+		default:
+			change = changeNone
 		}
 		if bal.Dumper != nil {
 			var mtime int64

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list