Introduce BackEnd gRPC server #565
Conversation
Add base unary grpc support Add EthBackEnd server [WIP]
Handle double explicit/implicit shutdown calls Drain completion queue in scheduler after shutdown Add unit tests
Rename flag
Codecov Report
@@ Coverage Diff @@
## master #565 +/- ##
==========================================
+ Coverage 81.16% 81.53% +0.37%
==========================================
Files 154 165 +11
Lines 13221 13624 +403
==========================================
+ Hits 10731 11109 +378
- Misses 2490 2515 +25
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Running node_test on this branch under AddressSanitizer fails on my MacBook:
address_sanitizer.log
On the master branch 7dc531d node_test works fine for me under ASAN.
That doesn't necessarily mean that this change introduces any memory issues, but I'd hate to lose the ability to run tests under sanitizers. Also perhaps we should prioritize and implement Issue #295. IMHO it's very important, but I have been postponing implementing it, mea culpa.
It seems the same problem reported in google/sanitizers#1317 and grpc/grpc#21838. I'm going to reproduce and try the suggested solutions (hope at least one works).
I agree we need to find a solution to avoid losing such ability. |
|
I've just tried to reproduce the issue on my Linux machine running Clang using libc++ with both ASAN and UB enabled, but stumbled upon this llvm/llvm-project#16778. Removing UB I've got a different error: |
|
I've added ASAN/TSAN suppression definitions in our CMake to avoid grpc/grpc#19224 without need to rebuild gRPC libraries as suggested in grpc/grpc#22325 |
canepat
changed the title
Add barebone gRPC server test
…ilkworm into backend_grpc_server
node/silkworm/rpc/backend_server.cpp
Outdated
|
|
||
| namespace silkworm::rpc { | ||
|
|
||
| inline types::H128* new_H128_from_bytes(const uint8_t* bytes) { |
There was a problem hiding this comment.
const uint8_t* is fine here because this function is only used internally. However, were we to make it public, I'd use gsl::span<const uint8_t, 16>.
There was a problem hiding this comment.
Ok, fair enough. There are also other Silkworm public APIs where such change should be considered, e.g. silkworm::endian
This PR adds:
Key points:
schedulerthread (implemented usingasio::io_context) is dedicated to process all the asynchronous notifications and handle RPC operations, another one (completion runner) waits indefinitely for the next completion tag from CQ and disptches the tag processor to the schedulerDetails:
This is a high-level class diagram (partial):
classDiagram class Server~AsyncService~{ <<abstract>> +run() +shutdown() #next_context() #request_calls()* } class BackEndServer~ETHBACKEND::AsyncService~{ #request_calls() } class RpcService~Rpc~{ <<abstract>> #add_request(Rpc) #remove_request(Rpc) } class BaseRpc{ <<abstract>> +process_done() #cleanup()* } class UnaryRpc~Service, Request, Response~{ +send_response(Response) -process_read() -process_finish() -cleanup() } class EtherbaseRpc{ <<typedef>> } class NetVersionRpc{ <<typedef>> } class EtherbaseService{ +create_rpc +process_rpc +cleanup_rpc } class NetVersionService{ +create_rpc +process_rpc +cleanup_rpc } class ServerContext{ } class asio_io_context{ } class grpc_ServerCompletionQueue{ } class CompletionRunner{ } class ServerContextPool{ +add_context() +run() +stop() +next_context() } Server <|-- BackEndServer BaseRpc <|-- UnaryRpc EtherbaseRpc ..> UnaryRpc : refine NetVersionRpc ..> UnaryRpc : refine EtherbaseService ..> EtherbaseRpc : create NetVersionService ..> NetVersionRpc : create RpcService <|-- EtherbaseService : refine RpcService <|-- NetVersionService : refine BackEndServer *-- "1" EtherbaseService BackEndServer *-- "1" NetVersionService ServerContext *-- "1" asio_io_context ServerContext *-- "1" grpc_ServerCompletionQueue ServerContext *-- "1" CompletionRunner ServerContextPool *-- "*" ServerContext Server *-- "1" ServerContextPool