HYPERFLEET-1493 - fix: eliminate CPU-contention flakes in perf latency specs - #157
Open
kuudori wants to merge 1 commit into
Open
HYPERFLEET-1493 - fix: eliminate CPU-contention flakes in perf latency specs#157kuudori wants to merge 1 commit into
kuudori wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds shared median-latency measurement with five default samples. Channel, cluster, version, and WIFConfig performance suites now run serially and use the shared helper. Delete tests measure multiple resources. Deferred cleanup helpers handle channels and WIFConfigs. E2E suite initialization shares the JWT from process 1. Documentation and threshold guidance describe serial Tier1 nightly performance tests. Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PerformanceSpec
participant MeasureMedianLatency
participant API
PerformanceSpec->>MeasureMedianLatency: provide threshold and sample callback
MeasureMedianLatency->>API: execute sampled API request
API-->>MeasureMedianLatency: return request result
MeasureMedianLatency-->>PerformanceSpec: log median and assert threshold
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/channel/perf_update_latency.go`:
- Around line 38-48: Vary the PATCH payload on every sample so each measured
request performs a real update rather than becoming a no-op. Update the callback
in e2e/channel/perf_update_latency.go:38-48,
e2e/version/perf_update_latency.go:46-58, and
e2e/wifconfig/perf_update_latency.go:38-48 to use the callback’s iteration value
and assign a distinct valid field value for each sample while preserving the
existing PATCH and latency-measurement flow.
In `@pkg/helper/perf.go`:
- Around line 27-36: Validate n at the start of MeasureMedianLatency before
allocation, reject non-positive counts, and calculate the true median for even
counts by averaging both middle durations (or explicitly enforce odd counts).
Add table-driven tests in pkg/helper/perf_test.go covering zero, negative, and
even sample counts, including the expected error or behavior for each case.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: da7a54c2-cc53-494b-b35b-6808c48def7d
📒 Files selected for processing (24)
docs/development.mde2e/channel/perf_create_latency.goe2e/channel/perf_delete_latency.goe2e/channel/perf_get_latency.goe2e/channel/perf_list_latency.goe2e/channel/perf_update_latency.goe2e/cluster/perf_list_filtered_latency.goe2e/cluster/perf_list_latency.goe2e/cluster/perf_read_entity_size_latency.goe2e/version/perf_create_latency.goe2e/version/perf_delete_latency.goe2e/version/perf_get_latency.goe2e/version/perf_list_latency.goe2e/version/perf_update_latency.goe2e/wifconfig/perf_create_latency.goe2e/wifconfig/perf_delete_latency.goe2e/wifconfig/perf_get_latency.goe2e/wifconfig/perf_list_latency.goe2e/wifconfig/perf_update_latency.gopkg/config/thresholds.gopkg/e2e/suite.gopkg/helper/helper.gopkg/helper/perf.gopkg/helper/perf_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
kuudori
force-pushed
the
HYPERFLEET-1493-perf-flake-fix
branch
from
August 11, 2026 03:14
63e38b8 to
9e5ba16
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/helper/perf_test.go`:
- Around line 11-13: Update TestMeasureMedianLatency to replace time.Sleep-based
timing with an internal clock or duration-measurement seam that accepts scripted
durations. Add assertions for durations exactly equal to the threshold, and for
even-sized samples test thresholds that distinguish lower-middle from
upper-middle selection. Preserve coverage of sampling, threshold behavior,
median calculation, and relevant error paths through the seam.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ff719dc9-880f-4be6-8dc6-27b1e4a7df27
📒 Files selected for processing (5)
e2e/channel/perf_update_latency.goe2e/version/perf_update_latency.goe2e/wifconfig/perf_update_latency.gopkg/helper/perf.gopkg/helper/perf_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/helper/perf.go
- e2e/version/perf_update_latency.go
- e2e/channel/perf_update_latency.go
- e2e/wifconfig/perf_update_latency.go
…y specs - Mark ms-scale perf latency specs (channel/version/wifconfig/cluster create/update/list/delete/read) ginkgo.Serial so Ginkgo's parallel procs don't contend for CPU and skew the threshold assertions - Extract shared median-of-N sampling into helper.MeasureMedianLatency, replacing hand-rolled timing code duplicated across 18 spec files - Split the median/threshold assertion out of MeasureMedianLatency into assertMedianBelowThreshold so its unit tests exercise the math with exact durations instead of time.Sleep, removing the last source of CPU-contention flakiness in this package's own test suite - Mint the suite JWT once via SynchronizedBeforeSuite instead of once per parallel process - Add DeferChannelCleanup/DeferWifConfigCleanup helpers alongside the existing DeferClusterCleanup
kuudori
force-pushed
the
HYPERFLEET-1493-perf-flake-fix
branch
from
August 11, 2026 03:53
9e5ba16 to
4d92809
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
ginkgo.Serialso Ginkgo's parallel procs don't contend for CPU and skew the threshold assertionshelper.MeasureMedianLatency, replacing hand-rolled timing code duplicated across 18 spec filesSynchronizedBeforeSuiteinstead of once per parallel processDeferChannelCleanup/DeferWifConfigCleanuphelpers alongside the existingDeferClusterCleanupTest plan
make check(generate, fmt-check, vet, lint, test) passes locallyMeasureMedianLatency(sampling count, threshold pass/fail, median-not-mean)