Elastic scaling: use an assumed CoreIndex in candidate-backing
#3229
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
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
…reim/backing_multiple_cores_per_para
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
3 tasks
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
sandreim
added
R0-silent
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
alindima
approved these changes
Feb 14, 2024
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
eskimor
reviewed
Feb 14, 2024
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
eskimor
reviewed
Feb 14, 2024
| /// The validator groups at this relay parent. | ||
| validator_groups: Vec<Vec<ValidatorIndex>>, | ||
| /// The associated group rotation information. | ||
| group_rotation_info: GroupRotationInfo, |
There was a problem hiding this comment.
Those two are really per session. Storing them per relay parent is rather unnecessary. Seems fine for an interim solution though.
| if inject_core_index { | ||
| let core_index_to_inject: BitVec<u8, BitOrderLsb0> = | ||
| BitVec::from_vec(vec![core_index.0 as u8]); | ||
| validator_indices.extend(core_index_to_inject); |
There was a problem hiding this comment.
Shouldn't we be using some nice BackedCandidate function to be introduced in the other PR for that?
There was a problem hiding this comment.
that's a good idea, will do that in the follow-up PR
…ltiple_cores_per_para
Signed-off-by: alindima <alin@parity.io>
…ltiple_cores_per_para
alindima
force-pushed
the
sandreim/backing_multiple_cores_per_para
branch
from
7bd84bd
to
8398bb1
Compare
…ltiple_cores_per_para
eskimor
reviewed
Feb 21, 2024
eskimor
reviewed
Feb 21, 2024
| /// The validator groups at this relay parent. | ||
| validator_groups: Vec<Vec<ValidatorIndex>>, | ||
| /// The validator index -> group mapping at this relay parent. | ||
| validator_to_group: IndexedVec<ValidatorIndex, Option<GroupIndex>>, |
There was a problem hiding this comment.
Is it really necessary to copy this to the relay parent state? Can't we just keep the session index here and then fetch a reference from state directly where we need it?
There was a problem hiding this comment.
this being an LruMap, it makes it a bit harder to reason about the existence of the session index in the map.
And we can't store a mutable reference here and just populate the map when the session is not present because we'd need to hand out multiple mutable references.
Realistically, backing can only happen on the current session, right? So the LruMap doesn't need a capacity larger than 1 (so then, it doesn't even need to be a LruMap). Am I right?
There was a problem hiding this comment.
with current async backing code, we cannot start backing a candidate in the next session in advance. But we may want to keep this possibility, making the LruMap neccessary.
What I can do is wrap it in an Arc, because they don't need to modify it. Which would achieve the same
There was a problem hiding this comment.
done. I'll merge it once CI passes again
eskimor
approved these changes
Feb 21, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 23, 2024
…rent cores (#3231) Fixes #3144 Builds on top of #3229 ### Summary Some preparations for Runtime to support elastic scaling, guarded by config node features bit `FeatureIndex::ElasticScalingMVP`. This PR introduces a per-candidate `CoreIndex` but does it in a hacky way to avoid changing `CandidateCommitments`, `CandidateReceipts` primitives and networking protocols. #### Including `CoreIndex` in `BackedCandidate` If the `ElasticScalingMVP` feature bit is enabled then `BackedCandidate::validator_indices` is extended by 8 bits. The value stored in these bits represents the assumed core index for the candidate. It is temporary solution which works by creating a mapping from `BackedCandidate` to `CoreIndex` by assuming the `CoreIndex` can be discovered by checking in which validator group the validator that signed the statement is. TODO: - [x] fix tests - [x] add new tests - [x] Bump runtime API for Kusama, so we have that node features thing! -> polkadot-fellows/runtimes#194 --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: alindima <alin@parity.io> Co-authored-by: alindima <alin@parity.io>
eskimor
pushed a commit
that referenced
this pull request
Feb 23, 2024
…3229) First step in implementing #3144 - switch statement `Table` candidate mapping from `ParaId` to `CoreIndex` - introduce experimental `InjectCoreIndex` node feature. - determine and assume a `CoreIndex` for a candidate based on statement validator index. If the signature is valid it means validator controls the validator that index and we can easily map it to a validator group/core. - introduce a temporary provisioner fix until we fully enable elastic scaling in the subystem. The fix ensures we don't fetch the same backable candidate when calling `get_backable_candidate` for each core. TODO: - [x] fix backing tests - [x] fix statement table tests - [x] add new test --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: alindima <alin@parity.io> Co-authored-by: alindima <alin@parity.io>
eskimor
pushed a commit
that referenced
this pull request
Feb 23, 2024
…rent cores (#3231) Fixes #3144 Builds on top of #3229 Some preparations for Runtime to support elastic scaling, guarded by config node features bit `FeatureIndex::ElasticScalingMVP`. This PR introduces a per-candidate `CoreIndex` but does it in a hacky way to avoid changing `CandidateCommitments`, `CandidateReceipts` primitives and networking protocols. If the `ElasticScalingMVP` feature bit is enabled then `BackedCandidate::validator_indices` is extended by 8 bits. The value stored in these bits represents the assumed core index for the candidate. It is temporary solution which works by creating a mapping from `BackedCandidate` to `CoreIndex` by assuming the `CoreIndex` can be discovered by checking in which validator group the validator that signed the statement is. TODO: - [x] fix tests - [x] add new tests - [x] Bump runtime API for Kusama, so we have that node features thing! -> polkadot-fellows/runtimes#194 --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: alindima <alin@parity.io> Co-authored-by: alindima <alin@parity.io>
eskimor
pushed a commit
that referenced
this pull request
Feb 26, 2024
…3229) First step in implementing #3144 ### Summary of changes - switch statement `Table` candidate mapping from `ParaId` to `CoreIndex` - introduce experimental `InjectCoreIndex` node feature. - determine and assume a `CoreIndex` for a candidate based on statement validator index. If the signature is valid it means validator controls the validator that index and we can easily map it to a validator group/core. - introduce a temporary provisioner fix until we fully enable elastic scaling in the subystem. The fix ensures we don't fetch the same backable candidate when calling `get_backable_candidate` for each core. TODO: - [x] fix backing tests - [x] fix statement table tests - [x] add new test --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: alindima <alin@parity.io> Co-authored-by: alindima <alin@parity.io>
eskimor
pushed a commit
that referenced
this pull request
Feb 26, 2024
…rent cores (#3231) Fixes #3144 Builds on top of #3229 ### Summary Some preparations for Runtime to support elastic scaling, guarded by config node features bit `FeatureIndex::ElasticScalingMVP`. This PR introduces a per-candidate `CoreIndex` but does it in a hacky way to avoid changing `CandidateCommitments`, `CandidateReceipts` primitives and networking protocols. #### Including `CoreIndex` in `BackedCandidate` If the `ElasticScalingMVP` feature bit is enabled then `BackedCandidate::validator_indices` is extended by 8 bits. The value stored in these bits represents the assumed core index for the candidate. It is temporary solution which works by creating a mapping from `BackedCandidate` to `CoreIndex` by assuming the `CoreIndex` can be discovered by checking in which validator group the validator that signed the statement is. TODO: - [x] fix tests - [x] add new tests - [x] Bump runtime API for Kusama, so we have that node features thing! -> polkadot-fellows/runtimes#194 --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: alindima <alin@parity.io> Co-authored-by: alindima <alin@parity.io>
This was referenced Feb 26, 2024
Merged
This was referenced Jun 5, 2024
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.
First step in implementing #3144
Summary of changes
Tablecandidate mapping fromParaIdtoCoreIndexInjectCoreIndexnode feature.CoreIndexfor a candidate based on statement validator index. If the signature is valid it means validator controls the validator that index and we can easily map it to a validator group/core.get_backable_candidatefor each core.TODO: