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.

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

Look-ahead collator: build on all assigned cores #3837

Closed
Tracked by #1829
sandreim opened this issue Mar 26, 2024 · 6 comments
Closed
Tracked by #1829

Look-ahead collator: build on all assigned cores #3837

sandreim opened this issue Mar 26, 2024 · 6 comments
Labels
I5-enhancement An additional feature request. T9-cumulus This PR/Issue is related to cumulus.

Comments

@sandreim
Copy link
Contributor

#3795 introduces some changes that allow the look ahead collator to specify which core a collation should be backed on. We need to implement something similar to what that PR does for the test collators which use collation pull via collator_fn.

@sandreim sandreim added I5-enhancement An additional feature request. T9-cumulus This PR/Issue is related to cumulus. labels Mar 26, 2024
@bkchr
Copy link
Member

bkchr commented Mar 26, 2024

The lookahead collator is going out of service, quite soon. I don't think we should invest anymore time to fix it.

@sandreim
Copy link
Contributor Author

I agree with you, but would love to hear some deadline on when #3168 is implemented such that lookahead collator is not needed anymore. Also this is a breaking change and I would expect people still be using it for a while before they invest to swtich.

@bkchr
Copy link
Member

bkchr commented Mar 26, 2024

Also this is a breaking change and I would expect people still be using it for a while before they invest to swtich.

No one is currently using the lookahead collator. So you can bring the argument for switching to it as well ;)

@skunert already told you that he can deliver a first version that supports multiple cores this week or the next week.

@skunert
Copy link
Contributor

skunert commented Mar 26, 2024

IMO it is fine to implement something for the lookahead collator if you want to get something running quickly for testing. However, integration will be non-optimal, since the lookahead collator only knows about relay triggered block production. If you just want to produce more blocks and submit them, that is not hard to integrate. They will be build directly in succession if you we do not build any kind of slot logic into it. Which brings us to #3168 .

As far as breakage goes, the way we author the blocks themselves does not change. So if you have a chain that has 6s slot time, I think we can make the slot-based authoring compatible.

As for ETA, AFAIR we aligned on April in the last sync meeting, but I think some initial version can come sooner, as I wrote in element.

@sandreim
Copy link
Contributor Author

Also this is a breaking change and I would expect people still be using it for a while before they invest to swtich.

No one is currently using the lookahead collator. So you can bring the argument for switching to it as well ;)

IIRC we are using it for our system parachains and people are supposed to use it for async backing which is something we are enabling on Kusuma/Polkadot right after the runtime upgrade.

@bkchr
Copy link
Member

bkchr commented Mar 26, 2024

"Supposed to use" and what people are doing are two different things ;) Also they are not required, because block production will continue to work as before.

@alindima alindima closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request. T9-cumulus This PR/Issue is related to cumulus.
Projects
Status: Completed
Development

No branches or pull requests

4 participants