-
Notifications
You must be signed in to change notification settings - Fork 237
[Feat] new server adapter #482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Move grpc_core/lib/grpc/http2 to grpc_core/lib/grpc/transport/http2 to better reflect that HTTP/2 is the transport layer for gRPC. Changes: - Rename GRPC.HTTP2.* to GRPC.Transport.HTTP2.* - Update all imports and aliases in grpc_server and grpc_client - Update all test files
Add detailed test coverage for HTTP/2 frame implementations in the frame/ directory, focusing on gRPC-specific use cases and edge cases. These tests cover gRPC-specific HTTP/2 scenarios including trailers-only responses, large message handling, connection keepalive, and flow control patterns commonly used in gRPC implementations.
Add detailed test coverage for HTTP/2 frame implementations in the frame/ directory, focusing on gRPC-specific use cases and edge cases. These tests cover gRPC-specific HTTP/2 scenarios including trailers-only responses, large message handling, connection keepalive, and flow control patterns commonly used in gRPC implementations.
HTTP/2 protocol (RFC 9113) requires that HEADERS frame with :status pseudo-header must be sent before any TRAILERS frame. The previous implementation was conditionally skipping HEADERS when a stream had already received END_STREAM from the client, causing protocol errors. This fix ensures that send_grpc_error ALWAYS sends HTTP/2 HEADERS (with required :status and :content-type headers) before sending TRAILERS (with grpc-status and grpc-message), regardless of the stream state (half-closed remote or not). This resolves the 'timeout_on_sleeping_server' interop test failure where Gun client was rejecting error responses with the message: 'A required pseudo-header was not found'.
ThousandIsland Adapter Implementation SummaryThis PR introduces a pure Elixir server adapter for gRPC using ThousandIsland, providing an alternative to the Cowboy adapter. Implementation Checklist
Test Coveragegrpc_core Package
grpc_client Package
grpc_server Package
Interop Test Suite
Total test count across all packages: 740+ tests Architecture OverviewHandler StructureConnection State MachineAsync Response ModelThe ThousandIsland adapter uses asynchronous message passing for non-blocking response sending (for bidi-streaming):
Compatibility
Performance Characteristics
Benchmark ResultsPerformance Comparison (default configuration: 1,000 requests)Cowboy Adapter:
ThousandIsland Adapter:
Summary:
Interop Test Stability
Memory Characteristics
This implementation provides a solid foundation for pure Elixir gRPC servers with excellent test coverage (740+ tests) and full protocol compliance. The ThousandIsland adapter is experimental but with strong capabilities and can serve as a drop-in replacement for the Cowboy adapter. |
polvalente
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking on the GRPC.Server.Stream opts bug
| @@ -0,0 +1,259 @@ | |||
| defmodule GRPC.Server.HTTP2.FrameTest do | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray file
…rpc/grpc into feat/new-server-adapter
This commit reduces a bit the general performance but increase correctness
Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>
…rpc/grpc into feat/new-server-adapter
Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>
…rpc/grpc into feat/new-server-adapter
|
I have an umbrella app with different apps starting their own ** (Mix) Could not start application discovery: Discovery.Application.start(:normal, []) returned an error: shutdown: failed to start child: GRPC.Server.Supervisor
** (EXIT) shutdown: failed to start child: GRPC.Server.StreamTaskSupervisor
** (EXIT) already started: #PID<0.871.0>
The There is a nearly identical one in another app in the same umbrella but with a different endpoint, port, etc. So there are multiple GRPC servers and multiple Server.Supervisors running in the same BEAM instance. This works with current releases of the grpc library, as well as with the |
This is something we already mapped out during code review yesterday. This branch is very much a work in progress. |
Understood, and that's fine. If early testing is not wanted / needed, just say so and I'll happily come back later in the process. |
@aseigo thank you very much! I think testing, specially external, is gonna be important when we move from the current state of flux we're in. For instance, we're trying to refactor the process structure to ensure correctness, but given how much this might impact performance this might take a bit of trial and error. We do appreciate your contributions a lot! |
Feel free to ping me when you are at that point and I'll kick some of the tires and look more closely at the implementation as well at that point. Cheers! |
Hi everyone, I'd like to talk a little about what I'm currently working on here.
I'm almost finished implementing a new server-side adapter entirely based on Elixir using the
thousand_islandlibrary. And the results are promising, as we can see below.Performance Metrics
Analysis
Advantages of ThousandIsland:
Processed more than double the number of requests in the same test period.
Used 45% less CPU time (user time).
Completed the tests in almost half the time
The ThousandIsland adapter demonstrates substantially superior performance compared to
Cowboyacross all measured aspects. The implementation delivers:This is still a draft and needs a lot of refinement. I opened the PR just to document the work and share it with everyone.