grpc_retry.WithMax(), how to use it properly?

[feliksik]

in this example, we see the following code:

func Example_deadlinecall() error {
	client := pb_testproto.NewTestServiceClient(cc)
	pong, err := client.Ping(
		newCtx(5*time.Second),
		&pb_testproto.PingRequest{},
		grpc_retry.WithMax(3),
		grpc_retry.WithPerRetryTimeout(1*time.Second))
	if err != nil {
		return err
	}
	fmt.Printf("got pong: %v", pong)
	return nil
}

But when I use one of those "option modifiers" as grpc_retry.WithMax() in a client call, it fails with a nullpointer exception. This is because the callOption wasn't set.

It seems to work well when I pass it as a modifier to the constructor of grpc_retry.UnaryClientInterceptor, such as in the test.

I do not yet fully understand the code architecture, and I don't know how a grpc.CallOption is to be usted. My question is: Am I overlooking something? Is the example just wrong? Or is the implementation wrong?

[mwitkow]

@feliksik In order to use the grpc_retry CallOptions within your code invocations, you need to use either the UnaryClient or ServerClient interceptors when you instantiate the connection (on grpc.Dial)

These interceptors are the things that actually drive the calling logic, and the CallOptions are just per-call overrides.

You spotted a bug that I have no immediate way of fixing. You see, the CallOptions interface in grpc itself has private methods. Which means one needs to resort to a hack:

• the grpc_retry.CallOption anonymously implements the grpc.CallOption interface. However it doesn't implement the private before and after functions, because it can't.
• the interceptors strip out these grpc_retry.CallOptions before passing them to the grpc.invoker call which relies on before and after being implemented.

As such, the code panics. Sadly there are two ways around this:

• either make the grpc_retry.CallOption use one of the Headers, Failfast, Peer or Trailers grpc.CallOptions as parents, and thus potentially disable the ability to use one of them by the user
• or work with upstream to implent grpc.CallOptions.Noop. I'll file a bug.

[mwitkow]

Let's see what upstream responds, this is a snippet of a test that will make it fail:

func (s *RetrySuite) TestCallOptionsDontPanicWithoutMiddleware() {
	s.srv.resetFailingConfiguration(3, codes.DataLoss, noSleep) // see retriable_errors
	nonMiddlewareClient := s.NewClient()
	_, err := nonMiddlewareClient.Ping(s.SimpleCtx(), goodPing,
		grpc_retry.WithMax(5),
		//grpc_retry.WithBackoff(grpc_retry.BackoffLinear(1*time.Millisecond)),
		//grpc_retry.WithCodes(codes.Aborted, codes.InvalidArgument),
		//grpc_retry.WithPerRetryTimeout(30*time.Millisecond),
	)
	require.NoError(s.T(), err, "the third invocation should succeed")
}

[feliksik]

Thanks for the clear response! I'll just make sure to use an interceptor in Dial() then.

When the NoopOption would be implemented upstream, I'm still not sure how the internals work to apply the grpc_retry.WithMax(5) magically to the correct interceptor... but I can work around that.

[mwitkow]

When the NoopOption is implemented, you will still need to have an interceptor implemented. A simple option is not going to be enough.

[mwitkow]

So it seems upstream has a PR for it:
grpc/grpc-go#1244

So there's hope for a proper fix! :)

[mwitkow]

Yup, there's a fix for it upstream, and we'll wait for the 1.4 gRPC Go release.