-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
client/v3: Add backoff before retry when watch stream returns unavailable #14556
Conversation
client/v3/watch.go
Outdated
} | ||
} | ||
time.Sleep(backoff) | ||
} |
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 code is duplicated to
Lines 1018 to 1028 in 6a0bbf3
if isUnavailableErr(w.ctx, err) { | |
// retry, but backoff | |
if backoff < maxBackoff { | |
// 25% backoff factor | |
backoff = backoff + backoff/4 | |
if backoff > maxBackoff { | |
backoff = maxBackoff | |
} | |
} | |
time.Sleep(backoff) | |
} |
Please get it included in a function so that it can be reused by both places.
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.
Thanks for the review. The backoff function has been added.
…able The client retries connection without backoff when the server is gone after the watch stream is established. This results in high CPU usage in the client process. This change introduces backoff when the stream is failed and unavailable. Signed-off-by: Hisanobu Tomari <[email protected]>
b3cb926
to
428fb96
Compare
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.
LGTM
Thank you @tomari
Codecov Report
@@ Coverage Diff @@
## main #14556 +/- ##
=======================================
Coverage 75.46% 75.46%
=======================================
Files 457 457
Lines 37187 37187
=======================================
Hits 28064 28064
Misses 7373 7373
Partials 1750 1750
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
lgtm. Thanks @tomari
@tomari Could you backport the fix to 3.5 and 3.4? |
The client retries connection without backoff when the server is gone after the watch stream is established. This results in high CPU usage in the client process. This change introduces backoff when the stream is failed and unavailable.
Signed-off-by: Hisanobu Tomari [email protected]