Add Backoff wrapper that implements client-go's reset timer behaviour
#729
Conversation
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
Signed-off-by: Teo Klestrup Röijezon <[email protected]>
nightkr
force-pushed
the
backoff-watcher-/reset-duration
branch
from
d8ad2fa to
bb273ce
Compare
nightkr
commented
Nov 22, 2021
| assert_eq!(backoff.next_backoff(), Some(Duration::from_secs(2))); | ||
| } | ||
|
|
||
| struct TokioClock; |
There was a problem hiding this comment.
Need a pausable clock source for tests.
clux
approved these changes
Nov 22, 2021
|
|
||
| use backoff::{backoff::Backoff, Clock, SystemClock}; | ||
|
|
||
| /// A [`Backoff`] wrapper that resets after a fixed duration has elapsed. |
There was a problem hiding this comment.
Think this is great. I was initially going to suggest
/// A [`Backoff`] wrapper that resets after a fixed duration has elapsed
///
/// This is used to reduce load during upstream unhealthiness and is similar to
/// [apimachinery's backoffmanager](https://github.com/kubernetes/apimachinery/blob/7149480564b7d40046f19040efa20171b49059ae/pkg/util/wait/wait.go#L310-L349).
but will leave a comment for that in the place where we instantiate it in Observer wrapping ExponentialBackoff.
what you have here is a more generic building block.
| assert_eq!(backoff.next_backoff(), Some(Duration::from_secs(2))); | ||
| } | ||
|
|
||
| struct TokioClock; |
|
As for where to put this; maybe we should try to upstream this into |
|
other possibilities could be to:
|
4 tasks
clux
added a commit
that referenced
this pull request
Dec 21, 2021
* implement backoff for watcher - for #577 Signed-off-by: clux <[email protected]> * move magic number into strategy Signed-off-by: clux <[email protected]> * expose backoff from watcher and semi-propagate into controller awkward. will write a comment Signed-off-by: clux <[email protected]> * potential abstraction Signed-off-by: clux <[email protected]> * another builder layer; allow eliding ListParams Signed-off-by: clux <[email protected]> * forgot to add file Signed-off-by: clux <[email protected]> * easy parts of code review Signed-off-by: clux <[email protected]> * rewrite as a helper (take N) jesus this stuff is hard. Signed-off-by: clux <[email protected]> * rename as suggested Signed-off-by: clux <[email protected]> * Reimplement watcher backoff as FSM (#720) * Fix clippy warnings Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Reimplement watch backoff as FSM Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Remove useless lifetime bounds Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Silence clippy size warning Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Silence clippy properly this time around Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Split StreamBackoff into a separate utils module Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Backoff tests Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Add stream close test Signed-off-by: Teo Klestrup Röijezon <[email protected]> * remove backoff pin, fix docs Signed-off-by: clux <[email protected]> * newline Signed-off-by: clux <[email protected]> * Add `Backoff` wrapper that implements client-go's reset timer behaviour (#729) Signed-off-by: Teo Klestrup Röijezon <[email protected]> * use new reset backoff and replicate client-go reflector values Signed-off-by: clux <[email protected]> * fix node watcher example Signed-off-by: clux <[email protected]> * Use released `backoff` Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Factor out default `Backoff` Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Add note to `watcher` about backoff Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Added backoff to Controller Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Changelog Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Revert `Observer` for now Signed-off-by: Teo Klestrup Röijezon <[email protected]> * The clippyman comes for us all, eventually And we must all pay our due respects, or pay the price. Signed-off-by: Teo Klestrup Röijezon <[email protected]> * Fix build warnings Signed-off-by: Teo Klestrup Röijezon <[email protected]> * remove backoff_watch Signed-off-by: clux <[email protected]> * doc tweaks Signed-off-by: clux <[email protected]> * sentence Signed-off-by: clux <[email protected]> * upgrading backoff is not actually breaking Signed-off-by: clux <[email protected]> Co-authored-by: Teo Klestrup Röijezon <[email protected]> Co-authored-by: Teo Klestrup Röijezon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See #703 (comment)
Not sure how we want to expose this. Feels like kube-runtime is starting to pick up on a lot of utility stuff that doesn't have much to do with Kubernetes.