Milo: Handle missing / transient LogDog failures.

milo/appengine/logdog/build.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package logdog
 
 import (
         "fmt"
         "net/http"
         "time"
 
+        "github.com/luci/luci-go/common/errors"
         log "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/common/proto/google"
         miloProto "github.com/luci/luci-go/common/proto/milo"
+        "github.com/luci/luci-go/common/retry"
         "github.com/luci/luci-go/grpc/grpcutil"
         "github.com/luci/luci-go/logdog/api/logpb"
         "github.com/luci/luci-go/logdog/client/coordinator"
         "github.com/luci/luci-go/logdog/common/types"
         "github.com/luci/luci-go/logdog/common/viewer"
         "github.com/luci/luci-go/luci_config/common/cfgtypes"
         "github.com/luci/luci-go/milo/api/resp"
         "github.com/luci/luci-go/milo/appengine/logdog/internal"
         "github.com/luci/luci-go/milo/common/miloerror"
 
         "github.com/golang/protobuf/proto"
         mc "github.com/luci/gae/service/memcache"
         "golang.org/x/net/context"
-        "google.golang.org/grpc/codes"
 )
 
 const (
         // intermediateCacheLifetime is the amount of time to cache intermediate (non-
         // terminal) annotation streams. Terminal annotation streams are cached
         // indefinitely.
         intermediateCacheLifetime = 10 * time.Second
 
         // defaultLogDogHost is the default LogDog host, if one isn't specified via
         // query string.
@@ skipping 72 matching lines @@
         log.Fields{
                 "host":    as.Client.Host,
                 "project": as.Project,
                 "path":    as.Path,
         }.Infof(c, "Making tail request to LogDog to fetch annotation stream.")
 
         var (
                 state  coordinator.LogStream
                 stream = as.Client.Stream(as.Project, as.Path)
         )
-        le, err := stream.Tail(c, coordinator.WithState(&state), coordinator.Complete())
-        switch code := grpcutil.Code(err); code {
-        case codes.OK:
-                break
 
-        case codes.NotFound:
-                return nil, &miloerror.Error{
-                        Message: "Stream not found",
-                        Code:    http.StatusNotFound,
+        var le *logpb.LogEntry
+        err = retry.Retry(c, retry.TransientOnly(streamLoadRetry), func() (err error) {
+                le, err = stream.Tail(c, coordinator.WithState(&state), coordinator.Complete())
+                switch errors.Unwrap(err) {
+                case nil:
+                        return nil
+
+                case coordinator.ErrNoSuchStream:

[hinoka, 2017/02/24 01:30:49]

Consider making this just a non-transient error.  If we do so, Milo should still
display the build as "pending" immediately, which is probably better than
waiting 5 seconds and seeing maybe the same thing or something different.

[dnj, 2017/02/24 03:24:09]

Good idea, Done.

+                        log.Warningf(c, "Stream not found (maybe transient).")
+                        return errors.WrapTransient(err)
+
+                default:
+                        // If this is a transient gRPC failure, retry.
+                        return grpcutil.WrapIfTransient(err)
                 }
-
-        default:
-                // TODO: Once we switch to delegation tokens and are making the request on
-                // behalf of a user rather than the Milo service, handle PermissionDenied.
-                log.Fields{
-                        log.ErrorKey: err,
-                        "code":       code,
-                }.Errorf(c, "Failed to load LogDog annotation stream.")
+        }, retry.LogCallback(c, "load stream"))
+        if err != nil {
+                log.WithError(err).Errorf(c, "Failed to load stream.")
                 return nil, &miloerror.Error{
                         Message: "Failed to load stream",
                         Code:    http.StatusInternalServerError,
                 }
         }
 
         // Make sure that this is an annotation stream.
         switch {
         case state.Desc.ContentType != miloProto.ContentTypeAnnotations:
                 return nil, &miloerror.Error{
@@ skipping 156 matching lines @@
                 if link.Label == "" {
                         link.Label = "unnamed"
                 }
                 return &link
 
         default:
                 // Don't know how to render.
                 return nil
         }
 }
+
+// streamLoadRetry is a retry iterator to apply to repeatedly load a log stream.
+//
+// This implements exponential backoff, capped at 5 seconds.
+func streamLoadRetry() retry.Iterator {
+        return &retry.ExponentialBackoff{
+                Limited: retry.Limited{
+                        Delay:   500 * time.Millisecond,
+                        Retries: 10,
+                },
+                MaxDelay: 5 * time.Second,
+        }
+}