TRT-2811: Make prow loader batch writes resilient to single-batch failures - #3798
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@mstaeble: This pull request references TRT-2811 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
WalkthroughProw job-run loading now performs best-effort batch writes, records batch failures, continues processing after write errors, respects context cancellation, and updates processed metrics from successful results. ChangesProw job-run processing
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
00bdc00 to
a101186
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 50 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 `@pkg/dataloader/prowloader/prow.go`:
- Around line 1206-1246: The accumulateAndWriteJobRuns flow currently hard-codes
writeJobRunBatch, preventing isolated regression tests. Add a function-type
writer field to ProwLoader, initialize it to writeJobRunBatch in production, and
have accumulateAndWriteJobRuns invoke that field; then add prow_test.go coverage
for failed-then-successful batches, per-batch pl.errors, and cancellation with a
pending batch.
- Around line 1225-1235: Update the result-processing loop around batch
accumulation and the final flush so cancellation is honored: check ctx.Err()
before appending each result and guard the len(batch) > 0 final flush with
ctx.Err() == nil. Preserve normal threshold-based flushing when the context
remains active.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b1f43d8-f50f-4476-945f-9b7e3a6bac25
📒 Files selected for processing (1)
pkg/dataloader/prowloader/prow.go
…lures When a batch write fails in accumulateAndWriteJobRuns, log the error and continue processing subsequent batches instead of returning immediately. Each batch error is appended individually to pl.errors so that LoaderWithMetrics counts them separately. A ctx.Err() check at the top of each loop iteration exits cleanly on cancellation to avoid doomed write attempts. The cancelFetch() call is removed since loadDailyTestAnalysisByJob sources from BigQuery and doesn't depend on PG writes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a101186 to
d31a9ac
Compare
|
Scheduling required tests: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mstaeble, petr-muller The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Scheduling required tests: |
|
@mstaeble: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
accumulateAndWriteJobRuns, log the error and continue processing subsequent batches instead of aborting the entire loadpl.errorssoLoaderWithMetricscounts them separately in Prometheus metricsctx.Err()check at the top of each loop iteration exits cleanly on cancellation to avoid doomed write attemptscancelFetch()call sinceloadDailyTestAnalysisByJobsources from BigQuery and doesn't depend on PG writesTest plan
go vet ./pkg/dataloader/prowloader/...passesgo test ./pkg/dataloader/prowloader/...passesmake lintpasses🤖 Generated with Claude Code
Summary by CodeRabbit