lore-aws: Carry fragment metadata on the S3 object by mjansson · Pull Request #157 · EpicGames/lore

mjansson · 2026-08-03T21:34:01Z

Problem

The AWS store keys S3 objects by the content hash while the object holds one representation of that content. Two writers may hold different valid representations of the same content (LZ4 and Zstd of the same bytes) and both address the same key. The fragment describing which representation is stored lives in a separate DynamoDB record, so two independently written records are required to agree. They can fail to:

Concurrent cross-partition writers — two writers upload different representations and publish different fragments. The interleaving leaving writer A's fragment beside writer B's payload is permanent and requires no failure.
A lost metadata write — a writer replaces the object, then fails to publish. The stored fragment describes bytes that are gone, and the affected partition cannot repair it: its re-put sees a full match and does nothing.

Both yield a payload that cannot be decompressed, reported as an internal size mismatch, undetected until a read fails.

Separately, lookup short-circuits to MatchNone when MatchFull is requested, so put's
MatchPartition and MatchHash arms are unreachable — content already durable in one partition is uploaded again for another.

Change

The fragment travels on the object as x-amz-meta-lore-fragment: <flags hex>:<size_payload>:<size_content>. Object metadata is part of the object version, so a GetObject returns headers and body from the same version, and a full-object PUT is atomic. Last-writer-wins becomes safe; the disagreement is removed rather than policed.

The DynamoDB fragment metadata table becomes a fragment state table: row presence means the hash exists, plus obliteration state. The existence probe stays a single GetItem with no S3 request.

No S3 or DynamoDB feature beyond the plainest ones — no conditional S3 write, no ETag
compare-and-set, no object-age heuristic, no transaction. The one conditional DynamoDB write is an attribute_not_exists create guarding an obliteration mark.

Flows

Put — probe association ∥ state, in parallel:

Probe	Action
state = Obliterating	`SLOWDOWN`
state present, associated	`OK`
state present, not associated, payload supplied	associate only, no S3
state present, not associated, no payload	`Payload buffer required`
no state, or Obliterated	upload → conditional create → associate

Ordering is load-bearing: object, then state row, then association. The reverse leaves a hash claiming to exist with no bytes, unrepairable by retry because the next put takes the
already-stored branch. Every step is idempotent, so a retry converges from any interruption.

Get — association GetItem ∥ GetObject. The fragment arrives on the response carrying the bytes; the size check is self-consistency on one object.

Query — association ∥ state, no S3. It is called once per fragment stored on the ingress path (query_match_full), which ADR-00008 identifies as the busiest path in the server.

ImmutableStore::get_metadata — new, defaulting to a MatchFull query; the AWS store overrides it with a HeadObject. The only path spending an S3 request purely on metadata.

Obliterate — mark → delete association (compliance discharged) → drain → count → release mark, or recurse sub-fragments, delete payload, tombstone.

Costs

Operation	main	This branch
Put, new content	1–3 DDB reads, 1 S3 PUT, 2 DDB writes	2 DDB reads (parallel), 1 S3 PUT, 2 DDB writes
Put, dedup across partitions	not supported — re-uploads	2 DDB reads, 1 DDB write, no S3
Get	2 DDB reads, 1 S3 GET	1 DDB read, 1 S3 GET (parallel)
Query	2 DDB reads	2 DDB reads (parallel), no S3
Copy	2 DDB reads, 1 DDB write	1 DDB read, 1 DDB write
get_metadata	2 DDB reads	2 DDB reads + 1 `HeadObject`

Production code in lore-aws grows ~460 lines; dynamodb.rs is untouched. This is not a
simplification measured in lines.

Rollout

Full stop, then full start — no mixed fleet. That removes the dual-write phase: nothing writes the old shape, and the guarantee holds from the first write. There is no clean rollback once writes are served — content written after cut-over is described only on its object.

Configuration is non-breaking: dynamodb_metadata_table is accepted as an alias for
dynamodb_fragment_state_table. The optional dynamodb_fragment_metadata_table gates the fallback read for pre-cut-over objects; leaving it unset declares no such object exists, so an object with no metadata is reported as damaged rather than described from a row that cannot be about it.

Testing

94 tests in lore-aws. The fake supports fault injection across nine operations, which makes error paths reachable. Every behavioural guard was verified by removing it and confirming the test fails.

Notable: concurrent_writers_cannot_tear_the_fragment_from_its_payload (4 writers × 4
representations × 64 randomized rounds), sub-fragment obliteration and its failure aggregation, the drain window, the post-upload obliteration race, and lost-payload detection and repair.

Known limitations

A lost payload (object gone, reference remains) is detected on both reads and repaired by clearing the state row; reconciling the whole population is left to the planned obliteration work table. A crashed obliteration leaves a mark that nothing clears. The obliteration drain narrows but does not close its race.

Design and migration: docs/proposals/2026-08-03-fragment-metadata-on-the-s3-object.md. Decision and alternatives: ADR-00018, which supersedes ADR-00006.

The AWS store keys S3 objects by the content hash while the object holds one representation of that content. Two writers may hold different valid representations of the same content and both address the same key, and the fragment describing which one is stored lives in a separate DynamoDB record. Two independently written records are required to agree, and they can fail to: concurrently, when two writers publish different fragments for the same key, or after a lost metadata write. Either way the payload cannot be decompressed, no retry fixes it, and nothing notices until a read fails. Store the fragment as S3 object metadata on the object holding the payload, as x-amz-meta-lore-fragment: <flags hex>:<size_payload>:<size_content>. Object metadata is part of the object version, so a GetObject returns headers and body from the same version and a full-object PUT is atomic. Last-writer-wins becomes safe: the disagreement is removed rather than policed, and no S3 or DynamoDB feature beyond the plainest ones is needed -- no conditional S3 write, no entity tag compare-and-set, no object-age heuristic, no transaction. The DynamoDB fragment metadata table becomes a fragment state table: row presence means the hash exists, plus obliteration state. That keeps the existence probe a single GetItem with no S3 request, which is what makes cross-partition deduplication possible at no additional cost -- previously lookup short-circuited so put resolved to a full triplet match or an upload, and content already durable in one partition was uploaded again for another. Consequences worth knowing. Query is answered from DynamoDB alone, because it runs once per fragment stored on the ingress path; it therefore reports whether a payload is durable rather than what representation is stored, and reading a representation moves to a new ImmutableStore::get_metadata. Get drops a DynamoDB read. A payload that is lost while still referenced is now counted, logged, and made recoverable by clearing its state row, which is only safe because that row carries no representation. Obliteration discharges its compliance obligation in one atomic DeleteItem and drains before counting so an in-flight put is counted rather than lost. Rollout is a full stop followed by a full start, with no mixed fleet and no clean rollback once writes are served. dynamodb_fragment_state_table is required; the older dynamodb_metadata_table spelling carries over as an alias for dynamodb_fragment_metadata_table, which gates the fallback read for objects predating the change. ImmutableStore::get_metadata is required rather than defaulted. A default delegating to query is right for a store whose query reports the representation, and silently wrong for a wrapper that forwards query alone -- the wrapper answers, the inner override never runs, and the caller gets a well-formed fragment with no sizes and no error. Requiring it makes that a compile error instead. Composite resolves local then durable, without the replica fan-out query does, since a representation is the same wherever it is read from. StoreResult stops returning a fragment for the same reason. A dispatched write reports before its leader compresses, so the only representation it could name is the one the caller passed in, and that is what it handed back on every path but the inline one. It now reports what the caller cannot know: the content size, and whether the payload is stored locally and durably. Write wrappers down to store_raw_local return an address alone; write_from_file also returns the content size, taken from the read that fed the hash so it agrees with the address, which stating the file again after the write does not. TrackedResult carries nothing, since no one read the fragment a leader or follower yielded. The write observer still takes a fragment -- the FragmentWrite event classifies on the payload flags and reports the payload size -- but assembles it from the caller's input rather than from the result. The state serialization trace loses its byte figures, the one place a stored representation reached a reader. Integration tests cover the migration read path against a real MinIO and DynamoDB: an object stored the way that era stored it -- bare bytes, fragment in a row -- reads back intact when the fragment metadata table is configured, and is reported as damaged when it is not. Both fail if the fallback is removed. Design and migration: docs/proposals/2026-08-03-fragment-metadata-on-the-s3-object.md Decision and alternatives: ADR-00018, which supersedes ADR-00006. Signed-off-by: Mattias Jansson <mjansson@gmail.com>

The comment saying the remote's metadata operation was not yet wired through survived the change that wired it through, and now sits directly above the comment describing what the method actually does. Signed-off-by: Mattias Jansson <mjansson@gmail.com>

epic-lore-bot · 2026-08-04T14:08:26Z

Imported as Lore CR-288.

peter-lockhart-pub

Sounds good, and will greatly simplify the Oodle migration as well not having to worry about so many races. I'll reach out to you about how we can track the follow up PRs and anything else we need before this merges.

Pre-approving

peter-lockhart-pub · 2026-08-05T09:31:30Z


+    /// Answered by this store's own `query`, so it reports whether the payload is durable rather
+    /// than what representation is stored. Wiring the remote's metadata operation through is
+    /// outstanding; until then a caller wanting a representation must ask the store holding it.


Multi-region deployments who leverage the replicated_store will behave inconsistently if the main region is backed by a store that is silently not implementing query as this fn expects

I'm seeing there really isn't any usages of get_metadata so this is safe to land an address in a follow up I guess?

peter-lockhart-pub · 2026-08-05T09:36:53Z

+    }
+
+    fn is_obliteration(self) -> bool {
+        self != Self::Stored


I would prefer more defensive if there are more enum states and explicitly check for the obliteration states

peter-lockhart-pub · 2026-08-05T09:43:45Z

+///
+/// Non-zero means content has been lost. The read itself is reported as a plain not-found, which is
+/// indistinguishable from content that was never stored, so without this the loss is silent.
+const METRICS_MISSING_PAYLOAD_METRIC_NAME: &str = "store.immutable.missing_payload";


IIRC the Instrument Provider will scope this already for you (which is currently urc.store.immutable.aws) so you just need some counter like naming num_missing_payload

peter-lockhart-pub · 2026-08-05T09:46:14Z

@@ -341,8 +493,16 @@ pub struct AwsImmutableStore {
    task_queue: TaskQueue<BatchTaskResult>,
    bucket: String,
    fragments_table_name: Arc<str>,
-    metadata_table_name: Arc<str>,
+    /// Table of [`FragmentStateEntry`] rows. Named "metadata" for historical reasons; it holds


Not named metadata

peter-lockhart-pub · 2026-08-05T09:47:38Z

@@ -366,6 +529,9 @@ impl AwsImmutableStore {
        let labels_obliterate = provider.get_labels_for_operation_context("obliterate");
        let labels_query = provider.get_labels_for_operation_context("query");
        let labels_copy = provider.get_labels_for_operation_context("copy");
+        let labels_get_metadata = provider.get_labels_for_operation_context("get_metadata");
+        let missing_payload_counter = provider.counter(METRICS_MISSING_PAYLOAD_METRIC_NAME);
+        let labels_missing_payload = provider.get_labels_for_operation_context("missing_payload");


I don't think these labels add anything - these labels are used to differentiate metrics in a common histogram - but you have a distinct counter for this metric so the labels aren't needed

peter-lockhart-pub · 2026-08-05T09:49:17Z

-            });
+            }),
+            Some(FragmentState::Obliterating | FragmentState::Obliterated) => {
+                debug!("Query found obliterated fragment at address {address}");


tracing fields please rather than string interpolation

peter-lockhart-pub · 2026-08-05T09:55:31Z

-                StoreError::internal_with_context(e, "DynamoDB metadata write failed")
+        match self.load_state(address.hash).await {
+            Ok(Some(FragmentState::Stored)) => {
+                if let Err(error) = self.clear_state(address.hash).await {


What about the race between clearing the state and someone doing a put to reinstate it? It might just be safer to not try to clear it up. We don't write the state first and just the s3 object, so this case shouldn't happen

peter-lockhart-pub · 2026-08-05T10:17:53Z

-        // amount of data we read does not match the expected size, we should fail the request.
-        // However, if it's off by exactly the size of fragment metadata, and we're in force-write
-        // mode, assume it's ok.
-        let buffer = if buffer_size > payload_size


I appreciate it is probably unlikely now, but do we still not need this workaround as there could be items out there that are like this?

And when it comes to rewriting the immutable data in a follow up to use this new state table, that is when we can ditch it?

peter-lockhart-pub · 2026-08-05T10:24:59Z

@@ -1408,118 +1866,63 @@ impl ImmutableStoreTrait for AwsImmutableStore {
            // expect this to be invoked, the log output in this method is intentionally very verbose.
            let span = tracing::Span::current();

-            let original_metadata = self
-                .metadata_with_size_validation(address.hash)
+            let Some(state) = self


nit: pretty sure the instrument() calls aren't needed in this fn since this fn is already wrapped in an instrument and will enter the span for us

peter-lockhart-pub · 2026-08-05T10:32:16Z

    }

+    /// Local first, then durable. Unlike `query` this does not fan out to read replicas: a
+    /// representation is the same wherever it is read from, so the first store that has it answers.


Another to follow up on in another PR (and I'm happy to do it) - but we should be using read replicas for this. The point of the read replicas is that we don't have to go to the durable store for some bit of data. In multi-region deployments going to the durable store is costly.

I think this comment is wrong

## Problem The AWS store keys S3 objects by the *content* hash while the object holds one *representation* of that content. Two writers may hold different valid representations of the same content (LZ4 and Zstd of the same bytes) and both address the same key. The fragment describing which representation is stored lives in a separate DynamoDB record, so two independently written records are required to agree. They can fail to: 1. **Concurrent cross-partition writers** — two writers upload different representations and publish different fragments. The interleaving leaving writer A's fragment beside writer B's payload is permanent and requires no failure. 2. **A lost metadata write** — a writer replaces the object, then fails to publish. The stored fragment describes bytes that are gone, and the affected partition cannot repair it: its re-put sees a full match and does nothing. Both yield a payload that cannot be decompressed, reported as an internal size mismatch, undetected until a read fails. Separately, `lookup` short-circuits to `MatchNone` when `MatchFull` is requested, so `put`'s `MatchPartition` and `MatchHash` arms are unreachable — content already durable in one partition is uploaded again for another. ## Change The fragment travels on the object as `x-amz-meta-lore-fragment: <flags hex>:<size_payload>:<size_content>`. Object metadata is part of the object version, so a `GetObject` returns headers and body from the same version, and a full-object PUT is atomic. Last-writer-wins becomes safe; the disagreement is removed rather than policed. The DynamoDB fragment metadata table becomes a **fragment state table**: row presence means the hash exists, plus obliteration state. The existence probe stays a single `GetItem` with no S3 request. **No S3 or DynamoDB feature beyond the plainest ones** — no conditional S3 write, no ETag compare-and-set, no object-age heuristic, no transaction. The one conditional DynamoDB write is an `attribute_not_exists` create guarding an obliteration mark. ## Flows **Put** — probe association ∥ state, in parallel: | Probe | Action | | --- | --- | | state = Obliterating | `SLOWDOWN` | | state present, associated | `OK` | | state present, not associated, payload supplied | associate only, **no S3** | | state present, not associated, no payload | `Payload buffer required` | | no state, or Obliterated | upload → conditional create → associate | Ordering is load-bearing: object, then state row, then association. The reverse leaves a hash claiming to exist with no bytes, *unrepairable by retry* because the next put takes the already-stored branch. Every step is idempotent, so a retry converges from any interruption. **Get** — association `GetItem` ∥ `GetObject`. The fragment arrives on the response carrying the bytes; the size check is self-consistency on one object. **Query** — association ∥ state, **no S3**. It is called once per fragment stored on the ingress path (`query_match_full`), which ADR-00008 identifies as the busiest path in the server. **`ImmutableStore::get_metadata`** — new, defaulting to a `MatchFull` query; the AWS store overrides it with a `HeadObject`. The only path spending an S3 request purely on metadata. **Obliterate** — mark → delete association (compliance discharged) → drain → count → release mark, or recurse sub-fragments, delete payload, tombstone. ## Costs | Operation | main | This branch | | --- | --- | --- | | Put, new content | 1–3 DDB reads, 1 S3 PUT, 2 DDB writes | 2 DDB reads (parallel), 1 S3 PUT, 2 DDB writes | | Put, dedup across partitions | not supported — re-uploads | 2 DDB reads, 1 DDB write, **no S3** | | Get | 2 DDB reads, 1 S3 GET | **1 DDB read**, 1 S3 GET (parallel) | | Query | 2 DDB reads | 2 DDB reads (parallel), **no S3** | | Copy | 2 DDB reads, 1 DDB write | **1 DDB read**, 1 DDB write | | get_metadata | 2 DDB reads | 2 DDB reads + 1 `HeadObject` | Production code in `lore-aws` grows ~460 lines; `dynamodb.rs` is untouched. This is not a simplification measured in lines. ## Rollout **Full stop, then full start — no mixed fleet.** That removes the dual-write phase: nothing writes the old shape, and the guarantee holds from the first write. **There is no clean rollback once writes are served** — content written after cut-over is described only on its object. Configuration is non-breaking: `dynamodb_metadata_table` is accepted as an alias for `dynamodb_fragment_state_table`. The optional `dynamodb_fragment_metadata_table` gates the fallback read for pre-cut-over objects; leaving it unset declares no such object exists, so an object with no metadata is reported as damaged rather than described from a row that cannot be about it. ## Testing 94 tests in `lore-aws`. The fake supports fault injection across nine operations, which makes error paths reachable. Every behavioural guard was verified by removing it and confirming the test fails. Notable: `concurrent_writers_cannot_tear_the_fragment_from_its_payload` (4 writers × 4 representations × 64 randomized rounds), sub-fragment obliteration and its failure aggregation, the drain window, the post-upload obliteration race, and lost-payload detection and repair. ## Known limitations A lost payload (object gone, reference remains) is detected on both reads and repaired by clearing the state row; reconciling the whole population is left to the planned obliteration work table. A crashed obliteration leaves a mark that nothing clears. The obliteration drain narrows but does not close its race. Design and migration: `docs/proposals/2026-08-03-fragment-metadata-on-the-s3-object.md`. Decision and alternatives: ADR-00018, which supersedes ADR-00006. ``` Imported-PR: #157 Imported-From: fe1f58e Imported-Base: 1029e04 Imported-Merge: 0cd7877 Imported-Author: Mattias Jansson (mjansson) GH-URL: #157 ``` Lore-RevId: 483 Lore-Signature: 5397c2a138a831f2183cbaed40e1a4f8c66e98a9165a04af008283657edeb39d

mjansson · 2026-08-06T05:40:54Z

Merged in fe4f4e5

mjansson requested review from FibHeap and peter-lockhart-pub August 3, 2026 21:34

mjansson force-pushed the proto/s3-header-metadata branch 12 times, most recently from f83be1e to 86bfd2d Compare August 4, 2026 08:05

mjansson force-pushed the proto/s3-header-metadata branch from 86bfd2d to 2f734cd Compare August 4, 2026 09:25

FibHeap approved these changes Aug 4, 2026

View reviewed changes

Merge branch 'EpicGames:main' into proto/s3-header-metadata

fe1f58e

mjansson added the ready-to-import Approved by Epic staff for import into Lore label Aug 4, 2026

epic-lore-bot Bot added imported Imported into Lore for internal review and removed ready-to-import Approved by Epic staff for import into Lore labels Aug 4, 2026

peter-lockhart-pub approved these changes Aug 5, 2026

View reviewed changes

mjansson mentioned this pull request Aug 6, 2026

lore-aws: Avoid rewriting S3 on fragment deduplication and fix data corruption from torn metadata/blob representation #155

Closed

mjansson closed this Aug 6, 2026

Uh oh!

Conversation

mjansson commented Aug 3, 2026

Problem

Change

Flows

Costs

Rollout

Testing

Known limitations

Uh oh!

epic-lore-bot Bot commented Aug 4, 2026

Uh oh!

peter-lockhart-pub left a comment

Choose a reason for hiding this comment

Uh oh!

peter-lockhart-pub Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjansson commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

peter-lockhart-pub Aug 5, 2026 •

edited

Loading