Retry network operations instead of entire update
Updating attempt flag to only retry network operations instead of full
jiri update as full jiri update tends to fail for slow network
connections.
Also retrying filesystem operations has little value as
they are going to fail most of the time on retry.
TO-382
Change-Id: Ib94fdb9dd50a38475e6d6dc67baf4a836c180a6b
diff --git a/cmd/jiri/update.go b/cmd/jiri/update.go
index dc16caf..cc36e0b 100644
--- a/cmd/jiri/update.go
+++ b/cmd/jiri/update.go
@@ -17,7 +17,7 @@
var (
gcFlag bool
localManifestFlag bool
- attemptsFlag int
+ attemptsFlag uint
autoupdateFlag bool
forceAutoupdateFlag bool
rebaseUntrackedFlag bool
@@ -33,7 +33,7 @@
cmdUpdate.Flags.BoolVar(&gcFlag, "gc", false, "Garbage collect obsolete repositories.")
cmdUpdate.Flags.BoolVar(&localManifestFlag, "local-manifest", false, "Use local manifest")
- cmdUpdate.Flags.IntVar(&attemptsFlag, "attempts", 1, "Number of attempts before failing.")
+ cmdUpdate.Flags.UintVar(&attemptsFlag, "attempts", 1, "Number of attempts before failing.")
cmdUpdate.Flags.BoolVar(&autoupdateFlag, "autoupdate", true, "Automatically update to the new version.")
cmdUpdate.Flags.BoolVar(&forceAutoupdateFlag, "force-autoupdate", false, "Always update to the current version.")
cmdUpdate.Flags.BoolVar(&rebaseUntrackedFlag, "rebase-untracked", false, "Rebase untracked branches onto HEAD.")
@@ -65,10 +65,16 @@
return jirix.UsageErrorf("unexpected number of arguments")
}
+ if attemptsFlag < 1 {
+ return jirix.UsageErrorf("Number of attempts should be >= 1")
+ }
+ jirix.Attempts = attemptsFlag
+
if autoupdateFlag {
// Try to update Jiri itself.
- err := jiri.UpdateAndExecute(forceAutoupdateFlag)
- if err != nil {
+ if err := retry.Function(jirix, func() error {
+ return jiri.UpdateAndExecute(forceAutoupdateFlag)
+ }, fmt.Sprintf("download jiri binary"), retry.AttemptsOpt(jirix.Attempts)); err != nil {
fmt.Printf("warning: automatic update failed: %v\n", err)
}
}
@@ -77,15 +83,13 @@
rebaseTrackedFlag = true
}
- // Update all projects to their latest version.
- // Attempt <attemptsFlag> times before failing.
- err := retry.Function(jirix.Context, func() error {
- if len(args) > 0 {
- return project.CheckoutSnapshot(jirix, args[0], gcFlag, runHooksFlag, hookTimeoutFlag)
- } else {
- return project.UpdateUniverse(jirix, gcFlag, localManifestFlag, rebaseTrackedFlag, rebaseUntrackedFlag, rebaseAllFlag, runHooksFlag, hookTimeoutFlag)
- }
- }, retry.AttemptsOpt(attemptsFlag))
+ var err error
+ if len(args) > 0 {
+ err = project.CheckoutSnapshot(jirix, args[0], gcFlag, runHooksFlag, hookTimeoutFlag)
+ } else {
+ err = project.UpdateUniverse(jirix, gcFlag, localManifestFlag,
+ rebaseTrackedFlag, rebaseUntrackedFlag, rebaseAllFlag, runHooksFlag, hookTimeoutFlag)
+ }
if err2 := project.WriteUpdateHistorySnapshot(jirix, "", localManifestFlag); err2 != nil {
if err != nil {
diff --git a/jiritest/x.go b/jiritest/x.go
index 8da1acd..cf0ec1f 100644
--- a/jiritest/x.go
+++ b/jiritest/x.go
@@ -35,5 +35,5 @@
t.Fatalf("RemoveAll(%q) failed: %v", root, err)
}
}
- return &jiri.X{Context: ctx, Root: root, Jobs: jiri.DefaultJobs, Color: color, Logger: logger}, cleanup
+ return &jiri.X{Context: ctx, Root: root, Jobs: jiri.DefaultJobs, Color: color, Logger: logger, Attempts: 1}, cleanup
}
diff --git a/project/project.go b/project/project.go
index 382aa12..99d9119 100644
--- a/project/project.go
+++ b/project/project.go
@@ -30,6 +30,7 @@
"fuchsia.googlesource.com/jiri/git"
"fuchsia.googlesource.com/jiri/gitutil"
"fuchsia.googlesource.com/jiri/osutil"
+ "fuchsia.googlesource.com/jiri/retry"
)
var (
@@ -783,6 +784,20 @@
// project names to a collections of commits.
type Update map[string][]CL
+// clone is a wrapper that reattempts a git clone operation on failure.
+func clone(jirix *jiri.X, repo, path string, opts ...gitutil.CloneOpt) error {
+ return retry.Function(jirix, func() error {
+ return gitutil.New(jirix).Clone(repo, path, opts...)
+ }, fmt.Sprintf("Cloning %s", repo), retry.AttemptsOpt(jirix.Attempts))
+}
+
+// fetch is a wrapper that reattempts a git fetch operation on failure.
+func fetch(jirix *jiri.X, path, remote string, opts ...gitutil.FetchOpt) error {
+ return retry.Function(jirix, func() error {
+ return gitutil.New(jirix, gitutil.RootDirOpt(path)).Fetch(remote, opts...)
+ }, fmt.Sprintf("Fetching for %s", path), retry.AttemptsOpt(jirix.Attempts))
+}
+
// CreateSnapshot creates a manifest that encodes the current state of
// HEAD of all projects and writes this snapshot out to the given file.
func CreateSnapshot(jirix *jiri.X, file string, localManifest bool) error {
@@ -1385,10 +1400,10 @@
return err
}
if project.HistoryDepth > 0 {
- return gitutil.New(jirix, gitutil.RootDirOpt(project.Path)).Fetch("origin", gitutil.PruneOpt(true),
+ return fetch(jirix, project.Path, "origin", gitutil.PruneOpt(true),
gitutil.DepthOpt(project.HistoryDepth), gitutil.UpdateShallowOpt(true))
} else {
- return gitutil.New(jirix, gitutil.RootDirOpt(project.Path)).Fetch("origin", gitutil.PruneOpt(true))
+ return fetch(jirix, project.Path, "origin", gitutil.PruneOpt(true))
}
}
@@ -1758,19 +1773,19 @@
return err
}
if jirix.Shared {
- if err := gitutil.New(jirix).Clone(cacheDirPath, path,
- gitutil.SharedOpt(true), gitutil.NoCheckoutOpt(true)); err != nil {
+ if err := clone(jirix, cacheDirPath, path, gitutil.SharedOpt(true),
+ gitutil.NoCheckoutOpt(true)); err != nil {
return err
}
} else {
- if err := gitutil.New(jirix).Clone(remoteUrl, path,
- gitutil.ReferenceOpt(cacheDirPath), gitutil.NoCheckoutOpt(true)); err != nil {
+ if err := clone(jirix, remoteUrl, path, gitutil.ReferenceOpt(cacheDirPath),
+ gitutil.NoCheckoutOpt(true)); err != nil {
return err
}
}
} else {
- if err := gitutil.New(jirix).Clone(remoteUrl, path, gitutil.NoCheckoutOpt(true)); err != nil {
+ if err := clone(jirix, remoteUrl, path, gitutil.NoCheckoutOpt(true)); err != nil {
return err
}
}
@@ -2027,11 +2042,15 @@
if _, err := os.Stat(filepath.Join(dir, "shallow")); err == nil {
// Shallow cache, fetch only manifest tracked remote branch
refspec := fmt.Sprintf("+refs/heads/%s:refs/heads/%s", branch, branch)
- if err := gitutil.New(jirix, gitutil.RootDirOpt(dir)).FetchRefspec("origin", refspec, gitutil.PruneOpt(true)); err != nil {
+
+ if err := retry.Function(jirix, func() error {
+ return gitutil.New(jirix, gitutil.RootDirOpt(dir)).FetchRefspec("origin", refspec, gitutil.PruneOpt(true))
+ }, fmt.Sprintf("Fetching for %s:%s", dir, refspec),
+ retry.AttemptsOpt(jirix.Attempts)); err != nil {
return err
}
} else {
- if err := gitutil.New(jirix, gitutil.RootDirOpt(dir)).Fetch("origin", gitutil.PruneOpt(true)); err != nil {
+ if err := fetch(jirix, dir, "origin", gitutil.PruneOpt(true)); err != nil {
return err
}
}
@@ -2512,7 +2531,7 @@
}
defer os.RemoveAll(tmpDir)
for _, hook := range hooks {
- jirix.Logger.Infof("running hook(%v) for project %q", hook.Name, hook.ProjectName)
+ jirix.Logger.Infof("running hook(%s) for project %q", hook.Name, hook.ProjectName)
go func(hook Hook) {
outFile, err := ioutil.TempFile(tmpDir, hook.Name+"-out")
if err != nil {
@@ -2528,20 +2547,24 @@
fmt.Fprintf(outFile, "output for hook(%v) for project %q\n", hook.Name, hook.ProjectName)
fmt.Fprintf(errFile, "Error for hook(%v) for project %q\n", hook.Name, hook.ProjectName)
cmdLine := filepath.Join(hook.ActionPath, hook.Action)
- ctx, cancel := context.WithTimeout(context.Background(), time.Duration(runHookTimeout)*time.Minute)
- defer cancel()
- command := exec.CommandContext(ctx, cmdLine)
- command.Dir = hook.ActionPath
- command.Stdin = os.Stdin
- command.Stdout = outFile
- command.Stderr = errFile
- env := jirix.Env()
- command.Env = envvar.MapToSlice(env)
- jirix.Logger.Tracef("Run: %q", cmdLine)
- err = command.Run()
- if ctx.Err() == context.DeadlineExceeded {
- err = ctx.Err()
- }
+ err = retry.Function(jirix, func() error {
+ ctx, cancel := context.WithTimeout(context.Background(), time.Duration(runHookTimeout)*time.Minute)
+ defer cancel()
+ command := exec.CommandContext(ctx, cmdLine)
+ command.Dir = hook.ActionPath
+ command.Stdin = os.Stdin
+ command.Stdout = outFile
+ command.Stderr = errFile
+ env := jirix.Env()
+ command.Env = envvar.MapToSlice(env)
+ jirix.Logger.Tracef("Run: %q", cmdLine)
+ err = command.Run()
+ if ctx.Err() == context.DeadlineExceeded {
+ err = ctx.Err()
+ }
+ return err
+ }, fmt.Sprintf("running hook(%s) for project %s", hook.Name, hook.ProjectName),
+ retry.AttemptsOpt(jirix.Attempts))
ch <- result{outFile, errFile, err}
}(hook)
@@ -2810,8 +2833,7 @@
}
if jirix.Shared && cache != "" {
- if err := gitutil.New(jirix).Clone(cache, tmpDir,
- gitutil.SharedOpt(true),
+ if err := clone(jirix, cache, tmpDir, gitutil.SharedOpt(true),
gitutil.NoCheckoutOpt(true), gitutil.DepthOpt(op.project.HistoryDepth)); err != nil {
return err
}
@@ -2821,8 +2843,7 @@
ref = ""
}
remote := rewriteRemote(jirix, op.project.Remote)
- if err := gitutil.New(jirix).Clone(remote, tmpDir,
- gitutil.ReferenceOpt(ref),
+ if err := clone(jirix, remote, tmpDir, gitutil.ReferenceOpt(ref),
gitutil.NoCheckoutOpt(true), gitutil.DepthOpt(op.project.HistoryDepth)); err != nil {
return err
}
diff --git a/retry/retry.go b/retry/retry.go
index ec81838..3b941af 100644
--- a/retry/retry.go
+++ b/retry/retry.go
@@ -10,7 +10,7 @@
"fmt"
"time"
- "fuchsia.googlesource.com/jiri/tool"
+ "fuchsia.googlesource.com/jiri"
)
type RetryOpt interface {
@@ -27,12 +27,12 @@
const (
defaultAttempts = 3
- defaultInterval = 10 * time.Second
+ defaultInterval = 5 * time.Second
)
// Function retries the given function for the given number of
// attempts at the given interval.
-func Function(ctx *tool.Context, fn func() error, opts ...RetryOpt) error {
+func Function(jirix *jiri.X, fn func() error, task string, opts ...RetryOpt) error {
attempts, interval := defaultAttempts, defaultInterval
for _, opt := range opts {
switch typedOpt := opt.(type) {
@@ -46,16 +46,19 @@
var err error
for i := 1; i <= attempts; i++ {
if i > 1 {
- fmt.Fprintf(ctx.Stdout(), "Attempt %d/%d:\n", i, attempts)
+ jirix.Logger.Infof("Attempt %d/%d: %s\n\n", i, attempts, task)
}
if err = fn(); err == nil {
return nil
}
- fmt.Fprintf(ctx.Stderr(), "%v\n", err)
if i < attempts {
- fmt.Fprintf(ctx.Stdout(), "Wait for %v before next attempt...\n", interval)
+ jirix.Logger.Errorf("%s\n\n", err)
+ jirix.Logger.Infof("Wait for %s before next attempt...: %s\n\n", interval, task)
time.Sleep(interval)
}
}
- return fmt.Errorf("Failed %d times in a row. Last error:\n%v", attempts, err)
+ if attempts > 1 {
+ return fmt.Errorf("%q failed %d times in a row, Last error: %s", task, attempts, err)
+ }
+ return err
}
diff --git a/x.go b/x.go
index f5d5080..f882a8f 100644
--- a/x.go
+++ b/x.go
@@ -99,6 +99,7 @@
Color color.Color
Logger *log.Logger
failures uint32
+ Attempts uint
}
func (jirix *X) IncrementFailures() {
@@ -181,12 +182,13 @@
}
x := &X{
- Context: ctx,
- Root: root,
- Usage: env.UsageErrorf,
- Jobs: jobsFlag,
- Color: color,
- Logger: logger,
+ Context: ctx,
+ Root: root,
+ Usage: env.UsageErrorf,
+ Jobs: jobsFlag,
+ Color: color,
+ Logger: logger,
+ Attempts: 1,
}
configPath := filepath.Join(x.RootMetaDir(), ConfigFile)
if _, err := os.Stat(configPath); err == nil {
@@ -335,6 +337,7 @@
RewriteSsoToHttps: x.RewriteSsoToHttps,
Logger: x.Logger,
failures: x.failures,
+ Attempts: x.Attempts,
}
}
