-
Notifications
You must be signed in to change notification settings - Fork 67
A0-1796: add justification broadcast ticker #833
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b4809b3
add justification broadcast ticker
maciejnems ebe044c
Merge branch 'main' into A0-1796-broadcast-ticker
maciejnems c989c8e
Join periodic and normal broadcast last Instant
maciejnems 78c5068
move clippy dead code
maciejnems d5a908b
rename to ticker
maciejnems bf0011c
restructure tests
maciejnems ab8954c
change timeout after try tick true
maciejnems 0d138ab
hmmmmmm
maciejnems 4a0d9ad
apply suggested changes to docs
maciejnems 699b965
add assert to constructor
maciejnems d5a351c
or maybe no assert
maciejnems 2d645e3
the in docs
maciejnems 3cc1a9e
wait_and_tick
maciejnems 10c94e5
Merge branch 'main' into A0-1796-broadcast-ticker
maciejnems 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 hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ mod nodes; | |
| mod party; | ||
| mod session; | ||
| mod session_map; | ||
| mod sync; | ||
| #[cfg(test)] | ||
| pub mod testing; | ||
|
|
||
|
|
||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| // TODO: remove when ticker is used | ||
| #[allow(dead_code)] | ||
| mod ticker; | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| use tokio::time::{sleep, Duration, Instant}; | ||
|
|
||
| /// This struct is used for determining when we should broadcast justification so that it does not happen too often. | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pub struct BroadcastTicker { | ||
| last_broadcast: Instant, | ||
| current_periodic_timeout: Duration, | ||
| max_timeout: Duration, | ||
| min_timeout: Duration, | ||
| } | ||
|
|
||
| impl BroadcastTicker { | ||
| pub fn new(max_timeout: Duration, min_timeout: Duration) -> Self { | ||
kostekIV marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Self { | ||
| last_broadcast: Instant::now(), | ||
| current_periodic_timeout: max_timeout, | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| max_timeout, | ||
| min_timeout, | ||
| } | ||
| } | ||
|
|
||
| /// Returns whether we should broadcast right now if we just imported a justification. | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// If min_timeout elapsed since last broadcast, returns true, sets last broadcast to now and will | ||
| /// return true again if called after `self.min_timeout`. | ||
| /// If not, returns false and sets periodic broadcast timeout to `self.min_timeout`. | ||
| /// This is to prevent from sending every justification when importing a batch of them. This way, | ||
| /// when receiving batch of justifications we will broadcast the first justification and the highest known | ||
| /// after `self.min_timeout` using periodic broadcast. | ||
| pub fn try_broadcast(&mut self) -> bool { | ||
| let now = Instant::now(); | ||
| if now.saturating_duration_since(self.last_broadcast) >= self.min_timeout { | ||
| self.last_broadcast = now; | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| true | ||
| } else { | ||
| self.current_periodic_timeout = self.min_timeout; | ||
| false | ||
| } | ||
| } | ||
|
|
||
| /// Sleeps until next periodic broadcast should happen. | ||
| /// In case enough time elapsed, sets last broadcast to now and periodic timeout to `self.max_timeout`. | ||
| pub async fn wait_for_periodic_broadcast(&mut self) { | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| let since_last = Instant::now().saturating_duration_since(self.last_broadcast); | ||
| sleep(self.current_periodic_timeout.saturating_sub(since_last)).await; | ||
| self.current_periodic_timeout = self.max_timeout; | ||
| self.last_broadcast = Instant::now(); | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use tokio::time::{sleep, timeout, Duration}; | ||
|
|
||
| use super::BroadcastTicker; | ||
|
|
||
| #[tokio::test] | ||
| async fn try_broadcast() { | ||
| let max_timeout = Duration::from_millis(700); | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| let min_timeout = Duration::from_millis(100); | ||
| let mut ticker = BroadcastTicker::new(max_timeout, min_timeout); | ||
|
|
||
| assert!(!ticker.try_broadcast()); | ||
| sleep(min_timeout).await; | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert!(ticker.try_broadcast()); | ||
| assert!(!ticker.try_broadcast()); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn wait_for_periodic_broadcast() { | ||
| let max_timeout = Duration::from_millis(700); | ||
| let min_timeout = Duration::from_millis(100); | ||
| let mut ticker = BroadcastTicker::new(max_timeout, min_timeout); | ||
|
|
||
| assert_ne!( | ||
| timeout(2 * min_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| assert_eq!( | ||
| timeout(max_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn wait_for_periodic_broadcast_after_try_broadcast_true() { | ||
| let max_timeout = Duration::from_millis(700); | ||
| let min_timeout = Duration::from_millis(100); | ||
| let mut ticker = BroadcastTicker::new(max_timeout, min_timeout); | ||
|
|
||
| sleep(min_timeout).await; | ||
| assert!(ticker.try_broadcast()); | ||
|
|
||
| assert_ne!( | ||
| timeout(2 * min_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| assert_eq!( | ||
| timeout(max_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn wait_for_periodic_broadcast_after_try_broadcast_false() { | ||
| let max_timeout = Duration::from_millis(700); | ||
| let min_timeout = Duration::from_millis(100); | ||
| let mut ticker = BroadcastTicker::new(max_timeout, min_timeout); | ||
|
|
||
| assert!(!ticker.try_broadcast()); | ||
|
|
||
| assert_eq!( | ||
| timeout(2 * min_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| assert_ne!( | ||
| timeout(2 * min_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| assert_eq!( | ||
| timeout(max_timeout, ticker.wait_for_periodic_broadcast()).await, | ||
| Ok(()) | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn try_broadcast_after_wait_for_periodic_broadcast() { | ||
| let max_timeout = Duration::from_millis(700); | ||
| let min_timeout = Duration::from_millis(100); | ||
| let mut ticker = BroadcastTicker::new(max_timeout, min_timeout); | ||
|
|
||
| assert_eq!( | ||
| timeout( | ||
| max_timeout + min_timeout, | ||
timorl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ticker.wait_for_periodic_broadcast() | ||
| ) | ||
| .await, | ||
| Ok(()) | ||
| ); | ||
|
|
||
| assert!(!ticker.try_broadcast()); | ||
| sleep(min_timeout).await; | ||
| assert!(ticker.try_broadcast()); | ||
| } | ||
| } | ||
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.
Uh oh!
There was an error while loading. Please reload this page.