Merge "veyron/runtimes/google/ipc: Failed name resolution should be retryable."
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index 2788afb..91fc5bc 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -300,6 +300,9 @@
// Caller specified deadline.
deadline = time.Now().Add(time.Duration(r))
}
+
+ skipResolve := getNoResolveOpt(opts)
+
var lastErr verror.E
for retries := 0; ; retries++ {
if retries != 0 {
@@ -307,12 +310,26 @@
break
}
}
- call, err := c.tryCall(ctx, name, method, args, opts)
+ call, err := c.tryCall(ctx, name, method, args, skipResolve, opts)
if err == nil {
return call, nil
}
lastErr = err
- if time.Now().After(deadline) || err.Action() != verror.RetryConnection {
+
+ shouldRetry := true
+ switch {
+ case err.Action() != verror.RetryConnection && err.Action() != verror.RetryRefetch:
+ shouldRetry = false
+ case time.Now().After(deadline):
+ shouldRetry = false
+ case verror.Is(err, verror.NoServers.ID) && skipResolve:
+ // If we're skipping resolution and there are no servers for
+ // this call retrying is not going to help, we can't come up
+ // with new servers if there is no resolution.
+ shouldRetry = false
+ }
+
+ if !shouldRetry {
span.Annotatef("Cannot retry after error: %s", err)
break
}
@@ -345,15 +362,18 @@
}
// tryCall makes a single attempt at a call, against possibly multiple servers.
-func (c *client) tryCall(ctx context.T, name, method string, args []interface{}, opts []ipc.CallOpt) (ipc.Call, verror.E) {
+func (c *client) tryCall(ctx context.T, name, method string, args []interface{}, skipResolve bool, opts []ipc.CallOpt) (ipc.Call, verror.E) {
mtPattern, serverPattern, name := splitObjectName(name)
noDischarges := shouldNotFetchDischarges(opts)
// Resolve name unless told not to.
var servers []string
- if getNoResolveOpt(opts) {
+ if skipResolve {
servers = []string{name}
} else {
if resolved, err := c.ns.Resolve(ctx, name, naming.RootBlessingPatternOpt(mtPattern)); err != nil {
+ if verror.Is(err, naming.ErrNoSuchName.ID) {
+ return nil, verror.Make(verror.NoServers, ctx, name)
+ }
return nil, verror.Make(verror.NoExist, ctx, name, err)
} else {
if len(resolved) == 0 {
diff --git a/runtimes/google/ipc/context_test.go b/runtimes/google/ipc/context_test.go
index 515ee26..cdc7e0e 100644
--- a/runtimes/google/ipc/context_test.go
+++ b/runtimes/google/ipc/context_test.go
@@ -16,9 +16,13 @@
// so we use a fake one that panics if used. The runtime
// implementation should not ever use the Runtime from a context.
func testContext() context.T {
+ ctx, _ := testContextWithoutDeadline().WithTimeout(20 * time.Second)
+ return ctx
+}
+
+func testContextWithoutDeadline() context.T {
ctx := InternalNewContext(&runtime.PanicRuntime{})
ctx, _ = vtrace.WithNewRootSpan(ctx, nil, false)
- ctx, _ = ctx.WithDeadline(time.Now().Add(20 * time.Second))
return ctx
}
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index a5be1c5..82199f8 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -36,7 +36,7 @@
"veyron.io/veyron/veyron/runtimes/google/lib/publisher"
inaming "veyron.io/veyron/veyron/runtimes/google/naming"
tnaming "veyron.io/veyron/veyron/runtimes/google/testing/mocks/naming"
- "veyron.io/veyron/veyron/runtimes/google/vtrace"
+ ivtrace "veyron.io/veyron/veyron/runtimes/google/vtrace"
)
func init() {
@@ -424,11 +424,14 @@
}
)
+ ctx := testContextWithoutDeadline()
+
// Start the main server.
- dc, err := InternalNewDischargeClient(mgr, ns, testContext(), vc.LocalPrincipal{pserver})
+ dc, err := InternalNewDischargeClient(mgr, ns, ctx, vc.LocalPrincipal{pserver})
if err != nil {
t.Errorf("InternalNewDischargeClient failed: %v", err)
}
+
_, server := startServer(t, pserver, mgr, ns, serverName, testServerDisp{&testServer{}}, dc)
defer stopServer(t, server, ns, serverName)
@@ -456,7 +459,8 @@
t.Errorf("%s: failed to create client: %v", name, err)
continue
}
- call, err := client.StartCall(testContext(), fmt.Sprintf("[%s]%s/suffix", test.pattern, serverName), "Method", nil)
+ dctx, cancel := ctx.WithTimeout(10 * time.Second)
+ call, err := client.StartCall(dctx, fmt.Sprintf("[%s]%s/suffix", test.pattern, serverName), "Method", nil)
if !matchesErrorPattern(err, test.errID, test.err) {
t.Errorf(`%d: %s: client.StartCall: got error "%v", want to match "%v"`, i, name, err, test.err)
} else if call != nil {
@@ -468,9 +472,8 @@
t.Errorf("%s: %q.MatchedBy(%v) failed", name, test.pattern, blessings)
}
}
- vlog.Infof("\nC")
+ cancel()
client.Close()
-
}
}
@@ -682,7 +685,7 @@
func (s *dischargeTestServer) Discharge(ctx ipc.ServerContext, cav vdlutil.Any, impetus security.DischargeImpetus) (vdlutil.Any, error) {
s.impetus = append(s.impetus, impetus)
- s.traceid = append(s.traceid, vtrace.FromContext(ctx).Trace().ID())
+ s.traceid = append(s.traceid, ivtrace.FromContext(ctx).Trace().ID())
return nil, fmt.Errorf("discharges not issued")
}
@@ -807,7 +810,7 @@
}
defer client.Close()
ctx := testContext()
- tid := vtrace.FromContext(ctx).Trace().ID()
+ tid := ivtrace.FromContext(ctx).Trace().ID()
// StartCall should fetch the discharge, do not worry about finishing the RPC - do not care about that for this test.
if _, err := client.StartCall(ctx, object, "Method", []interface{}{"argument"}); err != nil {
t.Errorf("StartCall(%+v) failed: %v", test.Requirements, err)
diff --git a/runtimes/google/testing/mocks/naming/namespace.go b/runtimes/google/testing/mocks/naming/namespace.go
index f6b0ba0..e3403e5 100644
--- a/runtimes/google/testing/mocks/naming/namespace.go
+++ b/runtimes/google/testing/mocks/naming/namespace.go
@@ -8,7 +8,7 @@
"veyron.io/veyron/veyron2/context"
"veyron.io/veyron/veyron2/naming"
- "veyron.io/veyron/veyron2/verror"
+ verror "veyron.io/veyron/veyron2/verror2"
"veyron.io/veyron/veyron2/vlog"
)
@@ -74,7 +74,7 @@
return ret, nil
}
}
- return nil, verror.NoExistf("Resolve name %q not found in %v", name, ns.mounts)
+ return nil, verror.Make(naming.ErrNoSuchName, ctx, fmt.Sprintf("Resolve name %q not found in %v", name, ns.mounts))
}
func (ns *namespace) ResolveX(ctx context.T, name string, opts ...naming.ResolveOpt) (*naming.MountEntry, error) {
@@ -95,7 +95,7 @@
return e, nil
}
}
- return nil, verror.NoExistf("Resolve name %q not found in %v", name, ns.mounts)
+ return nil, verror.Make(naming.ErrNoSuchName, ctx, fmt.Sprintf("Resolve name %q not found in %v", name, ns.mounts))
}
func (ns *namespace) ResolveToMountTableX(ctx context.T, name string, opts ...naming.ResolveOpt) (*naming.MountEntry, error) {
