venice icon indicating copy to clipboard operation
venice copied to clipboard

[server] Rewrite gRPC read Service to align with Netty-Based HTTP Server

Open sushantmane opened this issue 1 year ago • 2 comments

Rewrite gRPC read Service to align with Netty-Based HTTP Server

  • Refactored the Netty handler to enable code reuse for gRPC handlers
  • Added missing APIs to the gRPC service to close feature gaps with the HTTP implementation.
  • Fixed the gRPC I/O request path threading model: previously, requests were handled by the gRPC executor thread pool, but now they are processed
    by the StorageReadExecutor thread pool for better performance.
    • Deprecated the old get and multiGet gRPC services, which relied on a single schema and encouraged tightly coupled code. This refactor paves the way for implementing server-side streaming support for batch get requests more easily.
  • Simplified the request processing logic by replacing the chain of responsibility pattern with a single processor class.

How was this PR tested?

WIP (UTs and E2E will be added)

Does this PR introduce any user-facing changes?

  • [x] No. You can skip the rest of this section.
  • [ ] Yes. Make sure to explain your proposed changes and call out the behavior change.

sushantmane avatar Sep 09 '24 23:09 sushantmane

I'm still working on client side changes to support streaming within a single route. I've figured out structure for it and working on changes

sushantmane avatar Oct 17 '24 18:10 sushantmane

I'm still working on client side changes to support streaming within a single route. I've figured out structure for it and working changes

Nice, looking forward to it. When you're ready with your next iteration, please rebase, push it up and LMK. I'll take another look.

FelixGV avatar Oct 17 '24 19:10 FelixGV

Hi @sushantmane Do you still have plan to merge this change? It seems to be a good change, but it is too large and do you have plan to split it into multiple ones?

gaojieliu avatar Jun 16 '25 16:06 gaojieliu

I'll get back to this after PubSub migration.

sushantmane avatar Aug 13 '25 17:08 sushantmane

I'll get back to this after PubSub migration.

sushantmane avatar Aug 13 '25 17:08 sushantmane