Skip to content

fix(collector): widen uploader OSS readback retry window - #565

Merged
proerror77 merged 4 commits into
mainfrom
codex/upload-readback-window
Aug 1, 2026
Merged

proerror77 merged 4 commits into
mainfrom
codex/upload-readback-window

Conversation

@proerror77

@proerror77 proerror77 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Change contract

Uploader readback-after-upload: 12 attempts × 5s backoff, per-file timeout capped at 60s, and a 300s wall-clock budget for the whole loop (replacing the 3×1s window). This is the actual root cause of the repeated shadow-gate failures on 2026-08-01: the gate-script retry patch (#556) could not help because the 404 happens inside the uploader binary's own post-PUT verification.

Production evidence

Three gate invocations (3736e02d, 842bab38, 6d1bfe7f) failed on ossutil cp 404 NoSuchKey of just-uploaded shadow objects; in each case the object became HEAD-able moments after the 3×1s window expired (verified via ossutil stat afterwards). With the wider window the readback succeeds instead of failing the upload (and thus the gate).

Second-round review fixes

  • The first revision's 120s backoff cap was dead code (11×5s sleeps = 55s max); replaced with a wall-clock budget covering command time + sleeps.
  • Downloads previously inherited the 300s upload timeout (36 × 300s worst case); per-file readback timeout now capped at 60s.
  • Retry delay is 5ms under cfg(test): suite runs in 0.69s (was 55.7s).

Out of scope

Dependency / merge order

None. Release note: candidate cb55ac1d carries the old 3×1s window and cannot pass the readback in this environment — the next release built after this merge must be gated instead.

Focused validation

  • cargo test -p hft-collector --lib polymarket_upload --locked: 62 passed, 0 failed, 0.69s

Rollout / rollback impact

Retry timing only; fail-closed semantics unchanged (upload still fails if the object never appears). Rollback = revert.

Issue relationship

Refs #556

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when reading back uploaded files from storage.
    • Added automatic retries and time limits to prevent stalled downloads.
    • Ensured each file download respects the configured timeout limits.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@proerror77, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1cc5283d-5c9c-4ffb-a5bd-68c0e9e52891

📥 Commits

Reviewing files that changed from the base of the PR and between 339b76f and 74eb889.

📒 Files selected for processing (1)
  • rust_hft/tools/collector/src/polymarket_upload.rs
📝 Walkthrough

Walkthrough

OSS readback now retries with bounded attempts and delays. Each artifact download uses a capped timeout. The retry loop also enforces a total wall-clock limit.

Changes

OSS readback control

Layer / File(s) Summary
Bound OSS readback retries
rust_hft/tools/collector/src/polymarket_upload.rs
The collector increases retry attempts, applies production and test delays, caps each download at 60 seconds, and stops after 300 seconds or the attempt limit.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the widened uploader OSS readback retry window.
Description check ✅ Passed The description covers the change, issue relationship, scope, dependencies, validation, rollout, rollback, and fail-closed behavior.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/upload-readback-window

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proerror77
proerror77 enabled auto-merge (squash) August 1, 2026 05:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7c64f011c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread rust_hft/tools/collector/src/polymarket_upload.rs
Comment thread rust_hft/tools/collector/src/polymarket_upload.rs
@proerror77
proerror77 force-pushed the codex/upload-readback-window branch from c7c64f0 to fb3a5d9 Compare August 1, 2026 05:41
proerror77 pushed a commit that referenced this pull request Aug 1, 2026
CodeRabbit review on #565: a hung readback command could receive the
full per-attempt timeout 12 times, and always-failing fake runners
made the suite sleep 11x5s. Cap aggregate backoff at 120s and use a
5ms retry delay under cfg(test).

Refs #556
proerror77 pushed a commit that referenced this pull request Aug 1, 2026
CodeRabbit review on #565: a hung readback command could receive the
full per-attempt timeout 12 times, and always-failing fake runners
made the suite sleep 11x5s. Cap aggregate backoff at 120s and use a
5ms retry delay under cfg(test).

Refs #556
@proerror77
proerror77 force-pushed the codex/upload-readback-window branch from 452a7ac to 88c96ae Compare August 1, 2026 06:01

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@rust_hft/tools/collector/src/polymarket_upload.rs`:
- Line 1886: Update download_remote_artifacts_with and its retry loop so every
runner invocation and retry delay is capped by the remaining
OSS_READBACK_MAX_WALL_CLOCK deadline; pass the deadline into the function, stop
immediately when no duration remains, and preserve existing retry behavior
otherwise. Add a deterministic regression test covering exhaustion during
sequential downloads or retry delay.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 19d767ac-25fa-4a86-b0d8-a71f0c49df43

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed335b and 339b76f.

📒 Files selected for processing (1)
  • rust_hft/tools/collector/src/polymarket_upload.rs

Comment thread rust_hft/tools/collector/src/polymarket_upload.rs Outdated
proerror77 pushed a commit that referenced this pull request Aug 1, 2026
CodeRabbit review on #565: a hung readback command could receive the
full per-attempt timeout 12 times, and always-failing fake runners
made the suite sleep 11x5s. Cap aggregate backoff at 120s and use a
5ms retry delay under cfg(test).

Refs #556
@proerror77
proerror77 force-pushed the codex/upload-readback-window branch from 339b76f to ea684bd Compare August 1, 2026 06:21
proerror77 pushed a commit that referenced this pull request Aug 1, 2026
…ck budget

CodeRabbit round 3 on #565: per-command 60s caps did not account for
the remaining loop budget (3 sequential downloads could overrun it).
Each command timeout is now min(oss_timeout, 60s, budget_remaining).

Refs #556
Sonic Shih added 4 commits August 1, 2026 14:57
Production evidence (2026-08-01, gate invocations 3736e02d, 842bab38,
6d1bfe7f): a just-PUT object 404s for a few seconds past the 3x1s
window before becoming HEAD-able, so the uploader's own readback
fails and every shadow gate dies on 'combined gate evidence'.
Widen to 12 attempts x 5s (~60s).

Refs #556
CodeRabbit review on #565: a hung readback command could receive the
full per-attempt timeout 12 times, and always-failing fake runners
made the suite sleep 11x5s. Cap aggregate backoff at 120s and use a
5ms retry delay under cfg(test).

Refs #556
Second-round review: the 120s backoff cap was dead code (11 x 5s
sleeps = 55s maximum), and each download inherited the 300s upload
timeout (36 downloads => hours worst case). Now: per-file readback
timeout capped at 60s and the whole retry loop bounded by a 300s
wall-clock budget.

Refs #556
…ck budget

CodeRabbit round 3 on #565: per-command 60s caps did not account for
the remaining loop budget (3 sequential downloads could overrun it).
Each command timeout is now min(oss_timeout, 60s, budget_remaining).

Refs #556
@proerror77
proerror77 force-pushed the codex/upload-readback-window branch from 80c7f1c to 74eb889 Compare August 1, 2026 06:57
@proerror77
proerror77 merged commit 5dea0da into main Aug 1, 2026
42 checks passed
@proerror77
proerror77 deleted the codex/upload-readback-window branch August 1, 2026 07:08
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.

1 participant