TRT-2856: Extend partition start bound to cover outlier job start times - #3839
Conversation
ensurePartitions() only covered [loadSince-1day, now+2days], but jobs Prow eventually marks as aborted after getting stuck can report a StartTime days before the completion time the BQ query filters on. When such a job is fetched, prow_job_run_tests has no partition for its timestamp and the whole batch write fails (SQLSTATE 23514), which is what broke the sippy-backup CronJob against release 4.20. Extend the start bound to the earliest StartTime actually present in the fetched job set, so partitions always cover the real data being written regardless of how anomalous a single job's timestamp is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Skipping CI for Draft Pull Request. |
WalkthroughThe Prow loader now extends partition creation to earlier non-zero job start times while retaining a one-day grace period. ChangesProw partition bounds
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
petr-muller-reviewer
left a comment
There was a problem hiding this comment.
Solid, well-targeted fix. Adapting the partition window to actual data instead of guessing a larger grace period is the right design. Code is clean, well-tested, and follows project conventions. Two nits on comment accuracy, one observability question.
|
@petr-muller-reviewer: changing LGTM is restricted to collaborators 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 kubernetes-sigs/prow repository. |
- Correct partitionStartDate's doc comment: loadSince-1day is the default, extended earlier when needed, not a floor. - Fix ensurePartitions's doc comment: default lookback is DefaultLookbackDays (14), not one week. - Log a warning when the start bound is extended beyond the default, so operators can see when an outlier job's StartTime triggered it.
|
/retitle TRT-2856: Extend partition start bound to cover outlier job start times |
|
@petr-muller: This pull request references TRT-2856 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 bug 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. |
|
Scheduling required tests: |
|
@petr-muller: 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mstaeble, petr-muller, petr-muller-reviewer 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 |
Summary
sippy-backupCronJob pods (fresh-DBsippy load --init-database --release=4.20) have been failing repeatedly with:ensurePartitions()only creates daily partitions covering[loadSince-1day, now+2days]. Jobs that Prow eventually marksabortedafter getting stuck can report aStartTimedays before theircompletiontime (the field the BigQuery fetch query actually filters on), so such a job can be fetched while itsStartTimefalls outside the partition window. Confirmed one such job directly:periodic-ci-openshift-release-main-nightly-4.20-e2e-agent-single-node-ipv6-conformance(build2076094714775343104), started 2026-07-12, not marked complete (aborted) until 2026-07-15 — an 80 hour gap, well past the existing 1-day grace.abortedjobs (all releases) had start→completion gaps beyond 24h, with the worst observed around 83h.Status.StartTimeactually present in the fetched job set (with the existingloadSince - 1 dayas a floor), so partitions always cover the real data about to be written, regardless of how anomalous a single job's timestamp is.This is a follow-up to #3798 (which stops one bad batch from aborting/cancelling the entire load) — this PR addresses the actual partition-range gap that causes the batch to fail in the first place.
Test plan
go build ./pkg/dataloader/prowloader/...go vet ./pkg/dataloader/prowloader/...go test ./pkg/dataloader/prowloader/...golangci-lint run ./pkg/dataloader/prowloader/...(no new findings)TestPartitionStartDatecovering: no jobs, jobs within the grace window, zero-value start times, and an outlier job extending the bound🤖 Generated with Claude Code
Summary by CodeRabbit