Skip to content

fix(connectors): draw the retry delay inside the cap instead of clamping - #4129

Merged
hubcio merged 2 commits into
apache:masterfrom
ryankert01:feat/connectors-jitter-distribution
Sep 12, 2026
Merged

fix(connectors): draw the retry delay inside the cap instead of clamping#4129
hubcio merged 2 commits into
apache:masterfrom
ryankert01:feat/connectors-jitter-distribution

Conversation

@ryankert01

Copy link
Copy Markdown
Member

Which issue does this PR address?

Relates to #3702

Rationale

Follow-up 1 from #4104, kept out of that PR to stay reviewable.

What changed?

jitter computed its ±20 % window in whole milliseconds, so below 5 ms the
window rounded to zero and every instance waited the identical delay. It also
drew first and clamped to max_delay after, so any draw above the cap landed
on the cap exactly: roughly half of all draws once the backoff saturates, and
36 % at a 57 s target against a 60 s cap. Both put retrying instances back in
step, which is what jitter exists to prevent.

retry_backoff now draws uniformly, in nanoseconds, from a window already cut
off at max_delay, instead of drawing then clamping. That leaves jitter
with no caller, so it goes. exponential_backoff moves to nanoseconds for the
same rounding reason; its other caller (nack_retry_delay) passes whole
milliseconds and is unaffected.

#4104 narrowed jitter to pub(crate) rather than deleting it. The near-cap
case only showed up under measurement afterwards, which is what collapsed both
branches into a single draw.

Local Execution

  • Passed — fmt, clippy -D warnings, 165 SDK tests, cargo doc under
    RUSTDOCFLAGS=-D warnings. Both new tests confirmed failing against feat(connectors)!: add shared retry_async helper to the connector SDK #4104's
    behaviour first.
  • prek ran and passes, except markdownlint, license-headers,
    trailing-whitespace, trailing-newline and binary-artifacts, which need
    bash >= 4.2; ran those directly instead.

AI Usage

  • Claude Code.
  • Implementation, tests and docs.
  • Local chain above; distribution measured over 10k draws before and after.
  • Yes.

@ryankert01
ryankert01 force-pushed the feat/connectors-jitter-distribution branch from 9860ab3 to 30feb7c Compare September 11, 2026 09:10
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.41%. Comparing base (3b93200) to head (e1c425f).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
core/connectors/sdk/src/retry.rs 97.43% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4129       +/-   ##
=============================================
- Coverage     86.64%   67.41%   -19.23%     
  Complexity     1455     1455               
=============================================
  Files          1267     1267               
  Lines        211572   175176    -36396     
  Branches     176797   140377    -36420     
=============================================
- Hits         183310   118090    -65220     
- Misses        23778    52517    +28739     
- Partials       4484     4569       +85     
Components Coverage Δ
Rust Core 63.47% <97.43%> (-24.14%) ⬇️
Java SDK 67.58% <ø> (ø)
C# SDK 76.97% <ø> (+0.06%) ⬆️
Python SDK 91.34% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 96.17% <ø> (-0.07%) ⬇️
Go SDK 69.40% <ø> (ø)
Files with missing lines Coverage Δ
core/connectors/sdk/src/retry.rs 94.49% <97.43%> (+0.03%) ⬆️

... and 399 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ryankert01
ryankert01 marked this pull request as ready for review September 11, 2026 12:29
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 11, 2026
`retry_backoff` jittered first and clamped second, so any draw above
`max_delay` landed on the bound itself. Once the exponential saturates,
`exponential_backoff` returns exactly `max_delay` and half of every
symmetric draw clamps: 247 of 512 saturated draws returned `max_delay` to
the nanosecond, which puts the instances that waited longest back in step.
The same happens, less often, one step below saturation. At a target of
57s against a 60s cap, 36% of draws clamp onto the bound.

The delay is now drawn uniformly from the ±20 % window with the upper edge
cut off at `max_delay`. The window narrows as the backoff approaches the
cap and sits fully below it once the backoff saturates, so the bound holds
with no pile-up and one rule covers every case. That removes the separate
`jitter` helper, whose only caller was this function.

`jitter` also computed its window as `millis / 5`, which rounds to zero
below 5 ms, and `exponential_backoff` rounded the same way, so a
sub-millisecond `retry_delay` produced a zero delay and retried at once.
Both now compute in nanoseconds. A sub-millisecond delay is reachable from
configuration.

Two tests cover the cases, and both fail against the previous behavior.

Relates to apache#3702
Relates to apache#4084
@ryankert01
ryankert01 force-pushed the feat/connectors-jitter-distribution branch from 30feb7c to 341efe2 Compare September 11, 2026 12:30
@hubcio
hubcio merged commit 2b2e612 into apache:master Sep 12, 2026
66 checks passed
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants