We received a report that serde logic for gRPC requests/responses may occupy the event loop. Hence, some users may want this logic to be executed from blockingTaskExecutor if specified via GrpcServiceBuilder#useBlockingTaskExecutor.
Ideally, the general rule of thumb should be:
- Receiving requests: the thread invoking
ServerCall.Listener callbacks should be either the eventLoop (if useBlockingTaskExecutor = false) or the blocking executor (if useBlockingTaskExecutor = true). Request serde/decompression should be executed in this thread.
- Sending responses: the user decides which threads should invoke
ServerCall callbacks (e.g. sendHeaders, sendMessage, etc...). This thread is responsible for serde/compressions/etc.
@ikhoon proposed the following approach for simplicity:
The simplest way to achieve this is probably to just:
- Move request deserialization to after the blockingTaskExecutor reschedule
|
final boolean grpcWebText = GrpcSerializationFormats.isGrpcWebText(serializationFormat); |
|
request = marshaller.deserializeRequest(message, grpcWebText); |
|
maybeLogRequestContent(request); |
|
|
|
if (unsafeWrapRequestBuffers && buf != null && !grpcWebText) { |
|
GrpcUnsafeBufferUtil.storeBuffer(buf, request, ctx); |
|
} |
- Move response serialization to be performed before the reschedule:
e.g. the following will be called as soon as sendMessage is called before the reschedule. We should be aware of leaks though.
|
final HttpData responseBody = toPayload(responseMessage); |
|
public void sendMessage(O message) { |
ref: https://discord.com/channels/1087271586832318494/1087272728177942629/1372734948967972978
Note: Another idea I was developing was to provide a Executor variant which:
- guarantees sequential execution (so no two tasks execute at the same time, but ordering is not necessarily guaranteed based on submission time)
- is aware of reentry and does not perform rescheduling
This approach can also help clean up the logic and remove most of the conditional statements regarding blockingTaskExecutor.
However, this might take a while to develop, so this idea can be tackled later if needed.
We received a report that serde logic for gRPC requests/responses may occupy the event loop. Hence, some users may want this logic to be executed from
blockingTaskExecutorif specified viaGrpcServiceBuilder#useBlockingTaskExecutor.Ideally, the general rule of thumb should be:
ServerCall.Listenercallbacks should be either the eventLoop (ifuseBlockingTaskExecutor = false) or the blocking executor (ifuseBlockingTaskExecutor = true). Request serde/decompression should be executed in this thread.ServerCallcallbacks (e.g.sendHeaders,sendMessage, etc...). This thread is responsible for serde/compressions/etc.@ikhoon proposed the following approach for simplicity:
The simplest way to achieve this is probably to just:
armeria/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java
Lines 356 to 362 in a9e3e4c
e.g. the following will be called as soon as
sendMessageis called before the reschedule. We should be aware of leaks though.armeria/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnaryServerCall.java
Line 154 in a9e3e4c
armeria/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnaryServerCall.java
Line 120 in a9e3e4c
ref: https://discord.com/channels/1087271586832318494/1087272728177942629/1372734948967972978
Note: Another idea I was developing was to provide a
Executorvariant which:This approach can also help clean up the logic and remove most of the conditional statements regarding
blockingTaskExecutor.However, this might take a while to develop, so this idea can be tackled later if needed.