Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

collation-generation + collator-protocol: collate on multiple assigned cores #3795

Merged
merged 31 commits into from
Mar 27, 2024
Merged

collation-generation + collator-protocol: collate on multiple assigned cores #3795

merged 31 commits into from
Mar 27, 2024

Conversation

sandreim
Copy link
Contributor

@sandreim sandreim commented Mar 22, 2024 •
edited
Loading

This works only for collators that implement the collator_fn allowing collation-generation subsystem to pull collations triggered on new heads.

Also enables request_v2::CollationFetchingResponse::CollationWithParentHeadData for test adder/undying collators.

TODO:

  • fix tests
  • new tests
  • PR doc

@sandreim
DistributeCollation includes CoreIndex
8f5d259 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
SubmitCollationParams includes CoreIndex
dc84796 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
collator-protocol updates
665ef06 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
collation-generation pull support for chained collations
78ee8b2 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
adjust validator buffer capacity for elastic scaling
a61d64a 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
fmt
1cbb090 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Per core index tracking of multiple collations
493a7a2 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
fix tests
8497413 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
enable collator-protocol elastic scaling extension for adder colaltor
0dbc63a 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Use backing state to get candidates pending availability
3906d34 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Merge branch 'master' of github.com:paritytech/polkadot-sdk into sand…
eb7d842 
…reim/collate_on_multiple_cores

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
use ok_or
d344a2d 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
subsystem-util request_para_backing_state
16e1cb4 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Use the exposed claim queue
91f9c40 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim changed the title collation-generation: collate on multiple assigned cores collation-generation + collator-protoocl: collate on multiple assigned cores Mar 22, 2024
@sandreim sandreim changed the title collation-generation + collator-protoocl: collate on multiple assigned cores collation-generation + collator-protocol: collate on multiple assigned cores Mar 22, 2024
@sandreim
refactor
154f3dd 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
remove type annotation
60ec79f 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
remove more merge damage
a6c4afe 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
make tests compile
62c13dc 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
happy clippy is good clippy
ddf057e 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
lookahead use core_index in SubmitCollationParams
13b149f 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim added the R0-silent Changes should not be mentioned in any release notes label Mar 25, 2024
@sandreim sandreim marked this pull request as ready for review March 25, 2024 11:51
@sandreim
Copy link
Contributor Author

Looks like CI gets stuck and it's this PR's fault: TRY 1 SLOW [>480.000s] test-parachain-undying-collator::integration collating_using_undying_collator TRY 1 SLOW [>540.000s] test-parachain-adder-collator::integration collating_using_adder_collator TRY 1 SLOW [>540.000s] test-parachain-undying-collator::integration collating_using_undying_collator

alindima
alindima approved these changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@alindima alindima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

cumulus/client/consensus/aura/src/collators/lookahead.rs Outdated Show resolved Hide resolved
cumulus/client/consensus/aura/src/collators/lookahead.rs Outdated Show resolved Hide resolved
polkadot/node/collation-generation/src/lib.rs Outdated Show resolved Hide resolved
polkadot/node/network/collator-protocol/src/collator_side/validators_buffer.rs Show resolved Hide resolved
@sandreim
Copy link
Contributor Author

For some weird reason finality halts

@sandreim
Copy link
Contributor Author

2024-03-25 15:05:01.006 WARN tokio-runtime-worker parachain::approval-voting: Failed to create assignment bitfield block_hash=0x7467bd478b3f36afa7be045b1ae79aef482410b8046335fb8d8f5f42fbea6ee8 err=NullAssignment

@sandreim
Copy link
Contributor Author

For some weird reason finality halts

#3826

@sandreim
review feedback
9721b69 
Signed-off-by: Andrei Sandu <[email protected]>
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5645876

@sandreim
await await
4eb804e 
Signed-off-by: Andrei Sandu <[email protected]>
eskimor
eskimor reviewed Mar 26, 2024
View reviewed changes
Copy link
Member

@eskimor eskimor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. My only real concern is: Do we have a breaking change here for Babe? If so, what can we do to make this as pain free as possible for parachains?

cumulus/client/consensus/aura/src/collators/lookahead.rs Show resolved Hide resolved
polkadot/node/network/collator-protocol/src/collator_side/mod.rs Show resolved Hide resolved
polkadot/node/network/collator-protocol/src/collator_side/mod.rs Outdated Show resolved Hide resolved
polkadot/node/network/collator-protocol/src/collator_side/mod.rs Outdated Show resolved Hide resolved
polkadot/node/primitives/src/lib.rs Show resolved Hide resolved
@sandreim sandreim mentioned this pull request Mar 26, 2024
Closed
@sandreim
Copy link
Contributor Author

sandreim commented Mar 26, 2024 •
edited
Loading

Looks good. My only real concern is: Do we have a breaking change here for Babe? If so, what can we do to make this as pain free as possible for parachains?

There is no change that touches Babe in this PR, SubmitCollationParams is a polkadot node side primitive

ordian
ordian approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good modulo missing PRDoc

cumulus/client/consensus/aura/src/collators/lookahead.rs Show resolved Hide resolved
@sandreim
review feedback
8b70bc2 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
prdoc
16607dd 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Merge branch 'master' of github.com:paritytech/polkadot-sdk into sand…
ba80669 
…reim/collate_on_multiple_cores
eskimor
eskimor approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member

@eskimor eskimor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

polkadot/node/network/collator-protocol/src/collator_side/mod.rs Outdated Show resolved Hide resolved
@sandreim
nice new syntax
69a63cc 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim enabled auto-merge March 27, 2024 14:39
@sandreim sandreim added this pull request to the merge queue Mar 27, 2024
Merged via the queue into master with commit 417c54c Mar 27, 2024
121 of 132 checks passed
@sandreim sandreim deleted the sandreim/collate_on_multiple_cores branch March 27, 2024 15:08
@sandreim sandreim mentioned this pull request Mar 28, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Mar 29, 2024
@sandreim
collation-genereation: fix tests (#3883)
30ef865 
Somehow #3795 was merged
but tests are failing now on master. I suspect that CI is not even
running these tests anymore which is a big issue.

---------

Signed-off-by: Andrei Sandu <[email protected]>
pgherveou pushed a commit that referenced this pull request Apr 2, 2024
@sandreim @pgherveou
collation-genereation: fix tests (#3883)
bdcc95c 
Somehow #3795 was merged
but tests are failing now on master. I suspect that CI is not even
running these tests anymore which is a big issue.

---------

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim mentioned this pull request Apr 8, 2024
Closed
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
@sandreim @dharjeezy
collation-generation + collator-protocol: collate on multiple assigne…
e1a51b2 
…d cores (paritytech#3795)

This works only for collators that implement the `collator_fn` allowing
`collation-generation` subsystem to pull collations triggered on new
heads.

Also enables
`request_v2::CollationFetchingResponse::CollationWithParentHeadData` for
test adder/undying collators.

TODO:
- [x] fix tests
- [x] new tests
- [x] PR doc

---------

Signed-off-by: Andrei Sandu <[email protected]>
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
@sandreim @dharjeezy
collation-genereation: fix tests (paritytech#3883)
4f5d4c8 
Somehow paritytech#3795 was merged
but tests are failing now on master. I suspect that CI is not even
running these tests anymore which is a big issue.

---------

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim mentioned this pull request Apr 10, 2024
Closed
2 tasks
@alindima alindima mentioned this pull request May 31, 2024
Closed
@TDemeco TDemeco mentioned this pull request Sep 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skunert skunert skunert left review comments

@alindima alindima alindima approved these changes

@eskimor eskimor eskimor approved these changes

@ordian ordian ordian approved these changes

Assignees
No one assigned
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sandreim @paritytech-cicd-pr @eskimor @ordian @skunert @alindima