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 <teo@nullable.se>
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 <sszynrae@gmail.com> * move magic number into strategy Signed-off-by: clux <sszynrae@gmail.com> * expose backoff from watcher and semi-propagate into controller awkward. will write a comment Signed-off-by: clux <sszynrae@gmail.com> * potential abstraction Signed-off-by: clux <sszynrae@gmail.com> * another builder layer; allow eliding ListParams Signed-off-by: clux <sszynrae@gmail.com> * forgot to add file Signed-off-by: clux <sszynrae@gmail.com> * easy parts of code review Signed-off-by: clux <sszynrae@gmail.com> * rewrite as a helper (take N) jesus this stuff is hard. Signed-off-by: clux <sszynrae@gmail.com> * rename as suggested Signed-off-by: clux <sszynrae@gmail.com> * Reimplement watcher backoff as FSM (#720) * Fix clippy warnings Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Reimplement watch backoff as FSM Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Remove useless lifetime bounds Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Silence clippy size warning Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Silence clippy properly this time around Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Split StreamBackoff into a separate utils module Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Backoff tests Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Add stream close test Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * remove backoff pin, fix docs Signed-off-by: clux <sszynrae@gmail.com> * newline Signed-off-by: clux <sszynrae@gmail.com> * Add `Backoff` wrapper that implements client-go's reset timer behaviour (#729) Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * use new reset backoff and replicate client-go reflector values Signed-off-by: clux <sszynrae@gmail.com> * fix node watcher example Signed-off-by: clux <sszynrae@gmail.com> * Use released `backoff` Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Factor out default `Backoff` Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Add note to `watcher` about backoff Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Added backoff to Controller Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Changelog Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * Revert `Observer` for now Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * 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 <teo@nullable.se> * Fix build warnings Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se> * remove backoff_watch Signed-off-by: clux <sszynrae@gmail.com> * doc tweaks Signed-off-by: clux <sszynrae@gmail.com> * sentence Signed-off-by: clux <sszynrae@gmail.com> * upgrading backoff is not actually breaking Signed-off-by: clux <sszynrae@gmail.com> Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de> Co-authored-by: Teo Klestrup Röijezon <teo@nullable.se>
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.