Skip to content

Bulk event insert applies the schema once per database, not per call (#4946)#4949

Merged
jeremydmiller merged 2 commits into
masterfrom
gh-4946-bulk-insert-schema-apply
Jul 14, 2026
Merged

Bulk event insert applies the schema once per database, not per call (#4946)#4949
jeremydmiller merged 2 commits into
masterfrom
gh-4946-bulk-insert-schema-apply

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Fixes #4946

Mechanism

Both batch BulkInsertEventsAsync overloads on DocumentStore began with:

await Storage.ApplyAllConfiguredChangesToDatabaseAsync().ConfigureAwait(false);

on every call. That is not "check the schema cheaply" — IMartenStorage.ApplyAllConfiguredChangesToDatabaseAsync() calls Tenancy.BuildDatabases() and runs a full schema-delta computation (partition introspection + information_schema sweeps) against every database in the store, in parallel. A conversion tool importing in ~1,000-event batches therefore paid a full store-wide introspection per batch, even under AutoCreate.None where the apply cannot do anything by design.

Measured in production on a 512-database sharded store: import throughput collapsed to ~17 events/s (a 686k-event tenant projecting to ~11 hours), with the connection pool filled by ~370 connections whose last statement was the Weasel partition-introspection query. The streaming overload BulkInsertEventStreamAsync has no such call and does >3,000 events/s — the tell that the apply was a leftover rather than part of the contract.

The fix

ensureBulkInsertSchemaAsync(IMartenDatabase, CancellationToken) in DocumentStore.cs replaces the unconditional call:

  1. Floor — skip when AutoCreate is None. The apply is a contractual no-op there, so calling it is pure waste. Nothing is executed, no connection is taken.
  2. Once per database, not per call. The apply is memoized against the identifier of the database being imported into. A caller looping batches over one store pays introspection once, not once per batch.
  3. Only the database being written to. The old call fanned out across every database in the store; a bulk insert only ever writes to the target tenant's database. A 512-database store now applies at most once per database it actually imports into, instead of 512 applies per batch.

Thread safety

Batches may be imported concurrently. The memo is a ConcurrentDictionary<string, Lazy<Task>> keyed by IMartenDatabase.Identifier, with LazyThreadSafetyMode.ExecutionAndPublication, so racing callers for the same database all await one in-flight apply — GetOrAdd's factory can race, but Lazy guarantees the apply body runs once. Keying on the database identifier rather than on the DocumentStore is what makes the guard correct for a sharded/multi-tenant store: per-database, once each; never per-call.

A failed apply evicts its entry, so a later call retries rather than caching the failure (or a cancellation) forever.

Document bulk insert path

Checked: no, BulkInsertAsync / BulkInsertDocumentsAsync do not have this bug. They go through BulkInsertion, which calls _tenant.Database.EnsureStorageExistsAsync(typeof(T), ...) — the ordinary per-feature, per-database ensure that Weasel memoizes, the same one every session write uses. It is not a full-store schema delta. Left alone.

Tests

src/EventSourcingTests/Bugs/Bug_4946_bulk_insert_does_not_apply_schema_per_call.cs — these assert how many times the apply runs, which is the actual regression, not merely that the insert still works:

  • 5 sequential batches on one store → exactly 1 apply
  • 5 sequential batches for a conjoined tenant → exactly 1 apply
  • 10 concurrent batches → exactly 1 apply
  • AutoCreate.None over a pre-applied schema, 3 batches → 0 applies, events land correctly

Green: the new 4, the existing BulkEventAppendTests / BulkEventStreamAppendTests (29), and TenantPartitionedEventsTests Regressions + Migration (26), which cover bulk import on a partitioned store.

Not verifiable locally: I cannot stand up a 512-database sharded store, so the production throughput numbers are the reporter's, not remeasured here. What is verified locally is the invocation count, which is the mechanism behind them.

Docs: added a "Schema Application" note to docs/events/bulk-appending.md.

Reported by @erdtsieck with production measurements.

🤖 Generated with Claude Code

jeremydmiller and others added 2 commits July 13, 2026 19:28
…H-4946)

The batch BulkInsertEventsAsync overloads began with
Storage.ApplyAllConfiguredChangesToDatabaseAsync() on every call: a full schema
delta -- partition introspection plus information_schema sweeps -- across every
database in the store, per batch. A conversion tool importing in 1,000-event
batches paid that per batch; measured in a 512-database sharded store it
collapsed throughput to ~17 events/s and parked the connection pool on Weasel's
partition introspection query. The streaming overload has no such call and is
fine, which is the tell that the apply is a leftover, not part of the contract.

The apply is now:

- skipped entirely when the effective AutoCreate is None, where it is a no-op by
  contract and only the introspection cost remains, and
- otherwise run at most once per *database*, and only for the database being
  imported into rather than every database in the store.

Concurrent batches are safe: the per-database entry is a Lazy<Task> with
ExecutionAndPublication, so racing callers await a single in-flight apply. A
failed apply is evicted so a later call retries instead of caching the failure.

Reported by @erdtsieck with production measurements.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…H-4946)

The per-database apply is memoized, so the first caller to arrive owns the
single in-flight task that every concurrent caller for that database awaits.
Binding that shared task to the first caller's CancellationToken meant one
batch's cancellation would fail sibling batches that were never cancelled.

The apply now runs untethered and each caller awaits it under its own token.
A schema apply is short and idempotent, so letting it run to completion is
the cheaper trade.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit 0324509 into master Jul 14, 2026
9 checks passed
@jeremydmiller
jeremydmiller deleted the gh-4946-bulk-insert-schema-apply branch July 14, 2026 00:49
This was referenced Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant