madara
madara copied to clipboard
RPC cache system
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
- Feature
What is the current behavior?
In the current implementation of the madara client, we fetch block data by going through the generated logs of our pallet. As the block data is important and we need fast retrieval, it seems important to:
- Properly store the block data in a dedicated
StorageValue
in our pallet - When possible retrieve the block data through storage query instead of digest API
- Add a cache layer on top of it to ease retrieval of recurrent queried data
Resolves: #947
What is the new behavior?
Right now, the update to the code base are as follow:
- Added a
CurrentBlock
type in the starknet pallet, representing aStorageValue
containing our block data. Also added a getter to retrieve it through our runtime API. - Added necessary method over our storage implementation to both retrieve the data through the runtime API and directly from our storage.
- Implemented a base LRUCache limited by allocated memory, along with an asynchronous task responsible for the handling of the cache and retrieving data.
- Updated the necessary calls in our RPC implementation, to use the cache task instead of directly retrieving the block data through our logs.
- Added an argument over the run command to set the allocated memory size limit for the cache.
The new flow is:
- Run a madara run while specifying
--starknet-log-block-cache-size
. For now, the default value is 200 (most likely will cache little if not nothing) - On RPC call, we will send the requested block hash to our data cache task.
- If the data cache has the block, returns it.
- Otherwise, we will directly query our storage for it.
Does this introduce a breaking change?
No
Other information
Original issue: #947
Codecov Report
Patch coverage: 15.93%
and project coverage change: -0.35%
:warning:
Comparison is base (
a6f6554
) 40.71% compared to head (b63d1a8
) 40.36%. Report is 1 commits behind head on main.
:exclamation: Current head b63d1a8 differs from pull request most recent head 59b7c99. Consider uploading reports for the commit 59b7c99 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #1083 +/- ##
==========================================
- Coverage 40.71% 40.36% -0.35%
==========================================
Files 96 98 +2
Lines 11069 11281 +212
Branches 11069 11281 +212
==========================================
+ Hits 4507 4554 +47
- Misses 6050 6209 +159
- Partials 512 518 +6
Files Changed | Coverage Δ | |
---|---|---|
crates/client/rpc-core/src/lib.rs | 0.00% <ø> (ø) |
|
crates/client/rpc/src/cache/mod.rs | 0.00% <0.00%> (ø) |
|
crates/client/rpc/src/events/mod.rs | 22.22% <0.00%> (-0.20%) |
:arrow_down: |
crates/client/rpc/src/lib.rs | 0.00% <0.00%> (ø) |
|
crates/client/rpc/src/madara_backend_client.rs | 0.00% <ø> (ø) |
|
crates/client/storage/src/lib.rs | 0.00% <ø> (ø) |
|
crates/client/storage/src/overrides/mod.rs | 0.00% <0.00%> (ø) |
|
...client/storage/src/overrides/schema_v1_override.rs | 0.00% <0.00%> (ø) |
|
crates/node/src/cli.rs | 0.00% <0.00%> (ø) |
|
crates/node/src/command.rs | 0.00% <0.00%> (ø) |
|
... and 7 more |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @tdelabro, here is a first draft for the RPC cache.
As we discussed under the issue, I've implemented the cache to work with allocated memory size instead of values size to make it more precise.
Right now, it seems to fail integration test, which is strange as it passes on my computer. I'll look into it.
The only problem that I have is that I think the benchmark tests to evaluate the cache efficiency are not there. I wanted to add some to try and measure it but I'm having trouble in identifying how to pull that off.
Could I ask for your input on that ? Ideally, I think that we want to try and spam a request on loop to our RPC methods that concern block fetch (e.g.: getblockWithTxHashes
)
Also, I'd be interested in your opinion of the choice I made to implement a new StorageValue
in pallet-starknet
. Does it seem like a fair choice to you?
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. This PR will be closed and locked in 7 days if no further activity occurs. Thank you for your contributions!
repository archived in favor of https://github.com/madara-alliance/madara