Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Refactor RollingSessionWindow into a subsystem #6144

Closed
sandreim opened this issue Oct 12, 2022 · 2 comments · Fixed by #7123
Closed

Refactor RollingSessionWindow into a subsystem #6144

sandreim opened this issue Oct 12, 2022 · 2 comments · Fixed by #7123
Labels
I8-refactor Code needs refactoring. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.

Comments

@sandreim
Copy link
Contributor

sandreim commented Oct 12, 2022

The RollingSessionWindow caches session information for a configurable number of sessions and advances the window when a new session is detected (block import). It is currently used by two consensus components: approval-voting and dispute-coordinator. These care about the session information of an arbitrary number of past sessions, potentially up to the session of the last finalized block.

We introduced DB persistence in this PR: #6106 such that past session information is available even when the node is restarted or when Runtime session information is no longer available - pruned/expired.

Usage from multiple subsystems

The main goal of the current design is providing the invariant of having same session info across the entire pipeline -> we can always dispute anything that we previously backed. We read the DB at node startup and only update it on session change from both subsystems via the RollingSessionWindow inner logic. We also have the guarantee of eventual consistency of overseer signal processing (on ActiveLeavesUpdate detecting the new session) in approval-voting and dispute-coordinator so we should hardly ever store stale data in the DB, even for a short period of time. However, in extreme circumstances, like for example one subsystem getting stuck we would have probably an older window saved to DB, but that's something which is immediately recovered on node startup, assuming the node doesn't go offline for too long (which is something that will force fetching sessions from chain state in any approach).

Refactoring as it’s own subsystem

To improve on the current design, we should move the implementation of the RollingSessionWindow into it’s own subsystem which would be the single source of truth. This is much better than having this logic happening in 2 (or more eventually) subsystems. The new subsystem should allow others to retrieve the earliest session and all of the sessions stored in the window.

For efficiency approval-voting, dispute-coordinator and possible other subsystems must use a local cache of the session window which is populated at startup and refresh the cache on each new session. This means that when starting up these subsystems should not start processing their messages until the RollingSessionWindow has started and sessions window caches have been filled.

@sandreim sandreim added I8-refactor Code needs refactoring. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. labels Oct 12, 2022
@eskimor
Copy link
Member

eskimor commented Oct 13, 2022

I would not say "must" use a local cache, but they can and we have one readily available, it would need to be modified to retrieve sessions from the "rolling session window" subsystem instead of from the runtime-api subystem directly.

@eskimor
Copy link
Member

eskimor commented Dec 30, 2022

Related: dispute-distribution is based on RuntimeInfo cache and not on RollingSessionWindow. This is actually a week spot, if RollingSessionWindow is made more robust against runtime failures via db, importing votes would still fail if they come via distribution, if dispute-distribution cannot fetch the SessionInfo for some reason.

This has never been an issue before though. I believe because dispute-distribution does not rely on a complete window being available, but will continue just fine if the currently needed session is available. It just uses an LRU with the session window size.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I8-refactor Code needs refactoring. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants