Skip to content

Scope MarkEventsAsSkipped to a tenant under UseTenantPartitionedEvents (#5234 follow-up) - #5698

Merged
jeremydmiller merged 1 commit into
JasperFx:masterfrom
lahma:fix/mark-events-skipped-tenant
Oct 19, 2026
Merged

jeremydmiller merged 1 commit into
JasperFx:masterfrom
lahma:fix/mark-events-skipped-tenant

Conversation

@lahma

@lahma lahma commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Follows up #5234 and #5236. #5236 scoped masking and compaction to the calling tenant and left the fourth site #5234 flagged open on purpose:

MartenDatabase.MarkEventsAsSkipped has the same where seq_id = ANY(...) shape and is the fourth site the reporter flagged. I did not fix it here: it is public IMartenDatabase API taking only long[] sequences with no tenant, so scoping it means adding API surface (an overload, or a loud guard that leaves partitioned users unable to skip at all) rather than appending a predicate. That is an API decision rather than a bug fix, so it should be yours.

This PR proposes an answer to that API question. It is one possible shape and you may prefer another. The core assertion, that tenant beta's event stays unskipped, holds whatever shape you choose. Only the assertions about which overload throws would change.

The defect

Under Events.UseTenantPartitionedEvents, each tenant draws seq_id from its own mt_events_sequence_{suffix}, so seq_id = 1 exists in every tenant's partition. With EnableEventSkippingInProjectionsOrSubscriptions on, IMartenDatabase.MarkEventsAsSkipped(long[]) runs

update {schema}.mt_events set is_skipped = TRUE where seq_id = ANY(:sequences)

A host that reads tenant alpha's sequences through alpha's session and passes them to the documented call (store.Storage.Database.MarkEventsAsSkipped(sequences), docs/events/skipping.md) also skips the same-numbered event of every other tenant. Those events then silently drop out of:

  • FetchStreamAsync;
  • AggregateStreamAsync;
  • FetchLatest / FetchForWriting;
  • projection rebuilds;
  • subscription replays.

Nothing throws. Unlike #5234, nothing is deleted or overwritten: resetting is_skipped restores the events.

The async daemon is not affected. Its own poison-event skipping (SkipApplyErrors) does not call MarkEventsAsSkipped and never writes is_skipped:

  • ProjectionExecution.buildBatchWithSkipping / GroupedProjectionExecution call EventRange.SkipEventSequence, which removes the event from the in-memory batch, bumps a metric and records a dead letter.
  • Under partitioning, every range holds one tenant's events:
    • the native daemon fans out per tenant;
    • EventLoader adds d.tenant_id = '{tenant}' for a per-tenant shard;
    • DistributesAgentsPerTenant makes Wolverine-managed hosts fan out per tenant too.

The only exposure is a host, or a tool, calling the public API.

The proposed shape

  1. A tenant-aware overload, IMartenDatabase.MarkEventsAsSkipped(string tenantId, long[] sequences, CancellationToken token = default).
    • It appends and tenant_id = :tenant_id under TenancyStyle.Conjoined, the same gate ConjoinedEventFilter uses since Scope event masking and stream compaction to the calling tenant #5236.
    • Under any other tenancy style the database is already the scope (database per tenant), so the tenant is not consulted.
    • The tenant id goes through TenantIdStyle.MaybeCorrectTenantId, as the session factories do.
  2. The tenant-less overload refuses under UseTenantPartitionedEvents with an InvalidOperationException that names the new overload. Everywhere else it is unchanged: with one global sequence a sequence names exactly one event, conjoined or not.
  3. A default interface member. The new member is a default interface implementation that throws NotSupportedException, so it is additive for anyone implementing IMartenDatabase.
  4. Binary and source compatibility.
    • The existing signature is untouched, and no optional parameter was added to an existing method.
    • The new overload's first parameter is a string, so no existing call shape ((long[]), (long[], CancellationToken), (null), (null, token)) rebinds to it.
  5. Docs. docs/events/skipping.md gets a short multi-tenancy section. The mark_events_as_skipped_by_tenant sample becomes a snippet that finds the tenant's database and names the tenant, which is right for every topology: one database, a database per tenant, or sharded.

Alternatives you might prefer:

  • gate on UseTenantPartitionedEvents instead of Conjoined;
  • keep the old overload working under partitioning, with only a doc warning;
  • put the operation on the session (session.Events), so the session's tenant scopes it;
  • or have the default interface member fall back to the tenant-less overload rather than throw.

Tests

src/TenantPartitionedEventsTests/Regressions/Bug_5234_mark_events_as_skipped_per_tenant.cs, on PartitionedStoreContext, mirrors Bug_5234_cross_tenant_event_rewrites:

  • Alpha and beta each start a one-event stream, and the test asserts that both events have the same seq_id, so it cannot pass vacuously.
  • the_tenantless_overload_must_not_skip_another_tenants_event calls the documented MarkEventsAsSkipped(long[]) with alpha's sequence. It asserts that beta's event is not skipped, reading is_skipped directly and through FetchStreamAsync. Then it asserts that the call threw the InvalidOperationException naming the new overload. It fails on master.
  • skipping_by_tenant_skips_only_that_tenants_event checks that the new overload skips alpha's event and leaves beta's alone. It needs the new API, so it cannot compile on master. Instead it was checked by mutation: dropping the tenant predicate from the fix turns it red with the same assertion.

src/EventSourcingTests/marking_events_as_skipped_by_tenant.cs covers the stores outside partitioning:

  • conjoined tenancy filters by the named tenant even when another tenant's sequence is passed;
  • conjoined tenancy without partitioning keeps the tenant-less overload working;
  • single tenancy does not consult the tenant.

Red, on unmodified master

failed TenantPartitionedEventsTests.Regressions.Bug_5234_mark_events_as_skipped_per_tenant.the_tenantless_overload_must_not_skip_another_tenants_event (880ms)
  Shouldly.ShouldAssertException : await isSkipped("beta", sequence)
      should be
  False
      but was
  True

  Additional Info:
      skipping tenant alpha's event must not skip tenant beta's same-numbered event

Mutation (the fix without its tenant predicate)

failed TenantPartitionedEventsTests.Regressions.Bug_5234_mark_events_as_skipped_per_tenant.skipping_by_tenant_skips_only_that_tenants_event (652ms)
  Shouldly.ShouldAssertException : await isSkipped("beta", sequence)
      should be
  False
      but was
  True
  Additional Info:
      skipping tenant alpha's event must not skip tenant beta's same-numbered event

Green

The suites were run on both target frameworks, against postgres:17-alpine.

Run net10.0 net9.0
Bug_5234_mark_events_as_skipped_per_tenant 2/2 2/2
TenantPartitionedEventsTests, the whole project (includes Bug_5234_cross_tenant_event_rewrites) 255/257, 2 pre-existing skips 254/257, 2 pre-existing skips, 1 known flake (below)
EventSourcingTests: marking_events_as_skipped_by_tenant, marking_events_as_skipped_as_Guid_identified, marking_events_as_skipped_as_string_identified, Bug_4416_closed_shape_metadata_binders 15/15 15/15

The net9.0 failure was dynamic_tenant_lifecycle_on_shard_during_daemon.tenant_added_to_shard_mid_run_is_provisioned_and_catches_up_without_daemon_restart, with "should be 3 but was 1". That is the known flake its [Trait("Retry", "3")] comment records: the daemon has not yet caught up on the shard. It does not touch skipping. A fresh-process rerun of that class, together with both Bug_5234_* classes on net9.0, passed 7/7.

docs/events/skipping.md:

  • dotnet mdsnippets regenerates the new snippet byte for byte;
  • markdownlint-cli --disable MD009 and cspell --config ./docs/cSpell.json are clean.

🤖 Generated with Claude Code

JasperFx#5234 follow-up)

Under UseTenantPartitionedEvents every tenant draws seq_id from its own
mt_events_sequence_{suffix}, so seq_id is not unique across tenants.
IMartenDatabase.MarkEventsAsSkipped(long[]) updated mt_events by seq_id
alone, so skipping one tenant's event #N also skipped event #N of every
other tenant, which then silently dropped out of FetchStreamAsync, live
aggregation, rebuilds and subscription replays. JasperFx#5234 flagged it as the
fourth site of that shape and JasperFx#5236 left it open as an API decision.

- Add MarkEventsAsSkipped(string tenantId, long[] sequences, ...), which
  appends "and tenant_id = :tenant_id" under conjoined event tenancy (the
  ConjoinedEventFilter gate). It is a default interface member that
  throws, so it is additive for other IMartenDatabase implementers, as
  ConfiguredCommandTimeout (JasperFx#5269) and SequenceFeature (JasperFx#5665) are.
- The tenant-less overload throws InvalidOperationException under
  UseTenantPartitionedEvents, naming the new overload. Everywhere else it
  is unchanged.
- docs/events/skipping.md gains a multi-tenancy section, and the
  by-tenant sample names the tenant.

The async daemon's own skipping (SkipApplyErrors) is in memory and per
tenant and never calls this method, so it is not affected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit 8065f44 into JasperFx:master Oct 19, 2026
32 checks passed
This was referenced Oct 20, 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

Development

Successfully merging this pull request may close these issues.

2 participants