Introduce basic slot-based collator #4097
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
skunert
added
T0-node
1 task
pepoviola
reviewed
Jun 21, 2024
pepoviola
reviewed
Jun 21, 2024
pepoviola
reviewed
Jun 21, 2024
Co-authored-by: Javier Viola <363911+pepoviola@users.noreply.github.com>
bkchr
approved these changes
Jun 27, 2024
| /// If this data has been fetched in the past for the incoming hash, it will reuse | ||
| /// cached data. | ||
| pub async fn get_relay_chain_data(&mut self) -> Result<RelayChainData, ()> { | ||
| let Ok(relay_parent) = self.relay_client.best_block_hash().await else { |
There was a problem hiding this comment.
A problem with this could be that if the relay chain block comes too fast, we may did not use all the available cores of the parent and then we are wasting one core. Not really required for this pull request, but we should make this more robust in the future.
davxy
approved these changes
Jul 3, 2024
Comment on lines
233
to
250
| let included_header = relay_client | ||
| .persisted_validation_data(relay_parent, para_id, OccupiedCoreAssumption::TimedOut) | ||
| .await?; | ||
| let included_header = match included_header { | ||
| Some(pvd) => pvd.parent_head, | ||
| None => return Ok(None), // this implies the para doesn't exist. | ||
| }; | ||
|
|
||
| // Fetch the pending header from the relay chain. | ||
| let pending_pvd = relay_client | ||
| .persisted_validation_data(relay_parent, para_id, OccupiedCoreAssumption::Included) | ||
| .await? | ||
| .and_then(|x| if x.parent_head != included_header { Some(x.parent_head) } else { None }); | ||
|
|
||
| let included_header = match B::Header::decode(&mut &included_header.0[..]).ok() { | ||
| None => return Ok(None), | ||
| Some(x) => x, | ||
| }; |
There was a problem hiding this comment.
@skunert I see. Thank you for your good explaination. This was super useful.
One consideration. If OccupiedCoreAssumption is Included all the pending candidates for the para are considered enacted right? (i.e. in practice the "relay-chain vision" of the para pending candidates in its unincluded segment),
|
|
||
| tracing::trace!(target: PARENT_SEARCH_LOG_TARGET, ?included_hash, included_num = ?included_header.number(), ?pending_hash , ?rp_ancestry, "Searching relay chain ancestry."); | ||
| while let Some(entry) = frontier.pop() { | ||
| let is_pending = pending_hash.as_ref().map_or(false, |h| &entry.hash == h); |
There was a problem hiding this comment.
Doesn't this condition apply to all the elements in the maybe_route_to_last_pending?
And thus, also to the note: below at line 344
cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs
Outdated
Show resolved
Hide resolved
cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
|
bot fmt |
|
@skunert https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6630012 was started for your command Comment |
|
@skunert Command |
TomaszWaszczyk
pushed a commit
to TomaszWaszczyk/polkadot-sdk
that referenced
this pull request
Jul 7, 2024
Part of paritytech#3168 On top of paritytech#3568 ### Changes Overview - Introduces a new collator variant in `cumulus/client/consensus/aura/src/collators/slot_based/mod.rs` - Two tasks are part of that module, one for block building and one for collation building and submission. - Introduces a new variant of `cumulus-test-runtime` which has 2s slot duration, used for zombienet testing - Zombienet tests for the new collator **Note:** This collator is considered experimental and should only be used for testing and exploration for now. ### Comparison with `lookahead` collator - The new variant is slot based, meaning it waits for the next slot of the parachain, then starts authoring - The search for potential parents remains mostly unchanged from lookahead - As anchor, we use the current best relay parent - In general, the new collator tends to be anchored to one relay parent earlier. `lookahead` generally waits for a new relay block to arrive before it attempts to build a block. This means the actual timing of parachain blocks depends on when the relay block has been authored and imported. With the slot-triggered approach we are authoring directly on the slot boundary, were a new relay chain block has probably not yet arrived. ### Limitations - Overall, the current implementation focuses on the "happy path" - We assume that we want to collate close to the tip of the relay chain. It would be useful however to have some kind of configurable drift, so that we could lag behind a bit. paritytech#3965 - The collation task is pretty dumb currently. It checks if we have cores scheduled and if yes, submits all the messages we have received from the block builder until we have something submitted for every core. Ideally we should do some extra checks, i.e. we do not need to submit if the built block is already too old (build on a out of range relay parent) or was authored with a relay parent that is not an ancestor of the relay block we are submitting at. paritytech#3966 - There is no throttling, we assume that we can submit _velocity_ blocks every relay chain block. There should be communication between the collator task and block-builder task. - The parent search and ConsensusHook are not yet properly adjusted. The parent search makes assumptions about the pending candidate which no longer hold. paritytech#3967 - Custom triggers for block building not implemented. --------- Co-authored-by: Davide Galassi <davxy@datawok.net> Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: Javier Viola <363911+pepoviola@users.noreply.github.com> Co-authored-by: command-bot <>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 17, 2024
Resolves #4468 Gives instructions on how to enable elastic scaling MVP to parachain teams. Still a draft because it depends on further changes we make to the slot-based collator: #4097 Parachains cannot use this yet because the collator was not released and no relay chain network has been configured for elastic scaling yet
paritytech-ci
pushed a commit
that referenced
this pull request
Jul 17, 2024
Resolves #4468 Gives instructions on how to enable elastic scaling MVP to parachain teams. Still a draft because it depends on further changes we make to the slot-based collator: #4097 Parachains cannot use this yet because the collator was not released and no relay chain network has been configured for elastic scaling yet
Jpserrat
pushed a commit
to Jpserrat/polkadot-sdk
that referenced
this pull request
Jul 18, 2024
Resolves paritytech#4468 Gives instructions on how to enable elastic scaling MVP to parachain teams. Still a draft because it depends on further changes we make to the slot-based collator: paritytech#4097 Parachains cannot use this yet because the collator was not released and no relay chain network has been configured for elastic scaling yet
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.
Part of #3168
On top of #3568
Changes Overview
cumulus/client/consensus/aura/src/collators/slot_based/mod.rscumulus-test-runtimewhich has 2s slot duration, used for zombienet testingNote: This collator is considered experimental and should only be used for testing and exploration for now.
Comparison with
lookaheadcollatorlookaheadgenerally waits for a new relay block to arrive before it attempts to build a block. This means the actual timing of parachain blocks depends on when the relay block has been authored and imported. With the slot-triggered approach we are authoring directly on the slot boundary, were a new relay chain block has probably not yet arrived.Limitations