Use only one rpc client at time #612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 02562ef.
AndreiEres
changed the title
sandreim
approved these changes
Dec 12, 2023
essentials/src/api/executor.rs
Outdated
| } | ||
|
|
||
| impl RequestExecutorError { | ||
| pub fn should_repeat(&self) -> bool { |
There was a problem hiding this comment.
Suggested change
| pub fn should_repeat(&self) -> bool { | |
| pub fn should_retry(&self) -> bool { |
essentials/src/api/executor.rs
Outdated
|
|
||
| loop { | ||
| tokio::select! { | ||
| message = from_frontend.recv() => { |
There was a problem hiding this comment.
you don't really need a select! here, you can just let message = from_frontend.recv().await
| } | ||
| } | ||
|
|
||
| pub trait RequestExecutorNodes { |
There was a problem hiding this comment.
I am a bit slow to understand what does the String represent?
There was a problem hiding this comment.
is it the rpc node url ?
There was a problem hiding this comment.
Yes, It's a bit odd way to allow pass different types of node urls as an argument.
essentials/src/api/executor.rs
Outdated
| ) { | ||
| let client = match build_client(&url, api_client_mode, &self.retry).await { | ||
| Some(v) => v, | ||
| None => return, |
There was a problem hiding this comment.
Shouldn't we log an error here ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes have been made to improve the stability of the subxt client (including the light client mode). It's done in two specific parts.
Updated subxt client implementation
We used to spawn a client for a number of watched parachains or even more. Despite the memory and CPU usage it was a working approach until we decided to switch to a light client instead of a full RPC node. Spawning a light client for each parachain leads to overuse of resources and as a result - crashes the entire application.
What changed:
Improved rpc node requests
Implementing the new subxt client revealed a hidden problem with the number of requests. One client was unable to process all requests at the same speed as many clients and became a bottleneck. After investigation it was found that the most common request is to check the HRMP channels of a given parachain (about 80%).
We used to fetch the ingress and egress channel index of each parachain and then check each channel. It could be optimized because most of the attempts were empty, i.e. not every open channel was used to send a message in a given block.
What changed:
New implementation of HRMP channel requesting works well with a full RPC node, but light client can't hold that load. Since we currently only display HRMP metrics in CLI mode and don't send them to Prometheus, we decided to make them optional for a while. More work on improving HRMP fetching on the light client is coming.
Results
Processing of HRMP channels for one block, while watching all parachains in the Polkadot network.
Memory usage, 5 minutes while watching all parachains in the Polkadot network.