Skip to content

fix: tiered CopyObject converges under its pre-forward version; converge retries once - #228

Merged
ovaistariq merged 2 commits into
mainfrom
fix/tiered-copy-converge
Sep 18, 2026
Merged

ovaistariq merged 2 commits into
mainfrom
fix/tiered-copy-converge

Conversation

@ovaistariq

@ovaistariq ovaistariq commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the three findings the bots raised on release PR #227:

  • CopyObject out-deleted racing PUTs in tiered mode (greptile P1): it was the last mode-blind mutating handler still using the unconditional convergeInvalidation. A small PUT acked while the copy was in flight — the local tier's only copy — lost its metadata behind the copy's 200 and read as an authoritative 404. It now claims the key (cancelling/excluding re-tiers) and captures a pre-forward token, converging via convergeTieredDelete — the same rule the forwarded DELETE paths use. Regression test fails against the old code, passes with the fix.
  • Tiered converge didn't retry transient errors (greptile P1): a converge that never lands leaves deleted metadata authoritative until TTL (a marker makes HEAD report the deleted object present). It now retries once, keeping the version guard — the discipline convergeInvalidation already applied.
  • Unreachable src.err branch in the engine PUT error path (cursor, low): removed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L5DLvDTo9dhD38YQXhHpU9


Note

Medium Risk
Changes tiered cache/metadata convergence for CopyObject and DELETE converge retries—incorrect version guards could cause stale metadata or data loss in race scenarios, but behavior is aligned with existing DELETE paths and heavily tested.

Overview
Tiered CopyObject now matches forwarded DELETE convergence: it claims the destination key (cancelling re-tiers), records the pre-forward metadata version, and after a successful upstream copy calls convergeTieredDelete instead of unconditional cache invalidation and warming. That stops a racing small PUT on the destination from being wiped when its local-tier copy is the only copy.

convergeTieredDelete retries DeleteMetaIfVersion once on failure, still using the same version token (aligned with convergeInvalidation).

localstore PutObject error handling drops a duplicate unreachable src.err branch.

Regression tests cover copy-vs-racing-PUT and single-retry converge behavior.

Reviewed by Cursor Bugbot for commit 30d5b8f. Bugbot is set up for automated code reviews on this repo. Configure here.

…rge retries once

CopyObject was the last mode-blind mutating handler whose post-success
converge out-deleted racing writers in tiered mode: a small PUT acked
while the copy was in flight — the local tier's only copy — lost its
metadata behind the copy's 200. It now claims the key and captures a
pre-forward token like the forwarded DELETE, converging only the state
the copy displaced. The tiered converge also retries once on a transient
error (the discipline the proxy-mode converge already had), keeping the
version guard: a converge that never lands leaves deleted metadata
authoritative until TTL. Dead unreachable branch removed from the
engine's PUT error path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5DLvDTo9dhD38YQXhHpU9
singler
singler previously approved these changes Sep 17, 2026
@greptile-apps

greptile-apps Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously requested retry coverage is now present and no new actionable issue was identified.

Summary

This PR makes tiered CopyObject converge destination metadata under its pre-forward version and retries transient tiered convergence failures once.

  • Claims the destination during tiered copies and uses version-guarded convergence to preserve racing writes.
  • Retries DeleteMetaIfVersion once with the same version token.
  • Adds regression coverage for racing PUT preservation, successful retry, stable version guards, and persistent failure.
  • Removes an unreachable error branch from the local-store PUT path.

Reviews (2) · Last reviewed commit: "test: tiered converge retry — same versi..."

Comment thread proxy/service.go
@ovaistariq

Copy link
Copy Markdown
Contributor Author

@greptile review — added the converge-retry regression test (same version across the retry, exactly two attempts on persistent failure).

@ovaistariq
ovaistariq merged commit ec20a03 into main Sep 18, 2026
9 checks passed
@ovaistariq
ovaistariq deleted the fix/tiered-copy-converge branch September 18, 2026 18:48
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 18, 2026
@tigrisdata-argocd-bot

Copy link
Copy Markdown

🎉 This PR is included in version 1.22.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants