Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

collator-protocol/validator_side: a couple of fixes #4179

Merged
merged 9 commits into from
Oct 30, 2021
Merged

collator-protocol/validator_side: a couple of fixes #4179

merged 9 commits into from
Oct 30, 2021

Conversation

ordian
Copy link
Member

@ordian ordian commented Oct 29, 2021 •
edited
Loading

This PR contains 2 quick and somewhat hacky fixes:

  1. The collation response is now checked every 5ms instead of previously it could take up to 1s because of select! blocking on any future to complete. This should be refactored in the future with a proper stream instead of a hashmap of futures.
  2. Validators that will be assigned in the next group for para_id are now not requesting a collation for para_id until it's their turn. We might refactor that in the future as well (cf. Connecting to next group in advance in collator protocol does not work at session boundaries #2985).
  • tests?

@ordian ordian added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Oct 29, 2021
@ordian ordian changed the title collator-prototol/validator_side: a couple of fixes collator-protocol/validator_side: a couple of fixes Oct 29, 2021
@crystalin
Copy link

Thank you @ordian , from my understanding there is no need to include it in the collators to have it tested.
I'll upgrade our validators on our internal networks to see if that improves the situation

drahnr
drahnr approved these changes Oct 30, 2021
View reviewed changes
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

Bunch of nits, other than that: LGTM!

@ordian ordian mentioned this pull request Oct 30, 2021
Closed
@ordian
review nits
8bc9321
@ordian
remove outdated comment
5aea24f
@ordian
Merge branch 'master' into ao-collator-prototol-a-fix
a21b8da 
* master:
  remove duplicate Deposit from OnUnbalanced implementation (#4180)
  differentiate log messages (#4183)
  increase ump_service_total_weight's default value (#4127)
  companion PR to removal of light client (#4105)
  Introduce `OriginPrivilegeCmp` (#4166)
@rphmeier rphmeier merged commit a6dd54e into master Oct 30, 2021
@rphmeier rphmeier deleted the ao-collator-prototol-a-fix branch October 30, 2021 19:20
emostov pushed a commit that referenced this pull request Nov 1, 2021
@ordian @emostov
collator-protocol/validator_side: a couple of fixes (#4179)
9f0ebcc 
* collator-protocol/validator: do not wait 1s to poll requested collations

* collator-protocol/validator: do not request collation for the next group

* put everything into select

* fmt

* more hacks yay

* a test

* review nits

* remove outdated comment
@ghzlatarev ghzlatarev mentioned this pull request Dec 4, 2021
Merged
23 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@drahnr drahnr drahnr approved these changes

@rphmeier rphmeier rphmeier approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ordian @crystalin @drahnr @rphmeier