Skip to content

#4667 Phase 2 — read-path LoadProjectedAsync bypasses session-shared trackers#4670

Merged
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase2-read-path-projection-safe-selectors
Jun 5, 2026
Merged

#4667 Phase 2 — read-path LoadProjectedAsync bypasses session-shared trackers#4670
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase2-read-path-projection-safe-selectors

Conversation

@jeremydmiller
Copy link
Copy Markdown
Member

Phase 2 of #4667 — eliminate
session-shared dictionary access from the projection read path. Builds on
#4669 which closed the write
sites.

Background

ProjectionStorage's LoadAsync / LoadManyAsync routed through the
session-aware closed-shape selectors (Lightweight / IdentityMap /
DirtyChecked), which per-row write into _session.Versions /
_session.ItemMap / _session.ChangeTrackers — the same race shape #4658

  • Phase 1 closed for the write path. Under the async daemon's 10-wide
    Block<EventSliceExecution> parallel workers sharing one IMartenSession,
    each row materialization was a race on those dictionaries.

What changes

New IDocumentStorage<T, TId> surface:

Task<T?> LoadProjectedAsync(TId id, IMartenDatabase database, string tenantId, CancellationToken token);
Task<IReadOnlyList<T>> LoadManyProjectedAsync(TId[] ids, IMartenDatabase database, string tenantId, CancellationToken token);

These take IMartenDatabase + tenantId instead of a session — the issue's
stated goal that storage read methods never accept an IMartenSession
argument.

Implementation: a new ClosedShapeProjectionLoader<TDoc, TId> static
helper opens a fresh connection from the database, executes the existing
BuildLoad{,Many}Command SQL, and deserializes the data column directly
via ISerializer.FromJsonAsync. Hierarchical storages dispatch through
DocumentMapping.TypeFor like HierarchicalClosedShapeQueryOnlySelector.
No metadata binders are applied — projections care about aggregate state,
not per-row CreatedAt / Headers / etc.; if a future projection scenario
needs metadata, it can be added here as a focused follow-up.

Column layout matches the writeable closed-shape selectors (id at col 0,
data at col 1, metadata at 2+). This was the v1 bug — I'd started with
the QueryOnly layout (data at col 0) and the first DaemonTests run failed
19 tests with JsonReaderException because the loader was deserializing
the id-column bytes as JSON.

Storage wiring:

Storage Behavior
LightweightClosedShapeStorage / IdentityMapClosedShapeStorage / DirtyCheckedClosedShapeStorage (3 writeable closed-shape mid-tiers) Override with the helper
QueryOnlyClosedShapeStorage Throws NotSupported — not used by the projection read path
SubClassDocumentStorage Delegates to parent + downcast
ValueTypeIdentifiedDocumentStorage Delegates to inner with the unwrapped id
DocumentStorage<T, TId> base default Throws NotImplementedException for non-closed-shape paths (legacy Roslyn-generated storages); those aren't projection-eligible
IDocumentStorage<T> (single-T) No change — new methods are on the <T, TId> interface only, so EventDocumentStorage / EventMapping<T> don't need them (events aren't projected documents)

ProjectionStorage rewrites:

  • LoadAsync(id, ct) — gated on UseIdentityMapForAggregates. Default
    (false) routes through LoadProjectedAsync. The opt-in (true) case
    falls through to the session-aware LoadAsync to preserve the
    inline-projection identity-map semantics that
    silently_turns_on_identity_map_for_inline_aggregates depends on —
    the inline projection needs to mutate the same instance the
    IdentitySession holds. Mirrors the same gating Phase 1 applied to
    Store(snapshot, id, ...) for FetchLatest doesnt return the latest version of the aggregate after appending an event when working with record-types #3850.
  • LoadManyAsync(identities, ct) — same gating.

Acceptance criteria

  • ProjectionStorage.LoadAsync / LoadManyAsync perform zero writes to
    session.Versions, session.ItemMap, or session.ChangeTrackers in the
    default (race-prone) configuration. ✅ (The opt-in UseIdentityMapForAggregates = true
    path intentionally retains the session-aware route.)
  • Existing identity-map-using projection tests still pass when
    UseIdentityMapForAggregates = true is set. ✅

Verification

Local on net9.0:

Suite Result
DaemonTests 187 / 187 ✅
EventSourcingTests 1361 passed / 7 pre-existing skips ✅
CoreTests 421 passed / 1 pre-existing skip ✅

Specifically green: the GH-3850 regression
(fetch_latest_immutable_aggregate_running_inline_and_identity_map) and the
silently_turns_on_identity_map_for_inline_aggregates regression that
caught the v1 gating gap.

Follow-up

Phase 3 (#4667) —
ProjectionDocumentSession overrides for user-code reads from inside
EvolveAsync (closes the open surface where user-supplied
operations.LoadAsync<X>(...) from a projection still routes through the
session-aware path).

🤖 Generated with Claude Code

…trackers

Closes the remaining read sites that ProjectionStorage's LoadAsync /
LoadManyAsync still routed through the session-aware closed-shape selectors,
which per-row write into _session.Versions / _session.ItemMap /
_session.ChangeTrackers (the same race shape #4658 + Phase 1 closed for the
write path).

Surface change:
  IDocumentStorage<T, TId>.LoadProjectedAsync(TId id, IMartenDatabase database, string tenantId, CancellationToken token)
  IDocumentStorage<T, TId>.LoadManyProjectedAsync(TId[] ids, IMartenDatabase database, string tenantId, CancellationToken token)

These take IMartenDatabase + tenantId, not a session — the issue's stated goal
that storage read methods never accept an IMartenSession argument.

Implementation: a new ClosedShapeProjectionLoader<TDoc, TId> static helper
opens a fresh connection from the database, executes the existing
BuildLoad{,Many}Command SQL, and deserializes the data column directly via
ISerializer.FromJsonAsync. Hierarchical storages dispatch through
DocumentMapping.TypeFor like HierarchicalClosedShapeQueryOnlySelector. No
metadata binders are applied — projections care about aggregate state, not
per-row CreatedAt / Headers / etc.; if a future projection scenario needs
metadata it can be added here as a focused follow-up.

Column layout matches the writeable closed-shape selectors (Lightweight /
IdentityMap / DirtyChecked): id at col 0, data at col 1, metadata at 2+. This
was the v1 bug — I'd started with the QueryOnly layout (data at col 0) and
the first DaemonTests run failed 19 tests with JsonReaderException because
the loader was deserializing the id column bytes as JSON.

Storage wiring:

* LightweightClosedShapeStorage / IdentityMapClosedShapeStorage /
  DirtyCheckedClosedShapeStorage (the 3 writeable closed-shape mid-tiers)
  override with the helper.
* QueryOnlyClosedShapeStorage throws NotSupported — it isn't used by the
  projection read path; ProjectionStorage holds a writeable storage for the
  projected document type.
* SubClassDocumentStorage delegates to parent + downcast like its other
  Load delegations.
* ValueTypeIdentifiedDocumentStorage delegates to inner with the unwrapped id.
* DocumentStorage<T, TId> base default impl throws NotImplementedException
  for non-closed-shape paths (legacy Roslyn-generated storages) — those
  aren't projection-eligible by construction.
* IDocumentStorage<T> (single-T) doesn't get the new methods — they're on
  the <T, TId> interface only, so EventDocumentStorage / EventMapping<T>
  don't need to implement them (events aren't projected documents).

ProjectionStorage rewrites:

* LoadAsync(id, ct) — gated on UseIdentityMapForAggregates. Default (false)
  routes through LoadProjectedAsync. The opt-in (true) case falls through
  to the session-aware LoadAsync to preserve the inline-projection
  identity-map semantics that fetching_inline_aggregates_for_writing.
  silently_turns_on_identity_map_for_inline_aggregates depends on — the
  inline projection needs to mutate the same instance the IdentitySession
  holds. Matches the same gating Phase 1 applied to Store(snapshot, id, ...)
  for GH-3850.
* LoadManyAsync(identities, ct) — same gating.

Verified locally on net9.0:

* DaemonTests — 187 / 187 ✅
* EventSourcingTests — 1361 passed / 7 pre-existing skips ✅
  (including the GH-3850 regression and the
  silently_turns_on_identity_map_for_inline_aggregates regression that
  caught the v1 gating gap)
* CoreTests — 421 passed / 1 pre-existing skip ✅

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller jeremydmiller merged commit fcf2444 into master Jun 5, 2026
8 checks passed
@jeremydmiller jeremydmiller deleted the feature/4667-phase2-read-path-projection-safe-selectors branch June 5, 2026 16:07
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.

1 participant