fix(datastore): keep a peer's deletions deleted across pull and push (swamp-club #2999) #463

Merged

stack72 merged 5 commits from 2999 into main

2026-10-06 19:34:36 +00:00

stack72 commented

2026-10-06 19:28:42 +00:00

Owner

Fixes swamp-club #2999.

Problem

After one peer ran swamp data gc against a shared S3 datastore, a second peer (swamp serve in the report) pulled but kept the GC'd files in its local cache. A pull only walked the remote index, so a file whose entry a peer had deleted stayed cached with no index entry. The next push walk saw it as new, uploaded it again and merged its entry back into the shard, which undid the GC. GCS has the same code and the same bug.

Fix (S3 and GCS, identical shape)

Before a pull, push or preparePush replaces the index, it reads the last-synced .datastore-index.json from disk. After an authoritative remote read, it removes each local file whose entry was in that index but is gone from the remote one, along with any directories that leaves empty.

Unchanged only: a file is removed only when it is provably unchanged since sync: same size plus a matching sha256, or the recorded mtime for entries without a hash. Locally edited copies are kept, and a later push that walks them uploads them.
Authoritative reads only: nothing is removed when the index came from the TTL cache, from the NotFound/discovery fallback, or lists no in-scope entries. An empty index reads the same as a wiped one.
Missing objects: an index entry whose object is missing from the bucket is still dropped from the index, but its local copy is kept. The object may have gone because of an expiry rule or a mistaken delete.
Resumable: the index is saved only after removal (shard-first and monolith, including a models-scoped pull's whole-index fallback), so an interrupted sync is finished by the next one.
Scoped pushes: a dirty-shards-only push only reconciles entries in the shards it read.

Testing

Unit tests: 17 new swamp-club#2999 tests per backend, using in-memory mocks. All of them fail on main except the four safety tests. The safety tests cover: changed files are kept, missing objects keep their copy, a missing or empty remote index removes nothing, and a peer emptying a whole model's shard still removes its files. Full suites: S3 409/409, GCS 360/360.
End to end, through swamp extension source add, against MinIO (versioned) and fake-gcs-server, main vs this branch:

Scenario	`main`	This branch
Two CLI peers: A runs `data gc`, B pulls, then runs a model and pushes	6 GC'd versions re-uploaded	stay deleted
B is `swamp serve` with a workflow scheduled every minute (the report)	shard 28 → 40 (GC'd entries restored)	28 → 32 → 36
100k files of 8 KB, 94,736 deleted by a peer	B keeps 100,000; next push re-uploads 4,737	B keeps 5,264; nothing re-uploaded
First pull after that delete	3–7 s	21–24 s

Limits and residual risk

Existing orphans: files orphaned by earlier versions are no longer in a peer's last-synced index, so they are not removed. Run swamp data gc once on each affected peer. Both READMEs say this.
Index trusted, not the bucket: removal is decided by index absence and does not check the bucket listing. If a shard ever dropped an entry while its object survived, the local copy is removed, and the object remains in the bucket unindexed. That is recoverable, but no longer self-heals. Shard writes have been compare-and-swap since #2245.
Push cost: every slow-path push now parses the on-disk index, which is a full JSON parse on large repos.
First pull after a large GC: it hashes the removed set once, about 20 s per 100k files of 8 KB.

Both manifests are bumped to 2026.10.06.1.

🤖 Generated with Claude Code

Fixes swamp-club #2999. ## Problem After one peer ran `swamp data gc` against a shared S3 datastore, a second peer (`swamp serve` in the report) pulled but kept the GC'd files in its local cache. A pull only walked the remote index, so a file whose entry a peer had deleted stayed cached with no index entry. The next push walk saw it as new, uploaded it again and merged its entry back into the shard, which undid the GC. GCS has the same code and the same bug. ## Fix (S3 and GCS, identical shape) Before a pull, push or preparePush replaces the index, it reads the last-synced `.datastore-index.json` from disk. After an authoritative remote read, it removes each local file whose entry was in that index but is gone from the remote one, along with any directories that leaves empty. - **Unchanged only:** a file is removed only when it is provably unchanged since sync: same size plus a matching sha256, or the recorded mtime for entries without a hash. Locally edited copies are kept, and a later push that walks them uploads them. - **Authoritative reads only:** nothing is removed when the index came from the TTL cache, from the NotFound/discovery fallback, or lists no in-scope entries. An empty index reads the same as a wiped one. - **Missing objects:** an index entry whose object is missing from the bucket is still dropped from the index, but its local copy is kept. The object may have gone because of an expiry rule or a mistaken delete. - **Resumable:** the index is saved only after removal (shard-first and monolith, including a models-scoped pull's whole-index fallback), so an interrupted sync is finished by the next one. - **Scoped pushes:** a dirty-shards-only push only reconciles entries in the shards it read. ## Testing - **Unit tests:** 17 new swamp-club#2999 tests per backend, using in-memory mocks. All of them fail on `main` except the four safety tests. The safety tests cover: changed files are kept, missing objects keep their copy, a missing or empty remote index removes nothing, and a peer emptying a whole model's shard still removes its files. Full suites: S3 409/409, GCS 360/360. - **End to end, through `swamp extension source add`, against MinIO (versioned) and fake-gcs-server, `main` vs this branch:** | Scenario | `main` | This branch | |---|---|---| | Two CLI peers: A runs `data gc`, B pulls, then runs a model and pushes | 6 GC'd versions re-uploaded | stay deleted | | B is `swamp serve` with a workflow scheduled every minute (the report) | shard 28 → 40 (GC'd entries restored) | 28 → 32 → 36 | | 100k files of 8 KB, 94,736 deleted by a peer | B keeps 100,000; next push re-uploads 4,737 | B keeps 5,264; nothing re-uploaded | | First pull after that delete | 3–7 s | 21–24 s | ## Limits and residual risk - **Existing orphans:** files orphaned by earlier versions are no longer in a peer's last-synced index, so they are not removed. Run `swamp data gc` once on each affected peer. Both READMEs say this. - **Index trusted, not the bucket:** removal is decided by index absence and does not check the bucket listing. If a shard ever dropped an entry while its object survived, the local copy is removed, and the object remains in the bucket unindexed. That is recoverable, but no longer self-heals. Shard writes have been compare-and-swap since #2245. - **Push cost:** every slow-path push now parses the on-disk index, which is a full JSON parse on large repos. - **First pull after a large GC:** it hashes the removed set once, about 20 s per 100k files of 8 KB. Both manifests are bumped to `2026.10.06.1`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

stack72 added 5 commits

2026-10-06 19:28:47 +00:00

fix(datastore): keep a peer's deletions deleted across pull and push (swamp-club #2999 ) 334ecaa303

A pull only walked the remote index, so a file a peer deleted (for
example by `swamp data gc`) stayed in the local cache with no index
entry. The next push walk treated it as new and uploaded it again,
merging its entry back into the shard and undoing the GC.

Pull, push and preparePush in both the S3 and GCS datastores now read
the last-synced on-disk index before the remote index replaces it.
After an authoritative remote read they remove each local file whose
entry was there but is gone remotely, when the copy is provably
unchanged since sync (size plus recorded mtime or sha256), along with
any directories that leaves empty. Pull also removes the unchanged
local copy of an entry pruned because its object is missing. Locally
changed copies are kept and pushed. Nothing is removed when the remote
index came from the TTL cache or the NotFound/discovery fallback, or
lists no in-scope entries.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(datastore): keep the local copy when an index entry's object is missing (swamp-club #2999 ) d208d55f54

Review follow-up. A pull no longer removes the unchanged local copy of
an entry it prunes because the object is missing from the bucket. That
case is not a peer's GC (a GC removes the index entry too, which the
last-synced comparison already handles); the object may have gone to
an expiry rule or a mistaken delete, and the local copy may be the
last one. The entry is still dropped and the copy is kept for a later
push to restore, as before.

Also moves localHasAllRemoteEntries' doc comment back above it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(datastore): finish peer-deletion removals across interrupted pulls (swamp-club #2999 ) 9d54669f51

Review follow-up for both the S3 and GCS datastores:

- A shard-index pull now saves the assembled index only after removing
  peer-deleted files. Saved first, an interrupted pull left the
  unremoved files out of the last-synced index, where no later sync
  would find them.
- A file that cannot be removed is reported by name instead of being
  skipped silently.
- Removal checks the sha256 whenever the entry has one; the recorded
  mtime is only used for hashless entries, so a same-second same-size
  rewrite on a coarse-mtime filesystem is not removed.
- Emptied-directory cleanup splits on both path separators.
- The kept-file warning and READMEs no longer promise the next push
  uploads a kept file; only a push whose walk reaches it does.

Adds tests for an interrupted pull and for a peer emptying a whole
model's shard.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(datastore): finish interrupted removals on monolith-index repos too (swamp-club #2999 ) d66248f0c9

Review follow-up for both the S3 and GCS datastores. pullIndex wrote
the fetched monolith index to disk before the caller removed
peer-deleted files, so on a repo still using the monolithic index an
interrupted pull, push or preparePush could leave the unremoved files
out of the last-synced index. pullIndex now takes persist: false, and
those callers save the fetched index only after the removal, as the
shard-first path already does.

Adds an interrupted monolith-index pull test to both backends.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(datastore): reconcile a models-scoped pull's whole-index fallback (swamp-club #2999 )

CI / Review Integrity (pull_request) Successful in 1m4s

Details

CI / Validate Attestation (pull_request) Successful in 58s

Details

0154a48072

Review follow-up for both the S3 and GCS datastores. A models-scoped
pull whose model has no shard fell back to pullIndex, which saved the
whole remote index before any peer-deleted files were removed, so they
were orphaned (on shard-first repos it also rebuilt and uploaded a
monolith from a listing). The fallback now reads the whole index the
same way an unscoped pull does, shards first, and reconciles before
saving.

pullIndex now reports a remote read per call through a report option
instead of an instance flag a concurrent call could reset, and the
save-after-removal writes share a saveIndex helper.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

stack72 merged commit 8ecc124a60 into main

2026-10-06 19:34:36 +00:00

stack72 deleted branch 2999

2026-10-06 19:34:36 +00:00

stack72 referenced this pull request from a commit

2026-10-06 19:34:36 +00:00

fix(datastore): keep a peer's deletions deleted across pull and push (swamp-club #2999) (#463)