This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
slots: incrementally backoff claiming slots if finality lags behind #7186
Merged
Merged
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
250fbeb
babe: backoff authoring blocks when finality lags
octol d531efa
babe: move backoff authoring params to default constructor
octol b060e9b
babe: deduplicate the test a bit
octol 0f95c44
babe: set backoff constants in service
octol df1951c
babe: use better names for backoff authoring block parameters
octol c17ca03
babe: remove last unwrap
octol 2892da0
babe: slight style tweak
octol 01f1ac3
babe: fix comment
octol 9794ffa
slots: move backoff block authorship logic to SimpleSlotWorker
octol d2ee780
aura: append SlotInfo in on_slot
octol 026a8d0
slots: use the correct types for parameters
octol f428a35
Merge branch 'master' into jon/incremental-backoff-on-finality
octol 6903837
slots: fix review comments
octol 4a6b8b7
Merge branch 'master' into jon/incremental-backoff-on-finality
octol 92b506a
aura: add missing backoff authoring blocks parameters
octol cda5150
slots: add comments for default values
octol 62f97dc
slots: add additional checks in test
octol 7fa2084
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol af8d30b
slots: update implementation for new master
octol 76b745a
slots: revert the change to SlotInfo
octol dafd2e5
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol dc04a1f
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol cb6d24c
Fix review comments
octol 55827c3
slots: rework unit tests for backing off claiming slots
octol 83538f0
slots: add test for asymptotic behaviour for slot claims
octol c87145d
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol 0435142
slots: address review comments
octol 38e61e5
slots: add test for max_interval
octol 1228aef
slots: add assertion for intervals between between claimed slots
octol f7b8c76
slots: remove rustfmt directive
octol 9790f60
slots: another attempt at explaining authoring_rate
octol f95225a
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol 461d5f9
slots: up unfinalized_slack to 50 by default
octol 796baf7
slots: add tests for time to reach max_interval
octol 0d11df5
slots: fix typo in comments
octol 9a968c6
Apply suggestions from code review
octol a607bc4
slots: additional tweaks to comments and info calls
octol bc32a7c
slots: rename to BackoffAuthoringOnFinalizedHeadLagging
octol eb45ed7
slots: make the backing off strategy generic
octol 5a40da8
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol 4f44b62
Apply suggestions from code review
octol 1423649
slots: implement backoff trait for () for simplicity
octol 44de2cf
slots: move logging inside backing off function to make it more specific
octol 90d0ad3
Merge remote-tracking branch 'upstream/master' into jon/incremental-b…
octol b6435ff
aura: add missing function parameter
octol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the most elegant maybe, is there a cleaner way @bkchr ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah not that nice, but yeah 🤷
@andresilva WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option would be to pass the logging target to the strategy itself when creating it but I think that's as nice as this. The reason we use the same logging target here as block production is to make sure these logging messages don't go unnoticed (i.e. people would forget to add some "slots" target). In this case the message is being logged under info so it should always be printed regardless of the target we provide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you're saying might as well drop the
logging_target
altogether?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either keep the changes you made or drop it altogether. I don't have a strict opinion about this, I guess we can keep it as is.