feat(EC-1816): add multi-component stress benchmark#3331
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds stress benchmark data preparation and publish scripts, a new stress benchmark runner that loads archived data and executes validation workloads, and README documentation for the new benchmark entry. ChangesStress benchmark infrastructure
Estimated code review effort: 2 (Simple) | ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Looks good to me Previous runReviewFindingsLow
Labels: PR adds Go benchmark/testing infrastructure under benchmark/stress/. Previous run (2)ReviewFindingsMedium
Low
Info
Previous run (3)ReviewFindingsLow
Info
Previous run (4)ReviewFindingsLow
Info
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
🤖 Finished Review · ✅ Success · Started 12:58 PM UTC · Completed 1:06 PM UTC |
|
🤖 Review · Started 12:25 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 12:34 PM UTC · Completed 12:44 PM UTC |
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 `@benchmark/stress/prepare_data.sh`:
- Around line 30-36: The oras pull command suppresses error output with
2>/dev/null and the script always falls back to regenerating from upstream on
failure, making CI runs non-reproducible and hiding infrastructure issues.
Remove the error suppression (2>/dev/null) from the oras pull command on line 30
and restructure the logic so that if the oras pull fails, the script exits with
an error rather than continuing to the regeneration fallback. This ensures
benchmark input remains deterministic and surfaces any Quay or authentication
failures instead of silently working around them.
In `@benchmark/stress/stress.go`:
- Around line 26-38: The imports in the stress.go file are not properly ordered
according to the gci formatting standards. Run the project's Go import
formatting tool (typically gci write or go fmt) on the stress.go file to
automatically reorder the imports into the correct grouping: standard library
imports first, followed by blank line, then third-party imports (like
golang.org/x/benchmarks), followed by blank line, then local package imports
(like github.com/conforma/cli). This will resolve the gci formatting check
failure.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 03e32d5c-c048-4271-92dd-eba0be016eaa
📒 Files selected for processing (3)
benchmark/stress/prepare_data.shbenchmark/stress/push_data.shbenchmark/stress/stress.go
st3penta
left a comment
There was a problem hiding this comment.
just one comment, but overall LGTM
|
🤖 Finished Review · ✅ Success · Started 1:06 PM UTC · Completed 1:18 PM UTC |
Add a stress benchmark under benchmark/stress/ that validates a multi-component snapshot with configurable worker count, simulating real-world release pipeline workloads that caused OOM (EC-1805). - Component count controlled via EC_STRESS_COMPONENTS (default 10) - Worker count controlled via EC_STRESS_WORKERS (default 35) - Uses the same golden-container image as the simple benchmark, duplicated across components at runtime - Reuses the existing benchmark/internal/suite harness - Includes prepare_data.sh to regenerate offline data archive - Automatically supported by make benchmark_stress via Makefile wildcard rules Resolves: EC-1816 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pull pre-built data.tar.gz from quay.io/conforma/benchmark-data in prepare_data.sh, falling back to upstream regeneration. Add push_data.sh for uploading the archive. Resolves: EC-1816 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reject zero and negative values for EC_STRESS_COMPONENTS and EC_STRESS_WORKERS to fail fast instead of producing meaningless benchmark results. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace deprecated --json-input with --images, add benchmark listing to README. Resolves: EC-1816 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I pushed nothing new in this PR; I had to rebase it to get the CVE fixes in. |
|
🤖 Finished Review · ✅ Success · Started 1:48 PM UTC · Completed 2:00 PM UTC |
| } | ||
|
|
||
| type source struct { | ||
| Git gitSource `json:"git"` |
There was a problem hiding this comment.
[low] edge-case
The buildSnapshot function names components as 'golden-container-0', 'golden-container-1', etc. When EC_STRESS_COMPONENTS=1, this produces 'golden-container-0', while the simple benchmark names its single component 'golden-container' (no index suffix). Functionally correct but worth noting for result comparison consistency between benchmarks.
benchmark/stress/that validates a multi-component snapshot with 35 workers, simulating the workload that caused the OOM incident (EC-1805)EC_STRESS_COMPONENTS, default 10) and worker count (EC_STRESS_WORKERS, default 35) are parameterized via env vars for CI tuningbenchmark/internal/suite, registry, untar) and the same golden-container image data, duplicated across components at runtimeResolves: EC-1816