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

Remove unnecessary mutexes in RPC server #646

Merged
merged 3 commits into from
Apr 26, 2022
Merged

Conversation

canepat
Copy link
Member

@canepat canepat commented Apr 22, 2022

This PR removes unnecessary synchronization code:

  • mutexes and scoped lockes in rpc::Server startup/shutdown and
  • shutdown thread in BackEnd and KV server main

Such a refactoring is possible because the BackEndKV server threading model changed since the synchronization introduction at the beginning of the implementation.

Specifically, each rpc::ServerContext now executes on its own threaded scheduler powered by asio::io_context, as opposed to a couple of threads per context in the first implementation. This simplifies the shutdown sequence which can be now started by the asio::io_context intercepting the termination signals and carried on by each rpc::ServerContext for its data structures (namely, grpc::CompletionQueue objects).

Remove unnecessary shutdown thread
@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #646 (a773873) into master (536cde7) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #646      +/-   ##
==========================================
+ Coverage   82.53%   82.58%   +0.04%     
==========================================
  Files         176      176              
  Lines       15239    15236       -3     
==========================================
+ Hits        12578    12583       +5     
+ Misses       2661     2653       -8     
Impacted Files Coverage Δ
node/silkworm/rpc/server_context_pool.cpp 93.75% <ø> (-0.13%) ⬇️
node/silkworm/rpc/server_context_pool.hpp 100.00% <ø> (+16.66%) ⬆️
node/silkworm/rpc/server.hpp 97.53% <100.00%> (-0.04%) ⬇️
node/silkworm/rpc/call.hpp 93.73% <0.00%> (+0.50%) ⬆️
core/silkworm/state/intra_block_state.cpp 97.46% <0.00%> (+0.94%) ⬆️
core/silkworm/state/in_memory_state.cpp 95.12% <0.00%> (+0.97%) ⬆️

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 536cde7...a773873. Read the comment docs.

@canepat canepat marked this pull request as ready for review April 22, 2022 16:51
@canepat canepat requested a review from mriccobene April 22, 2022 16:51
@canepat canepat merged commit 477d03f into master Apr 26, 2022
@canepat canepat deleted the remove_rpc_server_mutexes branch April 26, 2022 13:53
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