Retry RPC deadlines when fetching GAE settings.

common/lazyslot/lazyslot.go

Index: common/lazyslot/lazyslot.go
diff --git a/common/lazyslot/lazyslot.go b/common/lazyslot/lazyslot.go
index dd21a84a5d96dd6715cfbc9bae77312cfaaeb6ce..e6138599886523739c9450185b18d21a3212b18e 100644
--- a/common/lazyslot/lazyslot.go
+++ b/common/lazyslot/lazyslot.go
@@ -18,6 +18,7 @@ import (
 	"golang.org/x/net/context"
 
 	"github.com/luci/luci-go/common/clock"
+	"github.com/luci/luci-go/common/retry"
 )
 
 // Value is what's stored in a Slot. It is treated as immutable value.
@@ -41,9 +42,11 @@ type Fetcher func(c context.Context, prev Value) (Value, error)
 // Only one goroutine will be busy refreshing, all others will see a slightly
 // stale copy of the value during the refresh.
 type Slot struct {
-	Fetcher Fetcher       // used to actually load the value on demand
-	Timeout time.Duration // how long to allow to fetch, 5 sec by default.
-	Async   bool          // if true do fetches in background goroutine
+	Fetcher       Fetcher        // used to actually load the value on demand
+	Timeout       time.Duration  // how long to allow to fetch, 15 sec by default.
+	Async         bool           // if true do fetches in background goroutine
+	RetryFactory  retry.Factory  // if non-nil, defines how to retry fetch errors
+	RetryCallback retry.Callback // called before retries, useful in tests

[iannucci, 2016/03/01 00:44:21]

Is this still used?

[Vadim Sh., 2016/03/01 01:24:05]

No, but it can be useful in general (e.g. to plug in logging for retries), so I
kept it.

 
 	lock              sync.Mutex      // protects the guts below
 	current           *Value          // currently known value or nil if not fetched
@@ -78,7 +81,7 @@ func (s *Slot) Get(c context.Context) (result Value, err error) {
 	// there's nothing to return yet. All goroutines would have to wait for this
 	// initial fetch to complete. They'll all block on s.lock.Lock() above.
 	if s.current == nil {
-		result, err = doFetchNoPanic(c, s.Fetcher, Value{})
+		result, err = fetchWithRetries(s.makeFetcherCtx(c), s.Fetcher, s.RetryFactory, s.RetryCallback, Value{})
 		if err == nil {
 			s.current = &result
 		}
@@ -96,17 +99,15 @@ func (s *Slot) Get(c context.Context) (result Value, err error) {
 
 	// No one is fetching the value now, we should do it. Prepare new context
 	// that will be used to do the fetch once lock is released.
-	timeout := 5 * time.Second
-	if s.Timeout != 0 {
-		timeout = s.Timeout
-	}
-	s.currentFetcherCtx, _ = context.WithTimeout(c, timeout)
+	s.currentFetcherCtx = s.makeFetcherCtx(c)
 
 	// Copy lock-protected guts into local variables before releasing the lock.
 	currentFetcherCtx := s.currentFetcherCtx
 	fetchCb := s.Fetcher
 	prevVal := *s.current
 	async := s.Async
+	retryFactory := s.RetryFactory
+	retryCallback := s.RetryCallback
 
 	// Release the lock to allow other goroutines to grab stale copy while fetch
 	// is in the progress.
@@ -122,7 +123,7 @@ func (s *Slot) Get(c context.Context) (result Value, err error) {
 				s.current = &result
 			}
 		}()
-		return doFetchNoPanic(currentFetcherCtx, fetchCb, prevVal)
+		return fetchWithRetries(currentFetcherCtx, fetchCb, retryFactory, retryCallback, prevVal)
 	}
 
 	if async {
@@ -133,9 +134,36 @@ func (s *Slot) Get(c context.Context) (result Value, err error) {
 	return fetch()
 }
 
-// doFetchNoPanic calls fetcher callback, trapping panics and validating
+// makeFetcherCtx prepares a context to use for fetch operation.
+//
+// Must be called under the lock.
+func (s *Slot) makeFetcherCtx(c context.Context) context.Context {
+	timeout := 15 * time.Second
+	if s.Timeout != 0 {
+		timeout = s.Timeout
+	}
+	fetcherCtx, _ := clock.WithTimeout(c, timeout)
+	return fetcherCtx
+}
+
+// fetchWithRetries calls fetchNoPanic, retrying on errors.
+func fetchWithRetries(ctx context.Context, cb Fetcher, factory retry.Factory, retryCb retry.Callback, prev Value) (result Value, err error) {
+	if factory == nil {
+		return fetchNoPanic(ctx, cb, prev)
+	}
+	err = retry.Retry(clock.Tag(ctx, "retry"), factory, func() error {
+		result, err = fetchNoPanic(ctx, cb, prev)
+		return err
+	}, retryCb)
+	if err != nil {
+		result = Value{}
+	}
+	return
+}
+
+// fetchNoPanic calls fetcher callback, trapping panics and validating
 // return value.
-func doFetchNoPanic(ctx context.Context, cb Fetcher, prev Value) (result Value, err error) {
+func fetchNoPanic(ctx context.Context, cb Fetcher, prev Value) (result Value, err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("panic caught in lazyslot.Slot Fetcher - %s", r)