In process RPC service #7395
In process RPC service #7395
Conversation
fab-10
force-pushed
the
in-process-rpc-service-pre2
branch
from
1c24c62
to
1c8b066
Compare
There was a problem hiding this comment.
LGTM. Minor comments and suggestions which could be done in follow up PRs if you decide.
Appreciate the good PR description, test coverage and the way you've broken up the PR 👍
Great feature BTW, could be some interesting possibilities using this with the engine API.
| @@ -108,6 +112,7 @@ public class Runner implements AutoCloseable { | |||
| final Optional<GraphQLHttpService> graphQLHttp, | |||
| final Optional<WebSocketService> webSocketRpc, | |||
| final Optional<JsonRpcIpcService> ipcJsonRpc, | |||
| final Map<String, JsonRpcMethod> inProcessRpcMethods, | |||
There was a problem hiding this comment.
Comparing to JsonRpcHttpService, I think in-process is missing metrics, e.g. requestTimer.
Wonder If it's still useful to have separate metrics for these in-process RPC calls?
Also missing the opentelemetry tracing stuff, but not sure that if it makes sense to split the span in-process.
There was a problem hiding this comment.
good points, I think it is useful to have metrics, will review this in a separate PR
| import java.util.function.Function; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| /** The RPC endpoint service implementation. */ | ||
| public class RpcEndpointServiceImpl implements RpcEndpointService { | ||
| private final Map<String, Function<PluginRpcRequest, ?>> rpcMethods = new HashMap<>(); | ||
| private Map<String, JsonRpcMethod> inProcessRpcMethods; |
There was a problem hiding this comment.
Is this compatible with RPCs added via plugins as well, as per #2754?
I'm not 100% sure if this scenario makes sense, but perhaps we'd want to call a plugin's RPC method via another plugin, i.e.
- plugin1 registers rpc1 with besu
- plugin2 calls rpc1 in-process
There was a problem hiding this comment.
This a quick test, and found that it is possible to call a method defined in another plugin, but only if the plugin namespace is enabled via rpc-http-api, that's not optimal, since should be enough to enable it only using Xin-process-rpc-apis, will improve that on a following PR with proper tests
| @@ -48,6 +65,37 @@ public <T> void registerRPCEndpoint( | |||
| rpcMethods.put(namespace + "_" + functionName, function); | |||
| } | |||
|
|
|||
| @Override | |||
| public PluginRpcResponse call(final String methodName, final Object[] params) { | |||
There was a problem hiding this comment.
Should we LOG.trace the methodName and params here?
fab-10
force-pushed
the
in-process-rpc-service-pre2
branch
from
1c8b066
to
26ba456
Compare
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10
force-pushed
the
in-process-rpc-service-pre2
branch
from
26ba456
to
c9c0b39
Compare
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: gconnect <agatevureglory@gmail.com>
PR description
Expose a way to call RPC method in process, so they can be invoked by plugins without the overhead of the network (or native socket) and the json (de)serialization.
The interface is still fragile, because RPC method inputs and outputs are not typed, but just Object, so a next step will be to make them typed so plugins will discover at compile time that something is broken, but that will be address in another PR.
Do not be scared by the amount of file changed since many are due to the move of some classes in the plugin-api module.
This feature is disabled by default and can be enabled setting the new experimental flag
Xin-process-rpc-enabled=true, and to enabled specific RPC namespaces use the new experimental optionXin-process-rpc-apiswith a comma separated list of namespaces.Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests