Skip to content

fix(pools): keep the latest copy when a delete fails in one pool - #257

Merged
Vonng merged 5 commits into
pgsty:mainfrom
botdeniska:fix/delete-copies-across-pools
Oct 24, 2026
Merged

Vonng merged 5 commits into
pgsty:mainfrom
botdeniska:fix/delete-copies-across-pools

Conversation

@botdeniska

@botdeniska botdeniska commented Oct 23, 2026 •

Copy link
Copy Markdown

Contribution Licensing (no CLA, inbound=outbound, DCO required)

This pull request contributes to PGSTY SILO (pgsty/silo). Code contributions
are accepted under AGPL-3.0-or-later, the same license as the server.
This project does not use a CLA or require a separate Apache-2.0 license grant.
By submitting this pull request I represent that I have the right to contribute
the code changes under this repository's
GNU Affero General Public License v3.0 or later
and retain copyright in my original work. Existing copyright and license
notices remain intact; separately licensed material keeps its applicable terms.
Every commit must carry a DCO Signed-off-by trailer
(git commit -s) certifying the
Developer Certificate of Origin — see
CONTRIBUTING.md.

Description

Fixes #256. A failed multi-pool delete must not remove the authoritative copy while an older copy remains, and a bulk response must report failures for the correct key/version.

Single-object deletion retires older copies first. Bulk data deletion resolves each addressed object/version under the shared write lock, batches cleanup of every non-authoritative pool, then deletes only the authoritative copies whose cleanup succeeded. Pools remain parallel within each stage. A failed item does not roll back unrelated successful items. The online-drive check remains an early rejection of known quorum loss, not the consistency guarantee.

The change also preserves replication state in early error results, so the HTTP handler can map every failure back to its original request item. Retrying partial physical deletions combines successful removals and reliable already-absent replies toward quorum; marker creation and replication-state updates still require successful writes. Marker creation follows explicit data/version removal within a mixed batch, preserving suspended null-marker behavior.

Tiered and restored copies use the existing per-object reconciliation, so retiring one copy cannot queue a shared remote object for premature garbage collection. This also covers unversioned single DELETE. Ordinary local data retains staged bulk I/O.

Delete-marker placement for single DELETE skips known unwritable pools when another readable copy can accept it. No overwrite affinity, allocation-policy change, global setting, dependency change, or storage-format change is introduced. The branch includes current main and does not depend on #245.

Validation

  • Deterministic failure inside DeleteVersions after all drives are initially online: EIO, simulated disconnect, and real filesystem permission denial; two/three pools and every authoritative-pool position; unversioned, null, and UUID versions.
  • Assert per-item failure, no attempted deletion of the authoritative copy, correct GET/version listing after recovery, and successful retry. Unrelated items can succeed.
  • HTTP replication response identity; partial cleanup and retry; different authoritative pools in one batch; equal ModTime; unreadable metadata; cancellation and concurrent PUT; null purge plus marker creation; physical-delete versus marker/update quorum evidence; shared tier references, restored objects, distinct tier references, and SkipFreeVersion.
  • New tests fail against the original PR production code; corrected delete regressions pass three repeated race runs.
  • Local full cmd and internal/... regressions, related Object Lock/migration/API race tests, go vet ./..., go build ./..., make build, and make verifiers pass. The optional typos executable is absent locally; the other verifiers ran. After the tier follow-up, the final combined delete/tier/quorum race suite passed three runs, and final-head CI reran full packages.
  • Final-head Go CI (including supported-target cross compilation), DCO, and VulnCheck pass on a2073d25142ced87b9bde8afa38aef2d0383768e.
  • Real mcli smoke test on two isolated directory-backed pools passes: read baseline data, plain bulk deletion, UUID deletion, enabled/suspended delete markers, write new data, and restart the old server over the same data. This verifies restart upgrade/rollback, not distributed rolling upgrade.
  • 60 alternating object-layer performance samples: 4 KiB objects, 8 workers, 1/2/3 pools, 1/2/3 copies, single DELETE and batches of 64. All requests succeeded. Paired median bulk throughput changes versus main were +4.5% (1 pool), -7.8%/+2.2% (2 pools, 1/2 copies), and -16.9%/-14.5% (3 pools, 1/3 copies). Each scenario has only three pairs, all directory disks share one Apple SSD/APFS device, and tiered-object performance was not measured.
  • Supplemental boundary replay retains two pre-existing suspended-version cases involving UUID-only history, degraded bulk marker creation, and later explicit null-marker purge. They fail in main as well and are not claimed fixed here.

Compatibility and cost

Multi-pool data deletion adds bounded metadata lookup and two bulk I/O stages while holding the shared key locks. Single-pool scheduling and marker-only batching retain their existing paths. Tiered/restored data uses sequential per-item reconciliation within that subset; ordinary local data continues to use bulk I/O. SDK HTTP-200 per-item errors require the caller to handle failed items; automatic transport retry is not a data-consistency mechanism. Old coordinators retain the previous behavior during a rolling upgrade. No release or deployment is part of this PR.

Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>
Vonng added 3 commits October 24, 2026 15:13
…fety

Signed-off-by: Feng Ruohang <rh@vonng.com>
Resolve each addressed version under the shared write lock, keep bulk I/O parallel within cleanup and authoritative-deletion stages, and preserve replication response identity on errors. Treat reliable absence as successful physical cleanup on retries without accepting it for marker or replication-state writes.

Cover execution-time faults, partial cleanup recovery, mixed authoritative pools, response identity, null marker ordering, metadata uncertainty, cancellation, and concurrent PUT. Run the delete consistency regressions under race in CI.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Reuse deleteObjectReconciled for tiered bulk entries and unversioned single deletes. Its reference-aware cleanup avoids scheduling remote contents for garbage collection while another copy still needs them, including restored copies. Keep the normal local-data bulk path unchanged.

Extend existing tier GC regressions to bulk and unversioned deletes and include them in the multi-pool race CI step.

Signed-off-by: Feng Ruohang <rh@vonng.com>
@Vonng
Vonng merged commit aafe4f8 into pgsty:main Oct 24, 2026
8 checks passed
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.

Multi-pool: a DELETE that fails in one pool still serves an older copy of the object, and a multi-object delete reports it as deleted

2 participants