Skip to content

Widen timed-boundary bounds in benchmark_fb smoke for CI jitter - #3732

Merged
chenBright merged 1 commit into
apache:masterfrom
randomkang:fix-benchmark-fb-smoke-flaky-bounds
Nov 11, 2026
Merged

chenBright merged 1 commit into
apache:masterfrom
randomkang:fix-benchmark-fb-smoke-flaky-bounds

Conversation

@randomkang

@randomkang randomkang commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

The benchmark_fb_smoke timed-boundary assertions were too tight for shared GitHub runners. Two consecutive CI runs on macOS both failed with different assertions tripped by the same cause: scheduling and I/O jitter adds ~150 ms on top of the 600 ms server-side delay, so a single call takes ~750 ms end-to-end.

  • Run 1: seconds=2.2 hit the exclusive upper bound (was < 2.2).
  • Run 2: seconds=2.1 passed, but p50_us=750474 exceeded 700000.

Widen the bounds while keeping the intent:

  • seconds: < 2.6 still rejects a window stretched by a whole extra call (~3.4 s) but tolerates one jittered in-flight call.
  • latency upper bound: use the request timeout (5 s, matching --timeout_ms=5000) instead of a fixed 700 ms; keep the 600 ms floor (the server-side delay always applies) and the p50<=max histogram ordering check.

What problem does this PR solve?

Issue Number: resolve #3731

Problem Summary:

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

The benchmark_fb_smoke timed-boundary assertions were too tight for
shared GitHub runners. Two consecutive CI runs on macOS both failed
with different assertions tripped by the same cause: scheduling and
I/O jitter adds ~150 ms on top of the 600 ms server-side delay, so a
single call takes ~750 ms end-to-end.

- Run 1: seconds=2.2 hit the exclusive upper bound (was < 2.2).
- Run 2: seconds=2.1 passed, but p50_us=750474 exceeded 700000.

Widen the bounds while keeping the intent:

- seconds: < 2.6 still rejects a window stretched by a whole extra
  call (~3.4 s) but tolerates one jittered in-flight call.
- latency upper bound: use the request timeout (5 s, matching
  --timeout_ms=5000) instead of a fixed 700 ms; keep the 600 ms
  floor (the server-side delay always applies) and the p50<=max
  histogram ordering check.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes address CI scheduling and I/O jitter while retaining the benchmark’s timing-window, latency-floor, and histogram-ordering checks.

No open issues 🎉

What changed in this PR

This PR widens the FlatBuffers benchmark smoke test’s timed assertions to tolerate scheduling and I/O jitter on shared CI runners while preserving validation of the expected timing window and server-side latency.

Changes:

  • Expands the timed-window upper bound from < 2.2 to < 2.6 seconds.
  • Replaces the fixed 700 ms latency ceiling with the configured 5-second request timeout.
  • Retains the 600 ms latency floor and histogram ordering checks.
File Description
example/​benchmark_fb/​smoke.py Relaxes timed-boundary assertions and updates comments to accommodate CI jitter while preserving the benchmark’s key validation checks.

Review effort: Lite

@chenBright chenBright left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chenBright
chenBright merged commit 11a404b into apache:master Nov 11, 2026
35 checks passed
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.

CI test-”FB ON (macos-latest, cmake, apple-clang)“failed

3 participants