[ARVADOS] updated: 1.3.0-2592-gf525d01de

Git user git at public.arvados.org
Thu Jun 4 20:43:27 UTC 2020


Summary of changes:
 lib/config/load.go       |  18 +++++----
 lib/undelete/cmd.go      | 100 +++++++++++++++++++++++++++++++++++------------
 lib/undelete/cmd_test.go |   2 +-
 3 files changed, 85 insertions(+), 35 deletions(-)

       via  f525d01de971b8b06dabc827bc0bbf46ee7ce9cc (commit)
       via  58254d66b4a2c47a7c736c1e7c50203a8cf19805 (commit)
       via  3d80ac6a1ba336ae75fd7afa499d6e0dfd05bff3 (commit)
       via  cff3ee7ddf7caf971bae2850b3a44b9d5142931f (commit)
       via  1c3ce4051e76a877f41485f031c4219d2b732629 (commit)
       via  d9b06eb06c0ac38af922f102da0b8e405bb40f82 (commit)
      from  167e2537365ec68fe6be6a330a2eb698f177aa05 (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 f525d01de971b8b06dabc827bc0bbf46ee7ce9cc
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 16:35:05 2020 -0400

    16427: Don't print legacy config path flags if they won't be used.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/config/load.go b/lib/config/load.go
index 86a8f7df6..be6181bbe 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -64,14 +64,16 @@ func NewLoader(stdin io.Reader, logger logrus.FieldLogger) *Loader {
 //	// ldr.Path == "/tmp/c.yaml"
 func (ldr *Loader) SetupFlags(flagset *flag.FlagSet) {
 	flagset.StringVar(&ldr.Path, "config", arvados.DefaultConfigFile, "Site configuration `file` (default may be overridden by setting an ARVADOS_CONFIG environment variable)")
-	flagset.StringVar(&ldr.KeepstorePath, "legacy-keepstore-config", defaultKeepstoreConfigPath, "Legacy keepstore configuration `file`")
-	flagset.StringVar(&ldr.KeepWebPath, "legacy-keepweb-config", defaultKeepWebConfigPath, "Legacy keep-web configuration `file`")
-	flagset.StringVar(&ldr.CrunchDispatchSlurmPath, "legacy-crunch-dispatch-slurm-config", defaultCrunchDispatchSlurmConfigPath, "Legacy crunch-dispatch-slurm configuration `file`")
-	flagset.StringVar(&ldr.WebsocketPath, "legacy-ws-config", defaultWebsocketConfigPath, "Legacy arvados-ws configuration `file`")
-	flagset.StringVar(&ldr.KeepproxyPath, "legacy-keepproxy-config", defaultKeepproxyConfigPath, "Legacy keepproxy configuration `file`")
-	flagset.StringVar(&ldr.GitHttpdPath, "legacy-git-httpd-config", defaultGitHttpdConfigPath, "Legacy arv-git-httpd configuration `file`")
-	flagset.StringVar(&ldr.KeepBalancePath, "legacy-keepbalance-config", defaultKeepBalanceConfigPath, "Legacy keep-balance configuration `file`")
-	flagset.BoolVar(&ldr.SkipLegacy, "skip-legacy", false, "Don't load legacy config files")
+	if !ldr.SkipLegacy {
+		flagset.StringVar(&ldr.KeepstorePath, "legacy-keepstore-config", defaultKeepstoreConfigPath, "Legacy keepstore configuration `file`")
+		flagset.StringVar(&ldr.KeepWebPath, "legacy-keepweb-config", defaultKeepWebConfigPath, "Legacy keep-web configuration `file`")
+		flagset.StringVar(&ldr.CrunchDispatchSlurmPath, "legacy-crunch-dispatch-slurm-config", defaultCrunchDispatchSlurmConfigPath, "Legacy crunch-dispatch-slurm configuration `file`")
+		flagset.StringVar(&ldr.WebsocketPath, "legacy-ws-config", defaultWebsocketConfigPath, "Legacy arvados-ws configuration `file`")
+		flagset.StringVar(&ldr.KeepproxyPath, "legacy-keepproxy-config", defaultKeepproxyConfigPath, "Legacy keepproxy configuration `file`")
+		flagset.StringVar(&ldr.GitHttpdPath, "legacy-git-httpd-config", defaultGitHttpdConfigPath, "Legacy arv-git-httpd configuration `file`")
+		flagset.StringVar(&ldr.KeepBalancePath, "legacy-keepbalance-config", defaultKeepBalanceConfigPath, "Legacy keep-balance configuration `file`")
+		flagset.BoolVar(&ldr.SkipLegacy, "skip-legacy", false, "Don't load legacy config files")
+	}
 }
 
 // MungeLegacyConfigArgs checks args for a -config flag whose argument

commit 58254d66b4a2c47a7c736c1e7c50203a8cf19805
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 16:34:44 2020 -0400

    16427: Improve -help / usage message.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/undelete/cmd.go b/lib/undelete/cmd.go
index e2d16ec3a..cc0ea09ac 100644
--- a/lib/undelete/cmd.go
+++ b/lib/undelete/cmd.go
@@ -36,9 +36,32 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	}()
 
 	loader := config.NewLoader(stdin, logger)
+	loader.SkipLegacy = true
 
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
+	flags.Usage = func() {
+		fmt.Fprintf(flags.Output(), `Usage:
+	%s [options ...] /path/to/manifest.txt [...]
+
+	This program recovers deleted collections. Recovery is
+	possible when the collection's manifest is still available and
+	all of its data blocks are still available or recoverable
+	(e.g., garbage collection is not enabled, the blocks are too
+	new for garbage collection, the blocks are referenced by other
+	collections, or the blocks have been trashed but not yet
+	deleted).
+
+	For each provided collection manifest, once all data blocks
+	are recovered/protected from garbage collection, a new
+	collection is saved and its UUID is printed on stdout.
+
+	Exit status will be zero if recovery is successful, i.e., a
+	collection is saved for each provided manifest.
+Options:
+`, prog)
+		flags.PrintDefaults()
+	}
 	loader.SetupFlags(flags)
 	loglevel := flags.String("log-level", "info", "logging level (debug, info, ...)")
 	err = flags.Parse(args)
@@ -50,8 +73,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	}
 
 	if len(flags.Args()) == 0 {
-		fmt.Fprintf(stderr, "Usage: %s [options] uuid_or_file ...\n", prog)
-		flags.PrintDefaults()
+		flags.Usage()
 		return 2
 	}
 

commit 3d80ac6a1ba336ae75fd7afa499d6e0dfd05bff3
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 15:45:44 2020 -0400

    16427: Make test logging more obvious.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/undelete/cmd_test.go b/lib/undelete/cmd_test.go
index e45152d14..a5edaf90b 100644
--- a/lib/undelete/cmd_test.go
+++ b/lib/undelete/cmd_test.go
@@ -81,9 +81,9 @@ func (*Suite) TestUntrashAndTouchBlock(c *check.C) {
 	// dir that keepstore might be using.
 	for _, datadir := range datadirs {
 		if fi, err := os.Stat(datadir); err != nil || !fi.IsDir() {
-			c.Logf("skipping datadir %q, evidently neither keepstore is using it", datadir)
 			continue
 		}
+		c.Logf("placing backdated trashed block in datadir %q", datadir)
 		trashfile := datadir + "/dcd/dcd0348cb2532ee90c99f1b846efaee7.trash.999999999"
 		os.Mkdir(datadir+"/dcd", 0777)
 		err = ioutil.WriteFile(trashfile, []byte("undelete test"), 0777)

commit cff3ee7ddf7caf971bae2850b3a44b9d5142931f
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 15:44:13 2020 -0400

    16427: Explain workerThreads choice.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/undelete/cmd.go b/lib/undelete/cmd.go
index 7fc5a933d..e2d16ec3a 100644
--- a/lib/undelete/cmd.go
+++ b/lib/undelete/cmd.go
@@ -219,9 +219,16 @@ func (und undeleter) RecoverManifest(mtxt string) (string, error) {
 	blobsigexp := time.Now().Add(blobsigttl / 2)
 	und.logger.WithField("blobsigexp", blobsigexp).Debug("chose save deadline")
 
+	// We'll start a number of threads, each working on
+	// checking/recovering one block at a time. The threads
+	// themselves don't need much CPU/memory, but to avoid hitting
+	// limits on keepstore connections, backend storage bandwidth,
+	// etc., we limit concurrency to 2 per keepstore node.
+	workerThreads := 2 * len(services)
+
 	blkFound := make([]bool, len(blks))
 	var wg sync.WaitGroup
-	for i := 0; i < 2*len(services); i++ {
+	for i := 0; i < workerThreads; i++ {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()

commit 1c3ce4051e76a877f41485f031c4219d2b732629
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 15:44:11 2020 -0400

    16427: Explain choice of blob ttl.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/undelete/cmd.go b/lib/undelete/cmd.go
index 6ebf2baf1..7fc5a933d 100644
--- a/lib/undelete/cmd.go
+++ b/lib/undelete/cmd.go
@@ -203,7 +203,18 @@ func (und undeleter) RecoverManifest(mtxt string) (string, error) {
 	}
 	und.logger.WithField("services", services).Debug("got list of services")
 
-	// Choose a deadline for saving a rescued collection.
+	// blobsigexp is our deadline for saving the rescued
+	// collection. This must be less than BlobSigningTTL
+	// (otherwise our rescued blocks could be garbage collected
+	// again before we protect them by saving the collection) but
+	// the exact value is somewhat arbitrary. If it's too soon, it
+	// will arrive before we're ready to save, and save will
+	// fail. If it's too late, we'll needlessly update timestamps
+	// on some blocks that were recently written/touched (e.g., by
+	// a previous attempt to rescue this same collection) and
+	// would have lived long enough anyway if left alone.
+	// BlobSigningTTL/2 (typically around 1 week) is much longer
+	// than than we need to recover even a very large collection.
 	blobsigttl := und.cluster.Collections.BlobSigningTTL.Duration()
 	blobsigexp := time.Now().Add(blobsigttl / 2)
 	und.logger.WithField("blobsigexp", blobsigexp).Debug("chose save deadline")

commit d9b06eb06c0ac38af922f102da0b8e405bb40f82
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Jun 4 14:49:32 2020 -0400

    16427: Return error instead of ok bool from util funcs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/undelete/cmd.go b/lib/undelete/cmd.go
index 291291587..6ebf2baf1 100644
--- a/lib/undelete/cmd.go
+++ b/lib/undelete/cmd.go
@@ -6,6 +6,7 @@ package undelete
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -112,18 +113,18 @@ type undeleter struct {
 	logger  logrus.FieldLogger
 }
 
+var errNotFound = errors.New("not found")
+
 // Return the timestamp of the newest copy of blk on svc. Second
 // return value is false if blk is not on svc at all, or an error
 // occurs.
-func (und undeleter) newestMtime(logger logrus.FieldLogger, blk string, svc arvados.KeepService) (time.Time, bool) {
+func (und undeleter) newestMtime(logger logrus.FieldLogger, blk string, svc arvados.KeepService) (time.Time, error) {
 	found, err := svc.Index(und.client, blk)
 	if err != nil {
 		logger.WithError(err).Warn("error getting index")
-		return time.Time{}, false
-	}
-	if len(found) == 0 {
-		logger.Debug("not found")
-		return time.Time{}, false
+		return time.Time{}, err
+	} else if len(found) == 0 {
+		return time.Time{}, errNotFound
 	}
 	var latest time.Time
 	for _, ent := range found {
@@ -133,38 +134,40 @@ func (und undeleter) newestMtime(logger logrus.FieldLogger, blk string, svc arva
 		}
 	}
 	logger.WithField("latest", latest).Debug("found")
-	return latest, true
+	return latest, nil
 }
 
+var errTouchIneffective = errors.New("(BUG?) touch succeeded but had no effect -- reported timestamp is still too old")
+
 // Ensure the given block exists on the given server and won't be
 // eligible for trashing until after our chosen deadline (blobsigexp).
-// Returns false if the block doesn't exist on the given server, has
-// an old timestamp and can't be updated, or any error occurred.
-// Reports errors via logger.
+// Returns an error if the block doesn't exist on the given server, or
+// has an old timestamp and can't be updated.  Reports errors via
+// logger.
 //
 // After we decide a block is "safe" (whether or not we had to untrash
 // it), keep-balance might notice that it's currently unreferenced and
 // decide to trash it, all before our recovered collection gets
 // saved. But if the block's timestamp is more recent than blobsigttl,
 // keepstore will refuse to trash it even if told to by keep-balance.
-func (und undeleter) ensureSafe(ctx context.Context, logger logrus.FieldLogger, blk string, svc arvados.KeepService, blobsigttl time.Duration, blobsigexp time.Time) bool {
-	if latest, ok := und.newestMtime(logger, blk, svc); !ok {
-		return false
+func (und undeleter) ensureSafe(ctx context.Context, logger logrus.FieldLogger, blk string, svc arvados.KeepService, blobsigttl time.Duration, blobsigexp time.Time) error {
+	if latest, err := und.newestMtime(logger, blk, svc); err != nil {
+		return err
 	} else if latest.Add(blobsigttl).After(blobsigexp) {
-		return true
+		return nil
 	}
 	if err := svc.Touch(ctx, und.client, blk); err != nil {
-		logger.WithError(err).Warn("error updating timestamp")
-		return false
+		return fmt.Errorf("error updating timestamp: %s", err)
 	}
 	logger.Debug("updated timestamp")
-	if latest, ok := und.newestMtime(logger, blk, svc); !ok {
-		return false
+	if latest, err := und.newestMtime(logger, blk, svc); err == errNotFound {
+		return fmt.Errorf("(BUG?) touch succeeded, but then block did not appear in index")
+	} else if err != nil {
+		return err
 	} else if latest.Add(blobsigttl).After(blobsigexp) {
-		return true
+		return nil
 	} else {
-		logger.WithField("latest", latest).Error("BUG? touch return success, but newest reported timestamp is still too old")
-		return false
+		return errTouchIneffective
 	}
 }
 
@@ -225,7 +228,12 @@ func (und undeleter) RecoverManifest(mtxt string) (string, error) {
 							}
 							logger.Info("untrashed")
 						}
-						if und.ensureSafe(ctx, logger, blk, svc, blobsigttl, blobsigexp) {
+						err := und.ensureSafe(ctx, logger, blk, svc, blobsigttl, blobsigexp)
+						if err == errNotFound {
+							logger.Debug(err)
+						} else if err != nil {
+							logger.Error(err)
+						} else {
 							blkFound[idx] = true
 							continue nextblk
 						}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list