backing: improve session buffering for runtime information #6284

sw10pa · 2024-10-30T10:34:22Z

Issue

[#3421] backing: improve session buffering for runtime information

Description

In the current implementation of the backing module, certain pieces of information, which remain unchanged throughout a session, are fetched multiple times via runtime API calls. The goal of this task was to introduce a local cache to store such session-stable information and perform the runtime API call only once per session.

This PR implements caching specifically for the validators list, node features, executor parameters, minimum backing votes threshold, and validator-to-group mapping, which were previously fetched from the runtime or computed each time PerRelayParentState was built. Now, this information is cached and reused within the session.

TODO

Create a separate struct for per-session caches;
Cache validators list;
Cache node features;
Cache executor parameters;
Cache minimum backing votes threshold;
Cache validator-to-group mapping;
Update tests to reflect these changes;
Add prdoc.

For the next PR

Cache validator groups and any other session-stable data (if present).

alindima

Thank you! Looking good so far! Let's see what else we can cache 😄

polkadot/node/core/backing/src/lib.rs

…d caching for node_features, minimum_backing_votes and validator_to_group mapping

polkadot/node/core/backing/src/lib.rs

…with cached value

alindima

A couple of small suggestions left. Let's also add a prdoc. Thanks!

polkadot/node/core/backing/src/tests/mod.rs

polkadot/node/subsystem-util/src/lib.rs

polkadot/node/core/backing/src/tests/mod.rs

…refactor tests

eskimor

code looks good, but I would suggest to move the implementation to the general purpose one we already have.

eskimor · 2024-11-11T17:20:06Z

polkadot/node/core/backing/src/lib.rs

+	}
+}
+
+impl PerSessionCache {


Note that we already have such a cache:

polkadot-sdk/polkadot/node/subsystem-util/src/runtime/mod.rs

Line 75 in 4c618a8

pub struct RuntimeInfo {

I would suggest to adjust the existing implementation to cater to the needs here. For example, usage of multiple LRUs makes sense, so subsystems don't pay for what they don't use.

alindima · 2024-11-12T07:45:34Z

code looks good, but I would suggest to move the implementation to the general purpose one we already have.

@eskimor, I would suggest merging this PR first as it is, since there are already multiple subsystems doing their own caching and it was also a learning experience for @sw10pa as his first PR. The RuntimeInfo needs a redesign in order for it to be reusable by more subsystems. We can open an issue for it and be worked at a later time.

alindima · 2024-11-12T07:47:40Z

polkadot/node/core/backing/src/error.rs

@@ -105,6 +105,9 @@ pub enum Error {

 	#[error("Availability store error")]
 	StoreAvailableData(#[source] StoreAvailableDataError),
+
+	#[error("Data is not available")]
+	DataNotAvailable,


This doesn't say anything

For a temporary solution, I have changed the name and description of the error but, as discussed separately, will try to replace RuntimeApiErrors with Subsystem errors in a separate PR.

sandreim

Nice work @sw10pa ! I agree with @eskimor about adapting an existing solution and it should be fine to do that as part of larger refactor and diff PR as @alindima suggests

sandreim · 2024-11-12T08:23:30Z

polkadot/node/core/backing/src/lib.rs

+	}
+
+	/// Gets validators from the cache or fetches them from the runtime if not present.
+	async fn get_or_fetch_validators(


Given that this is a cache struct, the function names are overly verbose. I'd suggest just validators(), node_fetures(), executor_params.

feat(backing): add session buffering for validator list

77c71c6

sw10pa added I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T4-runtime_API This PR/Issue is related to runtime APIs. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Oct 30, 2024

sw10pa requested review from alindima and sandreim October 30, 2024 10:34

sw10pa self-assigned this Oct 30, 2024

sw10pa mentioned this pull request Oct 30, 2024

backing: improve session buffering for runtime information #3421

Closed

sw10pa linked an issue Oct 30, 2024 that may be closed by this pull request

backing: improve session buffering for runtime information #3421

Closed

sw10pa added T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Oct 30, 2024

alindima reviewed Oct 30, 2024

View reviewed changes

polkadot/node/core/backing/src/lib.rs Outdated Show resolved Hide resolved

refactor, feat(backing): split caching logic into separate struct, ad…

4d8dd50

…d caching for node_features, minimum_backing_votes and validator_to_group mapping

sw10pa requested a review from alindima November 4, 2024 23:24

alindima reviewed Nov 5, 2024

View reviewed changes

refactor(backing): refactor caching functions and update usages

22f274f

sw10pa requested a review from alindima November 6, 2024 19:17

sw10pa marked this pull request as ready for review November 6, 2024 19:17

sw10pa changed the title ~~[WIP] backing: improve session buffering for runtime information~~ backing: improve session buffering for runtime information Nov 7, 2024

sw10pa added 2 commits November 8, 2024 17:07

feat(backing): replace runtime API call for retrieving node features …

4f29762

…with cached value

feat(backing): add caching for executor_params

830166a

alindima approved these changes Nov 11, 2024

View reviewed changes

polkadot/node/core/backing/src/tests/mod.rs Show resolved Hide resolved

polkadot/node/subsystem-util/src/lib.rs Outdated Show resolved Hide resolved

polkadot/node/core/backing/src/tests/mod.rs Outdated Show resolved Hide resolved

sw10pa and others added 4 commits November 11, 2024 14:01

refactor(backing): use direct runtime API to get executor_params and …

4b1f371

…refactor tests

refactor(subsystem-util): rollback changes

bbe25d4

Merge branch 'master' into sw10pa/backing-session-buffering

4a7c946

docs(prdoc): add prdoc for PR #6284

74fc6d6

eskimor reviewed Nov 11, 2024

View reviewed changes

alindima reviewed Nov 12, 2024

View reviewed changes

sandreim approved these changes Nov 12, 2024

View reviewed changes

refactor(backing): change error and caching function names

f7898b7

alindima added this pull request to the merge queue Nov 13, 2024

Merged via the queue into master with commit e617d1d Nov 13, 2024
195 of 199 checks passed

alindima deleted the sw10pa/backing-session-buffering branch November 13, 2024 14:37

tdimitrov added a commit that referenced this pull request Nov 14, 2024

Fix merge conflicts from #6284

b873327

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backing: improve session buffering for runtime information #6284

backing: improve session buffering for runtime information #6284

sw10pa commented Oct 30, 2024 •

edited

Loading

alindima left a comment

alindima left a comment

eskimor left a comment

eskimor Nov 11, 2024

alindima commented Nov 12, 2024

alindima Nov 12, 2024

sw10pa Nov 12, 2024

sandreim left a comment

sandreim Nov 12, 2024

sw10pa Nov 12, 2024

backing: improve session buffering for runtime information #6284

backing: improve session buffering for runtime information #6284

Conversation

sw10pa commented Oct 30, 2024 • edited Loading

Issue

Description

TODO

For the next PR

alindima left a comment

Choose a reason for hiding this comment

alindima left a comment

Choose a reason for hiding this comment

eskimor left a comment

Choose a reason for hiding this comment

eskimor Nov 11, 2024

Choose a reason for hiding this comment

alindima commented Nov 12, 2024

alindima Nov 12, 2024

Choose a reason for hiding this comment

sw10pa Nov 12, 2024

Choose a reason for hiding this comment

sandreim left a comment

Choose a reason for hiding this comment

sandreim Nov 12, 2024

Choose a reason for hiding this comment

sw10pa Nov 12, 2024

Choose a reason for hiding this comment

sw10pa commented Oct 30, 2024 •

edited

Loading