Skip to content
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

Add KV RPC services #610

Merged
merged 15 commits into from
Mar 18, 2022
Merged

Add KV RPC services #610

merged 15 commits into from
Mar 18, 2022

Conversation

canepat
Copy link
Member

@canepat canepat commented Mar 9, 2022

Add bidirectional-streaming RPC support
Refactor RPC Server to handle many async services
Add structure for KV RPC services
Add KV RPC empty implementations
Refactor ServerContextPool to separate start from join

canepat added 7 commits March 9, 2022 21:44
Refactor Server to handle many async services
Add structure for KV RPC services
Add KV RPC empty implementations
Refactor ServerContextPool to separate start from join
Regenerate gRPC headers and sources
Patch KV generated header for Protobuf DELETE issue
Log gRPC version in BackEndKv standalone server
Regenerate gRPC headers and sources
Capture gRPC logging in BackEndKv standalone server
Register all first RPC requests on the same context [WIP]
Track total server-side RPC instances
Add peer tracing in server-side RPC services
Fix bidirectional streaming call state machine
Improve gRPC log messages
Remove temporary hack from CMake file
@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #610 (f60b1ee) into master (5f079ca) will increase coverage by 0.14%.
The diff coverage is 91.53%.

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
+ Coverage   81.75%   81.90%   +0.14%     
==========================================
  Files         167      170       +3     
  Lines       13947    14215     +268     
==========================================
+ Hits        11403    11643     +240     
- Misses       2544     2572      +28     
Impacted Files Coverage Δ
node/silkworm/rpc/server_context_pool.hpp 0.00% <0.00%> (ø)
node/silkworm/rpc/server_context_pool.cpp 75.24% <50.00%> (-13.89%) ⬇️
node/silkworm/rpc/util.hpp 91.66% <77.77%> (+13.09%) ⬆️
node/silkworm/rpc/call.hpp 92.90% <88.40%> (-2.63%) ⬇️
node/silkworm/rpc/server.hpp 92.22% <95.23%> (-7.78%) ⬇️
node/silkworm/rpc/backend_factories.cpp 100.00% <100.00%> (ø)
node/silkworm/rpc/backend_kv_server.cpp 100.00% <100.00%> (ø)
node/silkworm/rpc/backend_kv_server.hpp 100.00% <100.00%> (ø)
node/silkworm/rpc/factory.hpp 100.00% <100.00%> (ø)
node/silkworm/rpc/kv_factories.cpp 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f079ca...f60b1ee. Read the comment docs.

@canepat canepat marked this pull request as ready for review March 17, 2022 08:34
@canepat canepat requested a review from mriccobene March 17, 2022 08:35
Copy link
Member

@mriccobene mriccobene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should consider renaming silkworm::rpc::RpcService: the current naming causes confusion with gRPC Service, which is a well-known concept in the gRPC model, reflected also in generated code, i.e. Service and AsyncService generated classes.

@canepat
Copy link
Member Author

canepat commented Mar 17, 2022

Yes, you're right. Considering that the Service classes act mainly as factories of RPC calls, I'm going to rename silkworm::rpc::RpcService as silkworm::rpc::Factory and all the hierarchy as well.

Copy link
Member

@mriccobene mriccobene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@canepat canepat merged commit 0b5e74a into master Mar 18, 2022
@canepat canepat deleted the backend_kv_server3 branch March 18, 2022 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants