From f00cc2369c0a63127146436ca84a1c83ddbfd067 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Tue, 28 Jul 2026 11:22:18 -0500 Subject: [PATCH] fix(efcore): make conjoined tenant partition bucketing actually work (GH-3683) Tenant "bucketing" -- registering several small tenants against one partition suffix so they share a physical partition -- is documented on the EF Core multi-tenancy page and exposed via ConjoinedTenancyOptions.PartitionPerTenant(p => p.AllowPartitionSharing = true), but it did not work on either engine. It had no test coverage, which is why this went unnoticed. Both defects were reproduced against real PostgreSQL and SQL Server. The root cause was in Weasel and is fixed by JasperFx/weasel#392 (9.20.0): - PostgreSQL emitted one single-value ListPartition per tenant, so the second member of a bucket was swallowed by CREATE TABLE IF NOT EXISTS and its first write failed with 23514. Widening an existing partition needs DETACH + re-ATTACH, which IF NOT EXISTS cannot express. - SQL Server's ordinal registry had no column for the bucket, so a tenant joining an existing bucket in a later call could not discover its ordinal. Wolverine changes here: PostgresqlTenantPartitioning needs no change at all -- it hands Weasel the tenant -> suffix map and the fixed Weasel does the right thing. SqlServerTenantPartitioning was resolving the bucket's ordinal itself out of the tenant -> ordinal map, which is blind to buckets: for a brand new tenant the lookup matched nothing and it allocated a fresh ordinal, so sequentially registered members silently never shared. It now delegates to Weasel's bucket-aware overload, and its AllowPartitionSharing guard spans calls via the persisted bucket map, matching what PostgreSQL already did. Compliance coverage on BOTH engines for the acceptance criteria on GH-3683: members registered together and separately land in one physical partition, and dropping one member leaves the others reading, writing, and holding their data. That last one covers GH-3686, the co-tenant data loss found alongside this. Docs: the bucketing sample showed the sequential pattern without enabling AllowPartitionSharing, so as written it would have thrown. The config sample now enables it, and a new Tenant Bucketing section covers when to reach for it, the drop semantics, and the 9.20.0 requirement. Verified: EfCoreTests.MultiTenancy 193/193, SqlServerTests 388/390 (2 skipped), full wolverine.slnx Release build clean at 0 warnings. NOTE: requires Weasel 9.20.0 on nuget.org; verified locally against a packed build. --- Directory.Packages.props | 14 +-- docs/guide/durability/efcore/multi-tenancy.md | 56 ++++++++- .../ConjoinedPartitioningCompliance.cs | 118 +++++++++++++++++- .../MultiTenancyDocumentationSamples.cs | 13 +- .../SqlServerTenantPartitioning.cs | 81 ++++-------- 5 files changed, 212 insertions(+), 70 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 8d28feffc..76896691d 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -127,13 +127,13 @@ - - - - - - - + + + + + + + diff --git a/docs/guide/durability/efcore/multi-tenancy.md b/docs/guide/durability/efcore/multi-tenancy.md index 5f7c5f36f..7140c1c75 100644 --- a/docs/guide/durability/efcore/multi-tenancy.md +++ b/docs/guide/durability/efcore/multi-tenancy.md @@ -572,7 +572,12 @@ opts.Services.AddDbContextWithWolverineManagedConjoinedTenancy tenancy.PartitionPerTenant()); + tenancy => tenancy.PartitionPerTenant(partitioning => + { + // Opt in before registering two tenants against one suffix. + // Without this a shared suffix is rejected outright + partitioning.AllowPartitionSharing = true; + })); ``` snippet source | anchor @@ -582,7 +587,7 @@ With partitioning enabled: * On PostgreSQL, every non-saga `ITenanted` entity table becomes `PARTITION BY LIST (tenant_id)` with one partition per tenant, managed through a `wolverine_tenant_partitions` control table in the durability schema * On SQL Server (which can only range-partition over a compact value), entities gain an `int tenant_ordinal` column stamped automatically by Wolverine, and tables are `RANGE RIGHT` partitioned over the ordinal with a registry table mapping tenant ids to ordinals * The composite `(tenant, id)` primary key exists **only in the database** — your EF model keeps its own single key, so `FindAsync()`, `Attach()`, and saga loads keep exactly the same call shapes -* Multiple small tenants can share one physical partition ("bucketing") by registering them with the same partition suffix — the answer to SQL Server's partition count ceiling and to "small tenants don't deserve their own partition" +* Multiple small tenants can share one physical partition ("bucketing") by registering them with the same partition suffix — the answer to SQL Server's partition count ceiling and to "small tenants don't deserve their own partition". See [Tenant Bucketing](#tenant-bucketing) below * Partitioned conjoined contexts require `UseEntityFrameworkCoreWolverineManagedMigrations()` — EF migrations cannot express the partition DDL Manage tenants through `IConjoinedTenantPartitions`: @@ -596,8 +601,10 @@ var partitions = host.Services // Each tenant gets its own physical partition await partitions.AddTenantAsync("tenant1"); -// Or share one partition between small tenants ("bucketing") -- -// requires AllowPartitionSharing on the partitioning options +// Or share one partition between small tenants ("bucketing") by registering +// them against the same suffix -- requires AllowPartitionSharing above. +// Members can be added one at a time as tenants onboard; the bucket is +// resolved from storage, so they land in the same physical partition await partitions.AddTenantAsync("small-tenant-a", "shared_bucket"); await partitions.AddTenantAsync("small-tenant-b", "shared_bucket"); @@ -611,6 +618,47 @@ Note that with partitioning enabled, a tenant's partition must exist before rows Sagas are deliberately **not** partitioned in this release — they keep the conjoined query filtering and tenant stamping, but stay in unpartitioned tables so saga identity is untouched. +### Tenant Bucketing + +Giving every tenant its own physical partition stops scaling somewhere — SQL Server caps a table at 15,000 +partitions, and long before that a few thousand nearly-empty partitions cost more in planning time than they +save in scans. *Bucketing* is the escape hatch: register several small tenants against the same partition +suffix and they share one physical partition, while large tenants keep theirs to themselves. + +Bucketing is opt-in. Set `AllowPartitionSharing` on the partitioning options, then pass the same suffix for +every member of a bucket: + +```cs +tenancy => tenancy.PartitionPerTenant(p => p.AllowPartitionSharing = true); + +// ... + +// big tenants keep a partition each +await partitions.AddTenantAsync("enterprise-customer"); + +// small ones share -- registered together, or one at a time as they sign up +await partitions.AddTenantAsync("small-tenant-a", "shared_bucket"); +await partitions.AddTenantAsync("small-tenant-b", "shared_bucket"); +``` + +Members can be registered together or in completely separate calls; the bucket is resolved from storage, so +a tenant onboarding a release later still lands in the partition its bucket already owns. + +Dropping one member of a bucket removes only that tenant's rows — the remaining members keep the partition and +their data. The partition itself is released only when its last member is dropped. + +::: warning +`AllowPartitionSharing` is off by default, and passing a shared suffix without it fails fast rather than +quietly giving each tenant its own partition. Leave it off unless you actually want tenants sharing storage: +a shared partition means partition pruning no longer isolates those tenants from each other, and a +partition-level operation touches every member. +::: + +::: tip +Bucketing required Weasel 9.20.0 (`JasperFx/weasel#391`). On earlier versions a shared suffix did not +actually produce a shared partition on either engine — see [GH-3683](https://github.com/JasperFx/wolverine/issues/3683). +::: + ### Partition Status Reporting Partition DDL is applied one table at a time with failures isolated, so a batch registration can partially diff --git a/src/Persistence/EfCoreTests.MultiTenancy/ConjoinedTenancy/ConjoinedPartitioningCompliance.cs b/src/Persistence/EfCoreTests.MultiTenancy/ConjoinedTenancy/ConjoinedPartitioningCompliance.cs index e446007d2..a077c3e7c 100644 --- a/src/Persistence/EfCoreTests.MultiTenancy/ConjoinedTenancy/ConjoinedPartitioningCompliance.cs +++ b/src/Persistence/EfCoreTests.MultiTenancy/ConjoinedTenancy/ConjoinedPartitioningCompliance.cs @@ -114,7 +114,9 @@ public async Task InitializeAsync() opts.Services.AddDbContextWithWolverineManagedConjoinedTenancy( (builder, connectionString) => builder.UseNpgsql(connectionString.Value), AutoCreate.CreateOrUpdate, - tenancy => tenancy.PartitionPerTenant()); + // Bucketing is opt-in; enabling it here does not change the behavior of the + // non-bucketed tests, which still register tenants with no suffix + tenancy => tenancy.PartitionPerTenant(p => p.AllowPartitionSharing = true)); } else { @@ -122,7 +124,7 @@ public async Task InitializeAsync() opts.Services.AddDbContextWithWolverineManagedConjoinedTenancy( (builder, connectionString) => builder.UseSqlServer(connectionString.Value), AutoCreate.CreateOrUpdate, - tenancy => tenancy.PartitionPerTenant()); + tenancy => tenancy.PartitionPerTenant(p => p.AllowPartitionSharing = true)); } opts.UseEntityFrameworkCoreTransactions(); @@ -282,6 +284,118 @@ await theHost.ExecuteAndWaitAsync(c => (await green.Items.ToListAsync()).Single().Id.ShouldBe(id); } + [Fact] + public async Task bucketed_tenants_registered_separately_share_one_partition() + { + // GH-3683 / weasel#391: registering bucket members ONE AT A TIME is the documented shape and the + // natural tenant-onboarding shape, and it silently did not work on either engine. PostgreSQL's + // second member was swallowed by CREATE TABLE IF NOT EXISTS so its first write failed with 23514; + // SQL Server's registry had no bucket key, so each call quietly allocated a separate ordinal and + // the tenants never actually shared the partition that bucketing exists to give them. + await thePartitions.AddTenantAsync("smalla", "shared_bucket"); + await thePartitions.AddTenantAsync("smallb", "shared_bucket"); + + // Both members read and write... + var aId = Guid.NewGuid(); + var bId = Guid.NewGuid(); + await theHost.ExecuteAndWaitAsync(c => + c.InvokeForTenantAsync("smalla", new CreatePartitionedItem(aId, "a"))); + await theHost.ExecuteAndWaitAsync(c => + c.InvokeForTenantAsync("smallb", new CreatePartitionedItem(bId, "b"))); + + var a = await theBuilder.BuildAsync("smalla", CancellationToken.None); + (await a.Items.ToListAsync()).Single().Id.ShouldBe(aId); + + var b = await theBuilder.BuildAsync("smallb", CancellationToken.None); + (await b.Items.ToListAsync()).Single().Id.ShouldBe(bId); + + // ...and they genuinely share ONE physical partition, which is the entire point + (await distinctPartitionCountAsync(["smalla", "smallb"])).ShouldBe(1); + } + + [Fact] + public async Task bucketed_tenants_registered_together_share_one_partition() + { + await thePartitions.AddTenantsAsync(new Dictionary + { + ["smalla"] = "shared_bucket", + ["smallb"] = "shared_bucket" + }); + + (await distinctPartitionCountAsync(["smalla", "smallb"])).ShouldBe(1); + } + + [Fact] + public async Task dropping_one_bucket_member_leaves_the_others_working() + { + // The co-tenant data-loss defect found alongside GH-3683: on PostgreSQL the by-value drop resolved + // the tenant to its suffix and dropped BY SUFFIX, taking every co-tenant's rows with it. + await thePartitions.AddTenantAsync("smalla", "shared_bucket"); + await thePartitions.AddTenantAsync("smallb", "shared_bucket"); + + var survivorId = Guid.NewGuid(); + await theHost.ExecuteAndWaitAsync(c => + c.InvokeForTenantAsync("smalla", new CreatePartitionedItem(Guid.NewGuid(), "doomed"))); + await theHost.ExecuteAndWaitAsync(c => + c.InvokeForTenantAsync("smallb", new CreatePartitionedItem(survivorId, "survivor"))); + + await thePartitions.DropTenantAsync("smalla", deleteData: true); + + // The survivor keeps its rows... + var b = await theBuilder.BuildAsync("smallb", CancellationToken.None); + (await b.Items.ToListAsync()).Single().Id.ShouldBe(survivorId); + + // ...and can still write + var moreId = Guid.NewGuid(); + await theHost.ExecuteAndWaitAsync(c => + c.InvokeForTenantAsync("smallb", new CreatePartitionedItem(moreId, "more"))); + + b = await theBuilder.BuildAsync("smallb", CancellationToken.None); + (await b.Items.ToListAsync()).Select(x => x.Id).OrderBy(x => x) + .ShouldBe(new[] { survivorId, moreId }.OrderBy(x => x)); + } + + /// + /// How many distinct physical partitions the given tenants occupy. PostgreSQL partitions by list on + /// the tenant id, so the partition is the child table the row lands in; SQL Server partitions over a + /// compact ordinal, so it is the registered ordinal + /// + private async Task distinctPartitionCountAsync(string[] tenantIds) + { + if (_engine == DatabaseEngine.PostgreSQL) + { + await using var conn = new NpgsqlConnection(Servers.PostgresConnectionString); + await conn.OpenAsync(); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = @" +select count(distinct c.relname) +from pg_class c +join pg_inherits i on i.inhrelid = c.oid +join pg_class parent on i.inhparent = parent.oid +join pg_namespace ns on parent.relnamespace = ns.oid +where ns.nspname = 'conjoined_part' and parent.relname = 'partitioned_items' + and pg_get_expr(c.relpartbound, c.oid) like any (@patterns)"; + var parameter = cmd.CreateParameter(); + parameter.ParameterName = "patterns"; + parameter.Value = tenantIds.Select(x => $"%'{x}'%").ToArray(); + cmd.Parameters.Add(parameter); + return (int)(long)(await cmd.ExecuteScalarAsync())!; + } + + await using var sqlConn = new SqlConnection(Servers.SqlServerConnectionString); + await sqlConn.OpenAsync(); + await using var sqlCmd = sqlConn.CreateCommand(); + var names = tenantIds.Select((_, i) => $"@t{i}").ToArray(); + for (var i = 0; i < tenantIds.Length; i++) + { + sqlCmd.Parameters.AddWithValue($"@t{i}", tenantIds[i]); + } + + sqlCmd.CommandText = + $"SELECT COUNT(DISTINCT ordinal) FROM conjoined_part_wolverine.wolverine_tenant_partitions WHERE tenant_id IN ({string.Join(", ", names)})"; + return (int)(await sqlCmd.ExecuteScalarAsync())!; + } + [Fact] public async Task physical_partition_exists_per_tenant() { diff --git a/src/Persistence/EfCoreTests.MultiTenancy/MultiTenancyDocumentationSamples.cs b/src/Persistence/EfCoreTests.MultiTenancy/MultiTenancyDocumentationSamples.cs index ca1b6aa83..c503f7367 100644 --- a/src/Persistence/EfCoreTests.MultiTenancy/MultiTenancyDocumentationSamples.cs +++ b/src/Persistence/EfCoreTests.MultiTenancy/MultiTenancyDocumentationSamples.cs @@ -261,7 +261,12 @@ public async Task conjoined_partitioned_postgresql() // Weasel-managed physical partitioning: one partition (or shared // bucket) per tenant on every non-saga ITenanted entity table - tenancy => tenancy.PartitionPerTenant()); + tenancy => tenancy.PartitionPerTenant(partitioning => + { + // Opt in before registering two tenants against one suffix. + // Without this a shared suffix is rejected outright + partitioning.AllowPartitionSharing = true; + })); #endregion }); } @@ -275,8 +280,10 @@ public static async Task conjoined_tenant_management(IHost host) // Each tenant gets its own physical partition await partitions.AddTenantAsync("tenant1"); - // Or share one partition between small tenants ("bucketing") -- - // requires AllowPartitionSharing on the partitioning options + // Or share one partition between small tenants ("bucketing") by registering + // them against the same suffix -- requires AllowPartitionSharing above. + // Members can be added one at a time as tenants onboard; the bucket is + // resolved from storage, so they land in the same physical partition await partitions.AddTenantAsync("small-tenant-a", "shared_bucket"); await partitions.AddTenantAsync("small-tenant-b", "shared_bucket"); diff --git a/src/Persistence/Wolverine.SqlServer/MultiTenancy/SqlServerTenantPartitioning.cs b/src/Persistence/Wolverine.SqlServer/MultiTenancy/SqlServerTenantPartitioning.cs index d795ae05d..4152f62b2 100644 --- a/src/Persistence/Wolverine.SqlServer/MultiTenancy/SqlServerTenantPartitioning.cs +++ b/src/Persistence/Wolverine.SqlServer/MultiTenancy/SqlServerTenantPartitioning.cs @@ -79,69 +79,42 @@ public async Task AddTenantsAsync(ILogger logger, IDataba var db = (IDatabase)database; await _partitions.InitializeAsync(db, token); - var ordinals = new Dictionary(); - var tables = new List(); - - // Tenants without a suffix each get their own auto-allocated ordinal. - // Tenants sharing a suffix share one ordinal (a shared physical partition), - // mirroring the PostgreSQL suffix bucketing - var standalone = tenantIdToSuffix.Where(x => x.Value == null).Select(x => x.Key).ToArray(); - if (standalone.Any()) + if (!_options.AllowPartitionSharing) { - accumulate(await _partitions.AddPartitionsToAllTables(logger, db, standalone, token)); + assertNoSharing(tenantIdToSuffix); } + // Weasel resolves each named bucket to its ordinal through the registry's bucket column, so + // members of one bucket land in the same physical partition whether they were registered + // together or one release apart. Wolverine used to resolve the ordinal itself from the + // tenant -> ordinal map, which could not see the bucket at all: a brand new tenant matched + // nothing and silently got a fresh partition (GH-3683 / weasel#391) + var result = await _partitions.AddPartitionsToAllTables(logger, db, tenantIdToSuffix, token); + + return new TenantPartitionResult( + result.Ordinals.ToDictionary(x => x.Key, x => x.Value), + result.Tables.Select(toStatus).ToList()); + } + + /// + /// The bucket -> ordinal map is persisted, so this check spans calls just like the PostgreSQL + /// one: two tenants land in the same bucket whether they were registered together or one + /// release apart + /// + private void assertNoSharing(IReadOnlyDictionary tenantIdToSuffix) + { foreach (var bucket in tenantIdToSuffix.Where(x => x.Value != null).GroupBy(x => x.Value!)) { - if (!_options.AllowPartitionSharing && bucket.Count() > 1) - { - throw new InvalidOperationException( - $"Tenants {bucket.Select(x => x.Key).Join(", ")} share partition suffix '{bucket.Key}', but partition sharing is not enabled. Enable AllowPartitionSharing on the tenant partitioning options."); - } + var alreadyRegistered = _partitions.Buckets.TryGetValue(bucket.Key, out var ordinal) + ? _partitions.Ordinals.Where(x => x.Value == ordinal).Select(x => x.Key) + : []; - // The bucket's ordinal is whichever member is already registered, or - // the next free ordinal for a brand new bucket - var members = bucket.Select(x => x.Key).ToArray(); - var existing = members.Where(m => _partitions.Ordinals.ContainsKey(m)) - .Select(m => _partitions.Ordinals[m]).Distinct().ToArray(); + var members = bucket.Select(x => x.Key).Concat(alreadyRegistered).Distinct().ToArray(); - if (existing.Length > 1) + if (members.Length > 1) { throw new InvalidOperationException( - $"Tenants {members.Join(", ")} for suffix '{bucket.Key}' are already mapped to different partitions ({string.Join(", ", existing)})"); - } - - var ordinal = existing.Length == 1 - ? existing[0] - : (_partitions.Ordinals.Values.DefaultIfEmpty(0).Max() + 1); - - var mapping = members.ToDictionary(m => m, _ => ordinal); - accumulate(await _partitions.AddPartitionsToAllTables(logger, db, mapping, token)); - } - - return new TenantPartitionResult(ordinals, tables); - - void accumulate(TenantPartitionAddResult result) - { - foreach (var pair in result.Ordinals) - { - ordinals[pair.Key] = pair.Value; - } - - // A table can appear once per bucket; the worst status wins so a single - // failed batch isn't masked by a later successful one - foreach (var status in result.Tables) - { - var mapped = toStatus(status); - var index = tables.FindIndex(x => x.TableName == mapped.TableName); - if (index < 0) - { - tables.Add(mapped); - } - else if (tables[index].Status == TenantPartitionStatus.Complete) - { - tables[index] = mapped; - } + $"Tenants {members.Join(", ")} share partition suffix '{bucket.Key}', but partition sharing is not enabled. Enable AllowPartitionSharing on the tenant partitioning options."); } } }