grpc_retry.WithMax(), how to use it properly? about go-grpc-middleware HOT 6 CLOSED

@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.

from go-grpc-middleware.

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")
}

from go-grpc-middleware.

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.

from go-grpc-middleware.

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

from go-grpc-middleware.

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

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

from go-grpc-middleware.

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

from go-grpc-middleware.