Fix #4683 (fixes 1+2) — drop per-tenant sequence + progression rows on tenant removal#4695
Merged
Merged
Conversation
…n tenant removal
Under UseTenantPartitionedEvents, dropping a tenant via
AdvancedOperations.DeleteAllTenantDataAsync or RemoveMartenManagedTenantsAsync
removed the partition tables but left two side-tables orphaned:
1. The freestanding mt_events_sequence_{suffix} sequence -- created without
an OWNED BY linkage to the partition table, so the partition drop leaves
it behind. Every drop-add cycle accumulated another sequence.
2. The per-tenant mt_event_progression rows -- one per projection's catch-up
row ({Projection}:All:{tenantId}) plus the per-tenant high-water row
(HighWaterMark:{tenantId}). Silent leak across drop-add cycles.
Both were originally pinned in
src/TenantPartitionedEventsTests/Admin/delete_all_tenant_data_orphan_sequence_pin.cs
(part of #4617's deferred items) as documented bugs awaiting a fix. This PR is
that fix.
## What ships
New `Marten.Internal.PerTenantPartitionedCleanup` static helper:
* `CaptureSequenceSuffixes(options, tenantIds)` -- read the partition registry
BEFORE the partition drop runs, so we know which mt_events_sequence_{suffix}
to drop afterwards (Weasel's partition drop may clear the tenant entry).
* `RunAsync(options, database, tenantIds, capturedSuffixes, token)` -- runs the
cleanup batch:
* DROP SEQUENCE IF EXISTS mt_events_sequence_{suffix} for each captured tenant.
* DELETE FROM mt_event_progression WHERE name = ANY(...) for the union of
rows that match the dropped tenants via either:
- HighWaterShardIdentity.PerTenantPrefix exact match (Marten's per-tenant
HighWater grammar — ShardName collapses HighWaterMark identities to
the constant, so they don't round-trip through ShardName.TryParse).
- ShardName.TryParse where the parsed tenant slot matches the dropped
tenant -- handles every other per-tenant projection row across both
the v1 (`Name:ShardKey:Tenant`) and versioned (`Name:V{n}:ShardKey:Tenant`)
grammars.
* Store-global rows (`HighWaterMark`, `MyProjection:All`, etc) never match -- no
tenant slot. The cleanup is verified by direct progression-row seeding to
leave them intact.
Both `DeleteAllTenantDataAsync` (via `TenantDataCleaner`) and
`RemoveMartenManagedTenantsAsync` (direct) now route through this helper. The
SQL runs as a fresh-connection batch after the partition drop commits.
## Tests
The pre-existing pin in delete_all_tenant_data_orphan_sequence_pin.cs is flipped
from `ShouldBeTrue("...the orphan leak")` to `ShouldBeFalse("...now drops...")`.
The two original test cases (for both DeleteAll and RemoveMartenManagedTenants
paths) now verify the fix instead of pinning the bug.
A third test, `DeleteAllTenantDataAsync_removes_per_tenant_progression_rows_and_preserves_store_global`,
seeds 8 progression rows directly via SQL spanning both grammars + store-global
+ versioned variants and asserts:
* Dropped tenant's rows removed across all three grammars.
* Keep tenant's rows untouched.
* Store-global `HighWaterMark` and `SomeProjection:All` rows preserved.
Suites pass on net9.0:
* TenantPartitionedEventsTests: 178/178
* MultiTenancyTests (touches RemoveMartenManagedTenantsAsync): 146/146
* CoreTests: 432/435 -- one pre-existing flake confirmed on master
(partitioning_and_foreign_keys.* shared schema state) + the standard
unrelated skip.
## Docs
docs/events/multitenancy.md gets a new "Dropping Tenants" subsection under
"Per-Tenant Event Partitioning" that documents both routes, calls out
preservation of store-global rows, and notes the ShardName-grammar-based row
identification (vs LIKE matching).
## Followups
* **#4683 Fix 3** -- drop → re-add scale test in `Marten.ScaleTesting` (20M
events, full cycle). Tracked as a separate PR per the user's
"one focused PR at a time" preference.
* **Long-term cleaner**: at sequence creation time, declare each
`mt_events_sequence_{suffix}` as `OWNED BY` its partition table so the
partition drop drops the sequence automatically. Requires a schema migration
for existing deployments; deferred as a structural follow-up. The cleanup in
this PR handles both new and existing deployments today.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jeremydmiller
added a commit
that referenced
this pull request
Jun 9, 2026
…to-end Third and final slice of #4683 (fixes 1+2 landed in PR #4695). Per the issue's \"Scale test in Marten.ScaleTesting\" acceptance, this adds the dropcycle subcommand: integration smoke for the cleanup at scale, against a dedicated UseTenantPartitionedEvents store so the per-tenant sequence + per-tenant progression-row leaks the cleanup fixes actually arise (the harness's main store config, driven from Program.cs, is Conjoined-only without UseTenantPartitionedEvents, so the leaks wouldn't even surface there). ## Flow 1. Build an isolated partitioned DocumentStore in its own schema (`scaletest_dropcycle`). 2. Register `--tenants` tenants and append a couple events to each so the per-tenant sequence + partition tables exist. 3. Seed projection-shape per-tenant progression rows directly via SQL, spanning both ShardName grammar (`Name:ShardKey:Tenant`, `Name:V{n}:ShardKey:Tenant`) and HighWaterShardIdentity grammar (`HighWaterMark:Tenant`), plus store-global rows (`HighWaterMark`, `SomeProjection:All`) that must survive. 4. Snapshot the target tenant + a peer. 5. Call `DeleteAllTenantDataAsync(targetTenant)`. 6. Verify post-drop: - Target's partition / sequence / per-tenant progression rows removed - Peer's partition / sequence / per-tenant progression rows untouched - Store-global HighWaterMark + SomeProjection:All preserved 7. Re-add + append 5 events (configurable). Verify the fresh sequence starts at last_value=0 pre-append (catches the orphan-sequence regression the original pin documented) and advances after the append. 14 boolean checks total. Non-zero exit on any failure. ## Verification Smoke run against a local PG: `dotnet run --project src/Marten.ScaleTesting -- dropcycle --tenants 3 --tenant-index 1 --readd-event-count 5`. **14/14 checks passed in 1.1s.** This is the per-issue acceptance target adapted for one-off dev-box use -- not a 20M event scale run (which the user can scale up by tweaking `--tenants` and the inline seed-event count). The full 20M scale-stress run isn't required by the issue's acceptance criteria; the test just exists in the harness now and can be scaled at the user's discretion. ## Closes #4683 (parent issue, all three fixes done: PR #4695 + this PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 10, 2026
Merged
This was referenced Jun 17, 2026
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the first two fixes from #4683 (fix 3 — drop → re-add scale test in
Marten.ScaleTesting— split as a follow-up per the "one focused PR at a time" preference).Under
UseTenantPartitionedEvents, dropping a tenant via eitherDeleteAllTenantDataAsyncorRemoveMartenManagedTenantsAsyncremoved the partition tables but left two side-tables orphaned:mt_events_sequence_{suffix}sequenceOWNED BY, so the partition drop skips it. Every drop-add cycle accumulated another sequence.mt_event_progressionrows{Projection}:All:{tenantId}catch-up plus theHighWaterMark:{tenantId}row. Silent leak.Both were originally pinned in
delete_all_tenant_data_orphan_sequence_pin.cs(#4617 section 3e deferred items) as documented bugs awaiting a fix.What ships
Marten.Internal.PerTenantPartitionedCleanupCaptureSequenceSuffixes(...)runs BEFORE the partition drop (Weasel may clear the tenant entry);RunAsync(...)runs the batched DROP SEQUENCE + DELETE FROM mt_event_progression afterwards.TenantDataCleaner.ExecuteAsyncAdvancedOperations.RemoveMartenManagedTenantsAsyncTenantDataCleanerentirely before, so it had to be wired separately).Per-tenant
mt_event_progressionrows are identified by parsing the ShardName grammar rather than LIKE-matching the row name -- so a projection whose name happens to end with a tenant id is never mistakenly deleted. Two grammars handled:HighWaterShardIdentity.PerTenantPrefixexact match (Marten's per-tenant HighWater grammar --ShardNamecollapsesHighWaterMarkidentities to the constant, so they don't round-trip throughShardName.TryParse).ShardName.TryParsefor everything else, acrossName:ShardKey:Tenantand the versionedName:V{n}:ShardKey:Tenantform.Store-global rows (
HighWaterMark,MyProjection:All, etc) never match -- no tenant slot. Verified explicitly via direct progression-row seeding in the test.Tests
The pre-existing pin in
delete_all_tenant_data_orphan_sequence_pin.csis flipped fromShouldBeTrue(\"...the orphan leak\")toShouldBeFalse(\"...now drops...\"). The two original cases (DeleteAll path + RemoveMartenManagedTenants path) now verify the fix instead of pinning the bug.A third case,
DeleteAllTenantDataAsync_removes_per_tenant_progression_rows_and_preserves_store_global, seeds 8 progression rows directly via SQL spanning all three grammars + store-global + versioned variants and asserts::All:+ versioned:V2:All:).HighWaterMarkandSomeProjection:Allrows preserved.partitioning_and_foreign_keys.*shared-schema state); the test passes alone on both branchesDocs
docs/events/multitenancy.mdgets a new "Dropping Tenants" subsection under "Per-Tenant Event Partitioning" that documents both routes, preservation of store-global rows, and theShardName-grammar-based row identification.Followups
Marten.ScaleTesting(20M events, full cycle). Separate PR.mt_events_sequence_{suffix}asOWNED BYits partition table at creation time so the partition drop drops the sequence automatically. Needs a schema migration for existing deployments; deferred as a structural follow-up. This PR handles both new and existing deployments today.🤖 Generated with Claude Code