perf: skip retry sleep if context deadline is too short#973
perf: skip retry sleep if context deadline is too short#973shafayetfahim wants to merge 1 commit intoredis:mainfrom
Conversation
Introduce 100us buffer in WaitOrSkipRetry to fail-fast when the context will expire before the retry delay finishes.
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
| if dl, ok := ctx.Deadline(); !ok || time.Until(dl) > delay { | ||
| } | ||
| if delay > 0 { | ||
| if dl, ok := ctx.Deadline(); !ok || time.Until(dl) > (delay+(100*time.Microsecond)) { |
There was a problem hiding this comment.
How do you come up with the 100 microseconds?
There was a problem hiding this comment.
Thank you for taking the time to check this out! And good question. I landed on 100 microseconds as a fail-fast threshold with the following considerations:
- With high load, the time it'll take for the Go scheduler to wake a parked Goroutine and for the OS to perform a context switch will likely consume a significant portion of the 100 microseconds
- If RTT is typically greater than 200 microseconds then 100 microseconds keeps a retry completion unlikely before ctx.Deadline is reached
More than happy to take suggestions or adjust the value if you prefer a tighter bound here.
There was a problem hiding this comment.
Honestly, I am not sure if this is really worth adding.
2 participants
Problem: The current WaitOrSkipRetry implementation may initiate a sleep/wait even if the context.Deadline is guaranteed to expire before the sleep finishes. This leads to unnecessary timer allocations and goroutine parking in scenarios where a retry is inherently impossible.
Solution: Introduce a 100us buffer to account for runtime and scheduling overhead. If the remaining time in the context is less than the calculated delay + buffer, the function returns false immediately to fail-fast.
Verification: Added TestWaitOrSkipRetry_DeadlineBuffer to validate the early-exit logic. Verified that all integration tests (cluster, sentinel, locking) pass locally on darwin/arm64.
Note
Low Risk
Low risk perf tweak limited to retry backoff gating; main risk is behavior change at tight deadlines potentially reducing retry attempts in edge timing cases.
Overview
WaitOrSkipRetrynow fails fast when the remainingcontextdeadline is shorter than the computed retry delay plus a small (100µs) buffer, avoiding unnecessary timer/sleep work when a retry cannot realistically complete.Adds
TestWaitOrSkipRetry_DeadlineBufferto assert the function returns immediately (and non-retriable) when the deadline is shorter than the delay.Written by Cursor Bugbot for commit 6777b07. This will update automatically on new commits. Configure here.