quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

gRPC client deadline config semantics #41947

Open nahguam opened 4 months ago

nahguam commented 4 months ago

Describe the bug

I believe the gRPC client deadline behaviour either does not match the intent, the documentation is incorrect or it needs clarifying.

The gRPC client deadline config says:

The deadline used for each call.

The code that applies this config calls the gRPC AbstractStub.withDeadlineAfter method which says:

Returns a new CallOptions with a deadline that is after the given duration from now.

The Quarkus documentation implies that each call will have a new separate deadline that commences when each call starts. However the actual behaviour is a single deadline that commences when the CallOptions are created. This results in every call after the deadline has been reached to immediately fail with DEADLINE_EXCEEDED. This effectively renders the stub bean useless after the single deadline has been reached. Have I missed something?

Expected behavior

Each call has a separate deadline starting at the beginning of each call.

Actual behavior

The stub has a single deadline that starts when the bean is created. After the deadline has expired the stub is unable to execute any requests because they all immediately fail with DEADLINE_EXCEEDED.

How to Reproduce?

  1. Create app - quarkus create app --extension=grpc --java=17
  2. Give the helloGrpc stub in HelloGrpcServiceTest a name - @GrpcClient("hello")
  3. Add a deadline in application.properties - quarkus.grpc.clients."hello".deadline=2s
  4. Edit the test HelloGrpcServiceTest.testHello() - repeat the call twice with a sleep longer than the deadline between them.
  @Test
  void testHello() throws Exception {
    HelloReply reply = helloGrpc
        .sayHello(HelloRequest.newBuilder().setName("Neo").build()).await()
        .atMost(Duration.ofSeconds(5));
    assertEquals("Hello Neo!", reply.getMessage());
    Thread.sleep(4_000);
    reply = helloGrpc
        .sayHello(HelloRequest.newBuilder().setName("Neo").build()).await()
        .atMost(Duration.ofSeconds(5));
    assertEquals("Hello Neo!", reply.getMessage());
  }
  1. Run the test.

Outcome:

The stub will be created on the first call which commences the deadline. This call will be within the deadline so will succeed. The second call (and all subsequent calls) will fail with DEADLINE_EXCEEDED

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 4 months ago

/cc @alesj (grpc), @cescoffier (grpc), @radcortez (config)

jbonofre commented 4 months ago

@cescoffier wdyt ? Imho it looks like a bug 😄

cescoffier commented 4 months ago

I believe it's a gRPC Java behavior. I'm not sure if we can workaround it. Their CallOptions is not something we can modify once created.

The Vert.x client is getting deadline support soon (but it's not there yet).

nahguam commented 4 months ago

It is the gRPC Java behaviour yes, but it's typically applied per-call, not at the stub level to be reused by multiple calls. What is the utility of providing the deadline config option at all? The config implies that it's applying the deadline to each call - should the doc clarify at least what it is actually doing?

cescoffier commented 4 months ago

Doc clarification is more than welcome, as I'm not sure we can do it per call.

Hopefully once the very x client support deadlines, we will switch the default to use the Vertx client/server as default

cescoffier commented 2 months ago

Discussing it with @Vietj, we need very.x 5 to handle this properly.

So, there is no much we can do until we switch to that version (2025).