[ARVADOS] updated: 28aa91f035205ede87ce62594ebacc36df50a84e

Git user git at public.curoverse.com
Thu Jul 7 14:32:28 EDT 2016


Summary of changes:
 build/run-build-packages.sh               |  3 ++-
 sdk/cwl/setup.py                          |  3 ++-
 services/keep-balance/balance.go          | 38 +++++++++++++++++++++++++++----
 services/keep-balance/balance_run_test.go | 19 ++++++++--------
 services/keep-balance/integration_test.go |  4 +++-
 services/keep-balance/main.go             | 12 ++++++++--
 6 files changed, 60 insertions(+), 19 deletions(-)

       via  28aa91f035205ede87ce62594ebacc36df50a84e (commit)
       via  c07314d641177cfcecf4321a7b7e6771702f5916 (commit)
       via  ecda32d08a4f6d80a2f02ae305fdb43e141672ce (commit)
      from  edbd43f2b89a915cf33b963c1e1ae86447c1e93b (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 28aa91f035205ede87ce62594ebacc36df50a84e
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 7 14:32:03 2016 -0400

    9567: Avoid ruamel.yaml >0.11.11. refs #9567

diff --git a/build/run-build-packages.sh b/build/run-build-packages.sh
index 783d3d6..f2894e3 100755
--- a/build/run-build-packages.sh
+++ b/build/run-build-packages.sh
@@ -463,7 +463,8 @@ fpm_build schema_salad "" "" python 1.12.20160610104117
 
 # And schema_salad now depends on ruamel-yaml, which apparently has a braindead setup.py that requires special arguments to build (otherwise, it aborts with 'error: you have to install with "pip install ."'). Sigh.
 # Ward, 2016-05-26
-fpm_build ruamel.yaml "" "" python "" --python-setup-py-arguments "--single-version-externally-managed"
+# ...and schema_salad 1.12.20160610104117 doesn't work with ruamel-yaml > 0.11.11.
+fpm_build ruamel.yaml "" "" python 0.11.11 --python-setup-py-arguments "--single-version-externally-managed"
 
 # And for cwltool we have the same problem as for schema_salad. Ward, 2016-03-17
 fpm_build cwltool "" "" python 1.0.20160630171631
diff --git a/sdk/cwl/setup.py b/sdk/cwl/setup.py
index 131350d..4e72091 100644
--- a/sdk/cwl/setup.py
+++ b/sdk/cwl/setup.py
@@ -31,7 +31,8 @@ setup(name='arvados-cwl-runner',
       ],
       install_requires=[
           'cwltool==1.0.20160630171631',
-          'arvados-python-client>=0.1.20160322001610'
+          'arvados-python-client>=0.1.20160322001610',
+          'ruamel.yaml==0.11.11', # this should be declared by schema_salad instead, but see #9567
       ],
       data_files=[
           ('share/doc/arvados-cwl-runner', ['LICENSE-2.0.txt', 'README.rst']),

commit c07314d641177cfcecf4321a7b7e6771702f5916
Merge: edbd43f ecda32d
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 7 11:43:35 2016 -0400

    Merge branch '9456-less-clear-trash'
    
    closes #9456


commit ecda32d08a4f6d80a2f02ae305fdb43e141672ce
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jul 7 11:43:17 2016 -0400

    9456: Do not clear trash lists between runs when the list of keep services has not changed.

diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index d22074e..25b474b 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -6,6 +6,7 @@ import (
 	"math"
 	"os"
 	"runtime"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -50,11 +51,17 @@ type Balancer struct {
 }
 
 // Run performs a balance operation using the given config and
-// runOptions. It should only be called once on a given Balancer
-// object. Typical usage:
+// runOptions, and returns RunOptions suitable for passing to a
+// subsequent balance operation.
 //
-//   err = (&Balancer{}).Run(config, runOptions)
-func (bal *Balancer) Run(config Config, runOptions RunOptions) (err error) {
+// Run should only be called once on a given Balancer object.
+//
+// Typical usage:
+//
+//   runOptions, err = (&Balancer{}).Run(config, runOptions)
+func (bal *Balancer) Run(config Config, runOptions RunOptions) (nextRunOptions RunOptions, err error) {
+	nextRunOptions = runOptions
+
 	bal.Dumper = runOptions.Dumper
 	bal.Logger = runOptions.Logger
 	if bal.Logger == nil {
@@ -75,10 +82,20 @@ func (bal *Balancer) Run(config Config, runOptions RunOptions) (err error) {
 	if err = bal.CheckSanityEarly(&config.Client); err != nil {
 		return
 	}
-	if runOptions.CommitTrash {
+	rs := bal.rendezvousState()
+	if runOptions.CommitTrash && rs != runOptions.SafeRendezvousState {
+		if runOptions.SafeRendezvousState != "" {
+			bal.logf("notice: KeepServices list has changed since last run")
+		}
+		bal.logf("clearing existing trash lists, in case the new rendezvous order differs from previous run")
 		if err = bal.ClearTrashLists(&config.Client); err != nil {
 			return
 		}
+		// The current rendezvous state becomes "safe" (i.e.,
+		// OK to compute changes for that state without
+		// clearing existing trash lists) only now, after we
+		// succeed in clearing existing trash lists.
+		nextRunOptions.SafeRendezvousState = rs
 	}
 	if err = bal.GetCurrentState(&config.Client, config.CollectionBatchSize, config.CollectionBuffers); err != nil {
 		return
@@ -158,6 +175,17 @@ func (bal *Balancer) CheckSanityEarly(c *arvados.Client) error {
 	return nil
 }
 
+// rendezvousState returns a fingerprint (e.g., a sorted list of
+// UUID+host+port) of the current set of keep services.
+func (bal *Balancer) rendezvousState() string {
+	srvs := make([]string, 0, len(bal.KeepServices))
+	for _, srv := range bal.KeepServices {
+		srvs = append(srvs, srv.String())
+	}
+	sort.Strings(srvs)
+	return strings.Join(srvs, "; ")
+}
+
 // ClearTrashLists sends an empty trash list to each keep
 // service. Calling this before GetCurrentState avoids races.
 //
diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go
index a138d91..edc88aa 100644
--- a/services/keep-balance/balance_run_test.go
+++ b/services/keep-balance/balance_run_test.go
@@ -236,7 +236,7 @@ func (s *runSuite) TestRefuseZeroCollections(c *check.C) {
 	s.stub.serveKeepstoreIndexFoo4Bar1()
 	trashReqs := s.stub.serveKeepstoreTrash()
 	pullReqs := s.stub.serveKeepstorePull()
-	err := (&Balancer{}).Run(s.config, opts)
+	_, err := (&Balancer{}).Run(s.config, opts)
 	c.Check(err, check.ErrorMatches, "received zero collections")
 	c.Check(trashReqs.Count(), check.Equals, 4)
 	c.Check(pullReqs.Count(), check.Equals, 0)
@@ -254,7 +254,7 @@ func (s *runSuite) TestServiceTypes(c *check.C) {
 	s.stub.serveFourDiskKeepServices()
 	indexReqs := s.stub.serveKeepstoreIndexFoo4Bar1()
 	trashReqs := s.stub.serveKeepstoreTrash()
-	err := (&Balancer{}).Run(s.config, opts)
+	_, err := (&Balancer{}).Run(s.config, opts)
 	c.Check(err, check.IsNil)
 	c.Check(indexReqs.Count(), check.Equals, 0)
 	c.Check(trashReqs.Count(), check.Equals, 0)
@@ -271,7 +271,7 @@ func (s *runSuite) TestRefuseNonAdmin(c *check.C) {
 	s.stub.serveFourDiskKeepServices()
 	trashReqs := s.stub.serveKeepstoreTrash()
 	pullReqs := s.stub.serveKeepstorePull()
-	err := (&Balancer{}).Run(s.config, opts)
+	_, err := (&Balancer{}).Run(s.config, opts)
 	c.Check(err, check.ErrorMatches, "current user .* is not .* admin user")
 	c.Check(trashReqs.Count(), check.Equals, 0)
 	c.Check(pullReqs.Count(), check.Equals, 0)
@@ -289,7 +289,7 @@ func (s *runSuite) TestDetectSkippedCollections(c *check.C) {
 	s.stub.serveKeepstoreIndexFoo4Bar1()
 	trashReqs := s.stub.serveKeepstoreTrash()
 	pullReqs := s.stub.serveKeepstorePull()
-	err := (&Balancer{}).Run(s.config, opts)
+	_, err := (&Balancer{}).Run(s.config, opts)
 	c.Check(err, check.ErrorMatches, `Retrieved 2 collections with modtime <= .* but server now reports there are 3 collections.*`)
 	c.Check(trashReqs.Count(), check.Equals, 4)
 	c.Check(pullReqs.Count(), check.Equals, 0)
@@ -308,7 +308,7 @@ func (s *runSuite) TestDryRun(c *check.C) {
 	trashReqs := s.stub.serveKeepstoreTrash()
 	pullReqs := s.stub.serveKeepstorePull()
 	var bal Balancer
-	err := bal.Run(s.config, opts)
+	_, err := bal.Run(s.config, opts)
 	c.Check(err, check.IsNil)
 	c.Check(trashReqs.Count(), check.Equals, 0)
 	c.Check(pullReqs.Count(), check.Equals, 0)
@@ -332,7 +332,7 @@ func (s *runSuite) TestCommit(c *check.C) {
 	trashReqs := s.stub.serveKeepstoreTrash()
 	pullReqs := s.stub.serveKeepstorePull()
 	var bal Balancer
-	err := bal.Run(s.config, opts)
+	_, err := bal.Run(s.config, opts)
 	c.Check(err, check.IsNil)
 	c.Check(trashReqs.Count(), check.Equals, 8)
 	c.Check(pullReqs.Count(), check.Equals, 4)
@@ -362,13 +362,14 @@ func (s *runSuite) TestRunForever(c *check.C) {
 	s.config.RunPeriod = arvados.Duration(time.Millisecond)
 	go RunForever(s.config, opts, stop)
 
-	// Each run should send 4 clear trash lists + 4 pull lists + 4
-	// trash lists. We should complete four runs in much less than
+	// Each run should send 4 pull lists + 4 trash lists. The
+	// first run should also send 4 empty trash lists at
+	// startup. We should complete all four runs in much less than
 	// a second.
 	for t0 := time.Now(); pullReqs.Count() < 16 && time.Since(t0) < 10*time.Second; {
 		time.Sleep(time.Millisecond)
 	}
 	stop <- true
 	c.Check(pullReqs.Count() >= 16, check.Equals, true)
-	c.Check(trashReqs.Count(), check.Equals, 2*pullReqs.Count())
+	c.Check(trashReqs.Count(), check.Equals, pullReqs.Count() + 4)
 }
diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go
index b090614..0793889 100644
--- a/services/keep-balance/integration_test.go
+++ b/services/keep-balance/integration_test.go
@@ -78,8 +78,10 @@ func (s *integrationSuite) TestBalanceAPIFixtures(c *check.C) {
 			CommitTrash: true,
 			Logger:      log.New(logBuf, "", log.LstdFlags),
 		}
-		err := (&Balancer{}).Run(s.config, opts)
+		nextOpts, err := (&Balancer{}).Run(s.config, opts)
 		c.Check(err, check.IsNil)
+		c.Check(nextOpts.SafeRendezvousState, check.Not(check.Equals), "")
+		c.Check(nextOpts.CommitPulls, check.Equals, true)
 		if iter == 0 {
 			c.Check(logBuf.String(), check.Matches, `(?ms).*ChangeSet{Pulls:1.*`)
 			c.Check(logBuf.String(), check.Not(check.Matches), `(?ms).*ChangeSet{.*Trashes:[^0]}*`)
diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index 364bb3f..da4fb62 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -51,6 +51,12 @@ type RunOptions struct {
 	CommitTrash bool
 	Logger      *log.Logger
 	Dumper      *log.Logger
+
+	// SafeRendezvousState from the most recent balance operation,
+	// or "" if unknown. If this changes from one run to the next,
+	// we need to watch out for races. See
+	// (*Balancer)ClearTrashLists.
+	SafeRendezvousState string
 }
 
 var debugf = func(string, ...interface{}) {}
@@ -98,7 +104,7 @@ func main() {
 	if err != nil {
 		// (don't run)
 	} else if runOptions.Once {
-		err = (&Balancer{}).Run(config, runOptions)
+		_, err = (&Balancer{}).Run(config, runOptions)
 	} else {
 		err = RunForever(config, runOptions, nil)
 	}
@@ -138,7 +144,9 @@ func RunForever(config Config, runOptions RunOptions, stop <-chan interface{}) e
 			logger.Print("=======  Consider using -commit-pulls and -commit-trash flags.")
 		}
 
-		err := (&Balancer{}).Run(config, runOptions)
+		bal := &Balancer{}
+		var err error
+		runOptions, err = bal.Run(config, runOptions)
 		if err != nil {
 			logger.Print("run failed: ", err)
 		} else {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list