Skip to content

#4667 Phase 1 — write-path *Projected variants bypass session-shared trackers#4669

Merged
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase1-write-path-projected
Jun 5, 2026
Merged

#4667 Phase 1 — write-path *Projected variants bypass session-shared trackers#4669
jeremydmiller merged 1 commit into
masterfrom
feature/4667-phase1-write-path-projected

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Phase 1 of #4667 — eliminate
session-shared dictionary access from the projection write path.

Background

Async-daemon parallel slice workers share a single DocumentSessionBase (10-wide
Block<EventSliceExecution> per (range, tenant) in AggregationRunner). The
projection write path was still reaching into _session.Versions / _session.Revisions
/ _session.ItemMap from inside that parallel block. IMartenSession is documented
as not thread-safe; #4658 closed StoreProjection; this phase closes the remaining
write sites that ProjectionStorage's Store(snapshot) and
Store(snapshot, id, tenantId) routed through.

What changes

New IDocumentStorage<T> surface (mirrors the existing OverwriteProjected pattern):

IStorageOperation UpsertProjected(T document, string tenantId);
IStorageOperation InsertProjected(T document, string tenantId);
IStorageOperation UpdateProjected(T document, string tenantId);

Closed-shape storage leaves — the 9 writeable variants (Unversioned / Optimistic /
Numeric × Lightweight / IdentityMap / DirtyChecked) build their corresponding
closed-shape op with null trackers. QueryOnlyClosedShapeStorage throws
NotSupportedException; SubClassDocumentStorage / ValueTypeIdentifiedDocumentStorage
delegate; EventDocumentStorage / EventMapping throw — all consistent with the
established OverwriteProjected shape.

Null-tolerance audit on all four ConcurrencyMode op classes that hold a
tracker dictionary (Optimistic Insert/Update/Upsert + Numeric Insert/Update/Upsert):
_versions / _revisions are now nullable and the dict writes in PostprocessAsync
are guarded. Optimistic Update/Upsert's ConfigureCommand treats a null tracker as
"expected version unknown" and binds DBNull for the WHERE-guard slot — callers that
go through *Projected should also set IgnoreConcurrencyViolation = true if they
want silent no-op semantics on the conflict branch (the projection runtime already
does this via IRevisionedOperation in StoreProjection).

ProjectionStorage rewrites:

Site Before After
Store(snapshot) line 86 _storage.Upsert(snapshot, _session, TenantId) _storage.UpsertProjected(snapshot, TenantId)
Store(snapshot, id, tenantId) line 117–135 eject + _storage.Store(_session, ...) + _storage.Upsert(snapshot, _session, tenantId) gated eject + _storage.UpsertProjected(snapshot, tenantId)

The GH-3850 identity-map maintenance (EjectAggregateFromIdentityMap +
_storage.Store(_session, snapshot)) is preserved but gated on
Options.EventGraph.UseIdentityMapForAggregates. The default (false) path now takes
the session-state-free write; the opt-in (true) path still gets correct
inline-projection-rewriting-an-immutable-aggregate semantics. Per the #4667 Phase 3
design note, opt-in is documented as not safe for parallel projection workers — that
race is accepted with the flag on.

Acceptance criteria

  • Every ProjectionStorage write method now routes through a *Projected variant. ✅
  • grep -nE '_storage\.(Upsert|Insert|Update)\b' src/Marten/Internal/Sessions/DocumentSessionBase.ProjectionStorage.cs returns zero hits. ✅
  • All four closed-shape op Postprocess methods tolerate null trackers. ✅
  • Daemon tests in src/DaemonTests/ pass. ✅

Verification

Local on net9.0:

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

The GH-3850 regression test (fetch_latest_immutable_aggregate_running_inline_and_identity_map)
specifically covers the identity-map-maintenance path and is green.

Follow-ups

  • Phase 2 (#4667) — read-path
    LoadProjectedAsync / LoadManyProjectedAsync + a fifth projection-safe selector
    flavor per closed-shape storage.
  • Phase 3 — ProjectionDocumentSession overrides for user-code reads from inside
    EvolveAsync, gated by UseIdentityMapForAggregates.

🤖 Generated with Claude Code

…trackers

Async-daemon parallel slice workers share a single DocumentSessionBase; the
projection write path was still reaching into _session.Versions /
_session.Revisions / _session.ItemMap from inside the parallel block.
#4658 closed StoreProjection; this phase closes the remaining write sites
that ProjectionStorage's Store(snapshot) and Store(snapshot, id, tenantId)
routed through.

Surface change:
  IDocumentStorage<T>.UpsertProjected(T, string tenantId)
  IDocumentStorage<T>.InsertProjected(T, string tenantId)
  IDocumentStorage<T>.UpdateProjected(T, string tenantId)

These mirror the existing OverwriteProjected pattern. The 9 writeable
closed-shape storage leaves build their corresponding closed-shape op with
null trackers; QueryOnly throws NotSupported; SubClass / ValueType / Event
storages delegate or throw consistently with their existing OverwriteProjected
implementations.

Null-tolerance audit applied to all four ConcurrencyMode op classes that hold
a tracker dictionary (Optimistic Insert/Update/Upsert + Numeric Insert/Update/
Upsert): _versions / _revisions are now nullable and the dict writes in
PostprocessAsync are guarded. Optimistic Update/Upsert's ConfigureCommand
treats a null tracker as "expected version unknown" and binds DBNull for the
WHERE-guard slot.

ProjectionStorage rewrites:

* Store(snapshot) -> _storage.UpsertProjected(snapshot, TenantId)
* Store(snapshot, id, tenantId) -> _storage.UpsertProjected(snapshot, tenantId)

The GH-3850 identity-map maintenance (EjectAggregateFromIdentityMap +
_storage.Store(_session, snapshot)) is preserved but gated on
Options.EventGraph.UseIdentityMapForAggregates so the default (false) path
takes the session-state-free write and the opt-in (true) path still gets
correct inline-projection-rewriting-an-immutable-aggregate semantics. Per the
#4667 Phase 3 design note, opt-in is documented as not safe for parallel
projection workers — that race is accepted with the flag on.

Verified locally:
  * DaemonTests net9.0  — 187 / 187 green
  * EventSourcingTests net9.0 — 1361 / 1368 green (7 pre-existing skips,
    including the GH-3850 regression which still passes)
  * CoreTests net9.0 — 421 / 422 green (1 pre-existing skip)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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