Skip to content

feat(simulation): let the simulation store S3-style object tag sets on xorbs - #958

Open
sirahd wants to merge 8 commits into
mainfrom
sirahd/simulation-object-tag-sets
Open

feat(simulation): let the simulation store S3-style object tag sets on xorbs#958
sirahd wants to merge 8 commits into
mainfrom
sirahd/simulation-object-tag-sets

Conversation

@sirahd

@sirahd sirahd commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Adds get_xorb_tag_set / set_xorb_tag_set to DeletionControlableClient, carrying an ObjectTagSet of ordered (key, value) pairs. Writes replace the whole set, as S3 PutObjectTagging does.

Rename all references of the existing 32-byte ObjectTag to ETag to avoid confusion with the new ObjectTagSet

  • MemoryClient holds the sets beside xorbs; LocalClient writes a <canonical>.tagset JSON sidecar, matching the existing .gctag convention of keeping GC state next to the object rather than in it
  • Exposed as GET/PUT /simulation/xorbs/{hash}/tag_set, so SimulationControlClient picks it up like the other deletion controls
  • Tagging an absent xorb errors rather than leaving an orphan sidecar

🤖 Generated with Claude Code


Note

Medium Risk
Changes GC/compare-and-delete semantics (stable etags vs prior mtime-based tags) and adds new deletion-control surface area; impact is mostly confined to simulation/test clients but affects how conditional deletes behave under re-upload.

Overview
The simulation CAS layer now separates S3-style object ETags (32-byte, content/key-derived, for compare-and-delete) from object tag sets (ObjectTagSet key/value pairs). The old ObjectTag name and *_if_tag_matches / list_*_with_tags APIs are renamed to ETag equivalents; conditional delete and listing use etags derived from object key + stored length (or payload), not filesystem mtime, so byte-identical re-uploads keep the same etag—matching S3 and surfacing the “stale etag still matches” GC hazard, which last-upload tags are meant to address.

get_xorb_tag_set / set_xorb_tag_set are added on DeletionControlableClient, with wholesale replace semantics like PutObjectTagging. LocalClient persists tags in a .tagset JSON sidecar (etag unchanged), stamps last-upload on every xorb upload, and clears sidecars on delete/conditional delete. MemoryClient holds tag sets beside xorbs with tighter locking on upload/delete/conditional delete. The local simulation server exposes GET/PUT …/tag_set and renames HTTP paths to *_etags / etag_delete.

Tests are updated for the renames and extended to cover tag round-trip, upload stamping, sidecar cleanup, and stable etag across identical re-upload.

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

…n xorbs

`DeletionControlableClient` gains `get_xorb_tag_set` / `set_xorb_tag_set`,
carrying an `ObjectTagSet` of ordered `(key, value)` pairs. Writes replace the
whole set, as S3 `PutObjectTagging` does.

This is deliberately separate from the existing 32-byte `ObjectTag`: writing a
tag set leaves the object's bytes, and so its `ObjectTag`, untouched. That is
the property a caller needs to record something about an object without
invalidating anything keyed on its content — xet-garbage-collection uses it to
distinguish a xorb re-uploaded since its inventory snapshot from one that has
sat untouched, which it previously could only infer from the ETag moving.

Implemented for both backing clients and exposed over the local server as
`GET`/`PUT /simulation/xorbs/{hash}/tag_set`, so `SimulationControlClient` picks
it up like the other deletion controls. `MemoryClient` holds the sets beside
`xorbs`; `LocalClient` writes a `<canonical>.tagset` JSON sidecar, matching the
existing `.gctag` convention of keeping GC state next to the object rather than
in it. Tagging an absent xorb is an error rather than leaving an orphan sidecar.

Tests cover the round trip, wholesale replacement, that the `ObjectTag` and both
xorb listings are unaffected, and the absent-xorb error, for both clients.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9b4997d. Configure here.

Comment thread xet_client/src/cas_client/simulation/local_client.rs
Comment thread xet_client/src/cas_client/simulation/memory_client.rs
sirahd and others added 2 commits September 4, 2026 13:19
With `ObjectTagSet` alongside it, "tag" now meant three things in this module:
the 32-byte compare-and-delete value, the S3-style key/value set, and the
`gc-delete` lifecycle marker. The first is an ETag stand-in, so name it one:
`ObjectTag` -> `ObjectETag`, `list_xorbs_and_tags` -> `list_xorbs_and_etags`,
`delete_xorb_if_tag_matches` -> `delete_xorb_if_etag_matches` and the shard
equivalents, `HashWithTag` -> `HashWithETag`, `TagDelete{Request,Response}` ->
`ETagDelete{Request,Response}`, plus the local bindings and doc prose.

Simulation routes move with them: `/xorbs_with_etags`, `/shards_with_etags`,
`/{hash}/etag_delete`. Both ends of that wire are in this crate, so they change
together.

The lifecycle markers — `gc_tagged_*`, `lifecycle_tag_deletion`, `.gctag` — keep
their names. They are the third concept and unrelated to either of the others.

Breaking for `simulation`-feature consumers; xet-garbage-collection is the known
one and updates with its lockfile bump.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`deletion_controls` is `#[cfg(not(target_family = "wasm"))]`, so `ObjectTagSet`
does not exist on wasm, but `MemoryClient::xorb_tag_sets` was declared
unconditionally. Native builds passed because the type is in scope there;
wasm32 failed with E0425 on the field.

Gate the field and its initializer to match the module.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sirahd
sirahd requested a review from seanses September 4, 2026 12:10
sirahd and others added 4 commits September 4, 2026 14:26
…flag

The tag set was only writable out of band, so a simulated upload produced an
untagged xorb — a consumer reading `last-upload` saw nothing during an ordinary
run and could only be exercised by a test stamping the tag by hand.

`LocalTestServerBuilder::with_upload_tagging(true)` now makes `upload_xorb`
stamp `last-upload=<unix seconds>`, mirroring CAS. It sits beside the existing
`gc_tagged_*` clear in both clients, and for the same reason: PutObject replaces
an object's whole tag set, so the stamp both records this write and clears what
was there.

Off by default and opt-in like `with_lifecycle_tag_deletion`, so no existing
behaviour changes; a xorb uploaded without it still has no tag set.

`LAST_UPLOAD_TAG_KEY` and `last_upload_tag_set_now` are exported so a consumer
can read the stamp without restating the key. xet-core reproduces the stamp;
what the value means belongs to whoever reads it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ind a flag

CAS stamps `last-upload` on every xorb write unconditionally, so gating the
simulation's stamp behind a builder flag left the default simulation diverging
from production — the thing the simulation exists to reproduce.

Modelling it after `with_lifecycle_tag_deletion` was the wrong analogy: that
flag chooses between two real S3 deletion behaviours (hard delete vs. the
lifecycle tag), whereas `last-upload` has no "off" in production.

Drops `with_upload_tagging`, `set_upload_tagging` and the per-client flag. An
uploaded xorb now always carries the tag, and the tests that asserted a fresh
xorb has an empty tag set assert the round trip instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…was written

The simulation moved a xorb's etag on every write — MemoryClient mixed in a
generation counter, LocalClient hashed the file's mtime and ctime. A comment
called that "matching production ETag semantics", and it no longer is: S3's ETag
is a function of content, and CAS deliberately stopped rewriting a xorb on
re-upload, so a byte-identical re-upload now leaves the ETag alone.

Both clients now derive the etag from what is stored. Xorbs and shards are
content-addressed, so the key fixes the content and the length distinguishes a
differently serialized rewrite of the same key. The generation counter fed
nothing else and is gone.

This makes the simulation reproduce the hazard rather than hide it: a stale etag
snapshot now matches a resurrected object, exactly as in production, and
`last-upload` is what tells them apart. `test_list_xorbs_and_etags_timestamp_changes`
asserted the old behaviour and is inverted accordingly.

It also fixes a latent oddity in the conditional deletes, which computed the
etag from a temp path they had just renamed the object onto — under the old
derivation that was a different mtime, and so a different etag, from the object
the caller had listed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot, on #958. `set_xorb_tag_set` wrote an entry that no delete path cleared,
so a hard delete left the sidecar orphaned on disk (or the map entry resident),
and the tags outlived the object they described. That is neither S3's behaviour
— DeleteObject takes an object's tags with it — nor GC's, whose `gc-delete`
write replaces the whole set and so drops `last-upload` either way. Both delete
paths in both clients now clear it.

Also makes `MemoryClient`'s tag-set accessors treat a lifecycle-tagged xorb as
gone. Every other read there already does, and `LocalClient` errors on one
because the canonical file has been renamed away, so the two backends disagreed
on the same call.

Note the re-upload half of the report was already covered: `upload_xorb` stamps
`last-upload` unconditionally and replaces the whole set, so a re-uploaded xorb
could not inherit stale tags. The leak was the orphan itself, which is what the
new tests assert — the `MemoryClient` one reads the map directly, because a
deleted xorb refuses `get_xorb_tag_set` and a re-upload would mask the leak by
overwriting the entry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@seanses seanses left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

.

if path.exists() {
Self::clear_readonly(&path);
}
std::fs::write(&path, raw)?;

@seanses seanses Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not atomic write, is there a possible case that the same file is read at the same time, in the above get_xorb_tag_set function?

To atomic write, write to a temp file and then atomic mv, i.e. rename. There is also a utility for the entire process: SafeFileCreator

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

Comment on lines +1231 to +1233
self.xorbs.write().await.remove(hash);
}
self.clear_xorb_tag_set(hash).await;

@seanses seanses Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note that removing the hash from the xorbs map and clearing the xorb tag is not one atomic operation, so re-uploading can race with delete, leading to a situation where a new xorb is uploaded, but its tag is erased.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by acquiring all of the locks in the critical section within the same scope

`set_xorb_tag_set` truncated the sidecar in place, so a concurrent
`get_xorb_tag_set` could read a half-written file; it now goes through
`SafeFileCreator`, which writes a temp file and renames.

In `MemoryClient` the xorb map and the tag set were mutated under separate
locks, so a delete could land between an upload's write and its
`last-upload` stamp and erase the tag of the xorb just written. Upload and
both delete paths now hold the maps across the whole mutation. The
conditional delete additionally compared the etag under a lock it dropped
before deleting, which let a re-upload be deleted on the strength of an
etag it no longer had; that comparison is now inside the same section.

Locks are taken `gc_tagged_xorbs` before `xorbs` to match every existing
reader, so writers cannot deadlock against one holding both.
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.

2 participants