From 413bfb39a940455cb116c7d0ca715b2ced78a11c Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 31 Jul 2018 15:06:36 -0700 Subject: [PATCH] Use backoff package for retry logic PiperOrigin-RevId: 206834838 Change-Id: I9a44c6fa5f4766a01f86e90810f025cefecdf2d4 --- runsc/specutils/BUILD | 1 + runsc/specutils/specutils.go | 30 ++++++++++++++---------------- runsc/specutils/specutils_test.go | 4 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/runsc/specutils/BUILD b/runsc/specutils/BUILD index 34c952bdf..a22ab789a 100644 --- a/runsc/specutils/BUILD +++ b/runsc/specutils/BUILD @@ -13,6 +13,7 @@ go_library( "//pkg/abi/linux", "//pkg/log", "//pkg/sentry/kernel/auth", + "@com_github_cenkalti_backoff//:go_default_library", "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", ], ) diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 861e7fd70..27441cbde 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -26,6 +26,7 @@ import ( "syscall" "time" + "github.com/cenkalti/backoff" specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.googlesource.com/gvisor/pkg/abi/linux" "gvisor.googlesource.com/gvisor/pkg/log" @@ -313,33 +314,30 @@ func SandboxID(spec *specs.Spec) (string, bool) { // the 'ready' function returns true. It continues to wait if 'ready' returns // false. It returns error on timeout, if the process stops or if 'ready' fails. func WaitForReady(pid int, timeout time.Duration, ready func() (bool, error)) error { - backoff := 1 * time.Millisecond - for start := time.Now(); time.Now().Sub(start) < timeout; { + b := backoff.NewExponentialBackOff() + b.InitialInterval = 1 * time.Millisecond + b.MaxInterval = 1 * time.Second + b.MaxElapsedTime = timeout + + op := func() error { if ok, err := ready(); err != nil { - return err + return backoff.Permanent(err) } else if ok { return nil } // Check if the process is still running. - var ws syscall.WaitStatus - var ru syscall.Rusage - // If the process is alive, child is 0 because of the NOHANG option. // If the process has terminated, child equals the process id. + var ws syscall.WaitStatus + var ru syscall.Rusage child, err := syscall.Wait4(pid, &ws, syscall.WNOHANG, &ru) if err != nil { - return fmt.Errorf("error waiting for process: %v", err) + return backoff.Permanent(fmt.Errorf("error waiting for process: %v", err)) } else if child == pid { - return fmt.Errorf("process %d has terminated", pid) - } - - // Process continues to run, backoff and retry. - time.Sleep(backoff) - backoff *= 2 - if backoff > 1*time.Second { - backoff = 1 * time.Second + return backoff.Permanent(fmt.Errorf("process %d has terminated", pid)) } + return fmt.Errorf("process %d not running yet", pid) } - return fmt.Errorf("timed out waiting for process (%d)", pid) + return backoff.Retry(op, b) } diff --git a/runsc/specutils/specutils_test.go b/runsc/specutils/specutils_test.go index 2dc5d90cc..2c4e3e729 100644 --- a/runsc/specutils/specutils_test.go +++ b/runsc/specutils/specutils_test.go @@ -94,8 +94,8 @@ func TestWaitForReadyTimeout(t *testing.T) { err := WaitForReady(cmd.Process.Pid, 50*time.Millisecond, func() (bool, error) { return false, nil }) - if !strings.Contains(err.Error(), "timed out") { - t.Errorf("ProcessWaitReady got: %v, expected: timed out", err) + if !strings.Contains(err.Error(), "not running yet") { + t.Errorf("ProcessWaitReady got: %v, expected: not running yet", err) } cmd.Process.Kill() }