Skip to content

#4667 Phase 3 — ProjectionDocumentSession routes user-code LoadAsync through projection-safe path#4671

Merged
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase3-projection-document-session-overrides
Jun 5, 2026
Merged

#4667 Phase 3 — ProjectionDocumentSession routes user-code LoadAsync through projection-safe path#4671
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase3-projection-document-session-overrides

Conversation

@jeremydmiller
Copy link
Copy Markdown
Member

Phase 3 of #4667 — closes
the user-code escape hatch. Builds on #4669
(write path) and #4670
(read path on ProjectionStorage).

Background

When a user-supplied aggregation projection's Apply (or DetermineAction /
EvolveAsync) calls operations.LoadAsync<X>(...) from inside the daemon,
that call routed through IDocumentStorage<,>.LoadAsync(id, this, ct)
BuildSelector(session) → per-row writes into _session.Versions /
_session.ItemMap / _session.ChangeTrackers. The async daemon shares one
IMartenSession across the 10-wide Block<EventSliceExecution> parallel
workers per (range, tenant); each row was a race on those shared dicts.

What changes

QuerySession (the load API base): introduces two protected internal virtual
chokepoints:

ExecuteLoadOneAsync<T, TId>(storage, id, token)
ExecuteLoadManyAsync<T, TId>(storage, ids, token)

All public LoadAsync<T> / LoadManyAsync<T> overloads (string / object /
int / long / Guid × single / params / IEnumerable × with / without
CancellationToken) now call through the chokepoints instead of
storage.LoadAsync(id, this, token) directly. Default impl preserves the
existing session-aware route — zero behavior change for non-projection
sessions.

ProjectionDocumentSession overrides both chokepoints with
UseIdentityMapForAggregates gating that mirrors Phase 1 + Phase 2:

UseIdentityMapForAggregates Route
false (default in daemon) LoadProjectedAsync / LoadManyProjectedAsync (fresh connection, no session-shared dict writes)
true (opt-in) base session-aware route — preserves GH-3850 inline-projection-immutable-aggregate semantics; documented as not safe for parallel projection workers

Implementation note — the [return: MaybeNull] + bare Task<T> chokepoint
return shape: the override on ProjectionDocumentSession (a partial-class
chain not uniformly in #nullable enable context) loses the
reference-type-vs-Nullable<T> disambiguation of T? at the override site
(CS0508 + CS0453). The attribute form is unambiguous in either context and
produces the same call-site annotation. All public LoadAsync<T>
return-type Task<T?> is unchanged.

Regression test

Bug_4667_projection_load_async_parallel_no_race.cs runs 250 streams × 4
events through the async daemon's parallel Block<EventSliceExecution>
fanout against a SingleStreamProjection whose Apply does
session.LoadAsync<Bug4667Customer> per event. Pre-Phase 3 this path
per-row-wrote into the shared session's tracker dicts; post-Phase 3 it goes
through LoadProjectedAsync (fresh connection, no session state).

Acceptance criteria

Verification

Local on net9.0:

Suite Result
DaemonTests 188 / 188 ✅ (+1 new regression test)
EventSourcingTests 1361 passed / 7 pre-existing skips ✅
CoreTests 421 passed / 1 pre-existing skip ✅

Out of scope (Phase 4 / follow-up)

  • Query<T>() / LINQ — the issue lists Query<T> as a Phase 3 target, but
    the LINQ selector swap is more invasive than the LoadAsync chokepoint
    and the typical user-code escape hatch hits LoadAsync, not Query<T>.
    Defer to a focused follow-up once the load-harness in
    #4666 exposes a
    surviving Query<T> race.
  • Phase 4 (per-slice scoped session) — only needed if a race survives
    Phases 1–3 per the issue's "Skip until Phases 1–3 ship" guidance.

🤖 Generated with Claude Code

…through projection-safe path

Closes the user-code escape hatch that Phases 1 and 2 left open: when a
user-supplied aggregation projection's Apply (or DetermineAction / EvolveAsync)
calls operations.LoadAsync<X>(...) from inside the daemon, that call routed
through IDocumentStorage<,>.LoadAsync(id, this, ct) → BuildSelector(session) →
per-row writes into _session.Versions / _session.ItemMap / _session.ChangeTrackers.
The async daemon shares one IMartenSession across the 10-wide Block<
EventSliceExecution> parallel workers per (range, tenant); each row was a race
on those shared dictionaries.

Surface change:

* QuerySession (the load API base) — introduces two protected internal
  virtual chokepoints:
    ExecuteLoadOneAsync<T, TId>(storage, id, token)
    ExecuteLoadManyAsync<T, TId>(storage, ids, token)
  All public LoadAsync<T> / LoadManyAsync<T> overloads (string / object /
  int / long / Guid × single / params / IEnumerable × with / without
  CancellationToken) now call through the chokepoints instead of
  storage.LoadAsync(id, this, token) directly. Default impl preserves the
  existing session-aware route — zero behavior change for non-projection
  sessions.
* ProjectionDocumentSession overrides both chokepoints with
  UseIdentityMapForAggregates gating that mirrors Phase 1 + Phase 2:
    - default (false) routes through LoadProjectedAsync /
      LoadManyProjectedAsync (fresh connection, no session-shared dict
      writes)
    - opt-in (true) falls through to base session-aware route to keep
      GH-3850 inline-projection-immutable-aggregate semantics; documented
      as not safe for parallel projection workers.

Implementation note on the [return: MaybeNull] + bare Task<T> chokepoint
return shape: the override on ProjectionDocumentSession (a partial-class
chain not uniformly in #nullable enable context) loses the
reference-type-vs-Nullable<T> disambiguation of T? at the override site
(CS0508 + CS0453). The attribute form is unambiguous in either context
and produces the same callsite annotation. All public LoadAsync<T>
return-type Task<T?> is unchanged.

Regression test:
Bug_4667_projection_load_async_parallel_no_race.cs runs 250 streams × 4
events through the async daemon's parallel Block<EventSliceExecution>
fanout against a SingleStreamProjection whose Apply does
session.LoadAsync<Bug4667Customer> per event. Pre-Phase 3 this path
per-row-wrote into the shared session's tracker dicts; post-Phase 3 it
goes through LoadProjectedAsync (fresh connection, no session state).

Verified locally on net9.0:
* DaemonTests — 188 / 188 ✅ (+1 new regression test)
* EventSourcingTests — 1361 passed / 7 pre-existing skips ✅
* CoreTests — 421 passed / 1 pre-existing skip ✅ (one flaky run observed;
  re-run clean)

Out of scope (Phase 4 / follow-up):
* Query<T>() / LINQ — the issue lists Query<T> as a Phase 3 target, but
  the LINQ selector swap is more invasive than the LoadAsync chokepoint
  and the typical user-code escape hatch hits LoadAsync, not Query<T>.
  Defer to a focused follow-up once the load-harness in #4666 exposes a
  surviving Query<T> race.
* Phase 4 per-slice scoped session — only needed if a race survives
  Phases 1–3 per the issue's "Skip until Phases 1–3 ship" guidance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller jeremydmiller merged commit 56cb38d into master Jun 5, 2026
8 checks passed
@jeremydmiller jeremydmiller deleted the feature/4667-phase3-projection-document-session-overrides branch June 5, 2026 16:53
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