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 further in ClaimQueue during collation generation #4049

Closed
wants to merge 1 commit into from

Conversation

tdimitrov
Copy link
Contributor

Part of #1797

Implementation for #1797 (comment)

When generating a collation the first two elements from the ClaimQueue are examined:

  • depth 0 - this is the ParaId scheduled next. As suggested in the linked comment we will build a collation for this slot on startup or if our ParaId was not scheduled for two (or more) slots. Providing a collation for depth 0 will probably time out on the first try but the collation will be ready for the retry,
  • depth 1 - this is the most relevant slot from collation generation point of view. As discussed providing collation for depth 0 is challenging so we also prepare one more collation for the next slot if our ParaId is scheduled there.

To keep track of the work done in previous iterations of the collation-generation run loop a HashSet is passed to handle_new_activations. It persist all work scheduled in the current iteration and is overwritten on the next iteration. I used this instead of a bool indicating if this is a first iteration or not (as suggested in the comment) because I want to cover the case when the collator's ParaId has not been scheduled for two consecutive ClaimQueue slots.

This PR is still work in progress.

TODOS:

  • Collect feedback on the initial approach
  • Fix compilation errors in unit tests.
  • Write new unit tests.
  • Write end-to-end test.

@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/5843905

@bkchr
Copy link
Member

bkchr commented Apr 9, 2024

Collation generation is a subsystem that slowly has reached the end of its lifetime. The changes you are trying to incorporate here are not really necessary. None of the new collator logic will make use of the collation generation subsystem for triggering its block production. The new collator logic is being build based on what is described here, which already includes the stuff that is mentioned in the linked issue comment.

@tdimitrov tdimitrov closed this Apr 10, 2024
@tdimitrov tdimitrov deleted the tsv-collator-claimqueue branch April 11, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants