Skip to content

Commit b7d5edc

Browse files
jeremydmillerjeremydmiller
authored
Make managed tenant partition bucketing actually share a partition (#391) (#392)
* fix(partitioning): make managed tenant bucketing actually share a partition (#391) Both managed-partition strategies advertised multi-tenant "bucketing" -- several small tenants sharing one physical partition to stay clear of per-table partition ceilings -- but neither could express it end to end. Reproduced against real PostgreSQL and SQL Server. PostgreSQL (ManagedListPartitions) AddPartitionToAllTables emitted one single-value ListPartition per value, so two values sharing a suffix produced two CREATE TABLE IF NOT EXISTS statements against the same partition table name. The first created it with only its own value and the second was silently swallowed, so the partition never accepted the second value and its first write died with 23514 "no partition of relation found for row". The additive path now groups by SANITIZED suffix (that is what names the physical table) and carries the bucket's full membership. Since CREATE TABLE IF NOT EXISTS cannot alter an existing bound, widening an already-present partition is DETACH + re-ATTACH inside ONE transaction -- a failure between them would orphan the partition and leave every tenant in the bucket unroutable. DropPartitionFromAllTablesForValue resolved a value to its suffix and then dropped BY SUFFIX, deleting every registry row in the bucket and DROPping the shared partition table. Removing one small tenant therefore silently destroyed its co-tenants' rows. It now narrows the bucket -- deleting only the departing value's rows, then re-binding to the remaining members -- and releases the partition only with the last member. SQL Server (ManagedTenantPartitions) Bucketing worked inside a single batch (the caller supplied the shared ordinal itself) but silently did not across calls -- the natural tenant-onboarding shape. The registry persisted only tenant_id -> ordinal, so a brand-new tenant had no way to discover which ordinal its bucket already owned and each call allocated a fresh one; the tenants never actually shared. The registry gains a nullable `bucket` column plus a bucket-aware AddPartitionsToAllTables overload taking tenant -> bucket. Deliberately a COLUMN rather than a pseudo-tenant row: a row would keep the ordinal referenced forever, defeating release-on-last-member and TenantDropBehavior.DeleteData. A bucket is forgotten once its last member is dropped. Coverage: 5 new PostgreSQL tests (all red on the prior code) and 5 new SQL Server tests. Full suites green -- Weasel.Postgresql 780/783, Weasel.SqlServer 323/331 (skips pre-existing). * release: 9.20.0 New API surface on ManagedTenantPartitions (bucket-aware AddPartitionsToAllTables + Buckets), so a minor bump rather than a patch. Wolverine's GH-3683 adoption depends on this version.
1 parent 4e688c2 commit b7d5edc

5 files changed

Lines changed: 716 additions & 17 deletions

File tree

‎Directory.Build.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<Project>
33
<PropertyGroup>
4-
<Version>9.19.1</Version>
4+
<Version>9.20.0</Version>
55
<LangVersion>13.0</LangVersion>
66
<ImplicitUsings>enable</ImplicitUsings>
77
<Nullable>enable</Nullable>

‎src/Weasel.Postgresql.Tests/Tables/partitioning/managed_list_partitions.cs‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,217 @@ await conn.CreateCommand(
414414
(await partitionExists("teams", "orphan")).ShouldBeTrue();
415415
}
416416

417+
[Fact]
418+
public async Task bucketed_values_registered_together_share_one_partition()
419+
{
420+
// weasel#391: several values deliberately sharing one suffix ("bucketing") must land in ONE
421+
// physical partition carrying every member's value.
422+
await dropManagedListsSchema();
423+
var database = new ManagedListDatabase();
424+
await database.Partitions.ResetValues(database, new Dictionary<string, string>(), CancellationToken.None);
425+
await database.ApplyAllConfiguredChangesToDatabaseAsync();
426+
427+
await database.Partitions.AddPartitionToAllTables(NullLogger.Instance, database,
428+
new Dictionary<string, string> { { "smalla", "shared_bucket" }, { "smallb", "shared_bucket" } },
429+
CancellationToken.None);
430+
431+
(await partitionValues("teams", "shared_bucket")).ShouldBe(["smalla", "smallb"]);
432+
(await partitionValues("players", "shared_bucket")).ShouldBe(["smalla", "smallb"]);
433+
}
434+
435+
[Fact]
436+
public async Task bucketed_values_registered_separately_share_one_partition()
437+
{
438+
// weasel#391, the headline defect: bucket members are normally registered ONE AT A TIME as tenants
439+
// onboard. AddPartitionToAllTables emitted a single-value ListPartition per value, so the second
440+
// member's CREATE TABLE IF NOT EXISTS was a silent no-op against the partition the first member had
441+
// already created. The bound kept only the first value and the second tenant's very first write died
442+
// with 23514 "no partition of relation found for row".
443+
await dropManagedListsSchema();
444+
var database = new ManagedListDatabase();
445+
await database.Partitions.ResetValues(database, new Dictionary<string, string>(), CancellationToken.None);
446+
await database.ApplyAllConfiguredChangesToDatabaseAsync();
447+
448+
await database.Partitions.AddPartitionToAllTables(database, "smalla", "shared_bucket", CancellationToken.None);
449+
await database.Partitions.AddPartitionToAllTables(database, "smallb", "shared_bucket", CancellationToken.None);
450+
451+
try
452+
{
453+
(await partitionValues("teams", "shared_bucket")).ShouldBe(["smalla", "smallb"]);
454+
455+
// Both members write, and both rows land in the SAME physical partition — the entire point of
456+
// bucketing. Pre-fix the second insert threw.
457+
await insertTeam("Lions", "smalla");
458+
await insertTeam("Tigers", "smallb");
459+
460+
(await partitionOfTeam("Lions")).ShouldBe("managed_lists.teams_shared_bucket");
461+
(await partitionOfTeam("Tigers")).ShouldBe("managed_lists.teams_shared_bucket");
462+
}
463+
finally
464+
{
465+
// These rows would otherwise outlive the test and break a sibling's migration: the managed_lists
466+
// schema is shared across this class, and a later ResetValues to a partition set that no longer
467+
// covers 'smalla' makes the table rebuild fail with 23514.
468+
await dropManagedListsSchema();
469+
}
470+
}
471+
472+
[Fact]
473+
public async Task widening_a_bucket_preserves_the_rows_already_in_it()
474+
{
475+
// Widening is DETACH + re-ATTACH rather than CREATE IF NOT EXISTS, so it has to be non-destructive
476+
// for the members already living in the partition.
477+
await dropManagedListsSchema();
478+
var database = new ManagedListDatabase();
479+
await database.Partitions.ResetValues(database, new Dictionary<string, string>(), CancellationToken.None);
480+
await database.ApplyAllConfiguredChangesToDatabaseAsync();
481+
482+
try
483+
{
484+
await database.Partitions.AddPartitionToAllTables(database, "smalla", "shared_bucket",
485+
CancellationToken.None);
486+
await insertTeam("Lions", "smalla");
487+
488+
await database.Partitions.AddPartitionToAllTables(database, "smallb", "shared_bucket",
489+
CancellationToken.None);
490+
491+
// The incumbent's row survives the DETACH/re-ATTACH untouched...
492+
(await teamNames()).ShouldBe(["Lions"]);
493+
(await partitionOfTeam("Lions")).ShouldBe("managed_lists.teams_shared_bucket");
494+
495+
// ...and the widen genuinely took effect, rather than being the old silent no-op.
496+
await insertTeam("Tigers", "smallb");
497+
(await partitionOfTeam("Tigers")).ShouldBe("managed_lists.teams_shared_bucket");
498+
(await teamNames()).ShouldBe(["Lions", "Tigers"]);
499+
}
500+
finally
501+
{
502+
await dropManagedListsSchema();
503+
}
504+
}
505+
506+
[Fact]
507+
public async Task dropping_one_bucket_member_preserves_the_other_members_data()
508+
{
509+
// weasel#391: DropPartitionFromAllTablesForValue resolved the value to its suffix and then dropped
510+
// BY SUFFIX — deleting every registry row in the bucket and DROPping the shared partition table.
511+
// Removing one small tenant therefore silently destroyed every co-tenant's rows.
512+
await dropManagedListsSchema();
513+
var database = new ManagedListDatabase();
514+
await database.Partitions.ResetValues(database, new Dictionary<string, string>(), CancellationToken.None);
515+
await database.ApplyAllConfiguredChangesToDatabaseAsync();
516+
517+
try
518+
{
519+
await database.Partitions.AddPartitionToAllTables(database, "smalla", "shared_bucket",
520+
CancellationToken.None);
521+
await database.Partitions.AddPartitionToAllTables(database, "smallb", "shared_bucket",
522+
CancellationToken.None);
523+
await insertTeam("Lions", "smalla");
524+
await insertTeam("Tigers", "smallb");
525+
526+
await database.Partitions.DropPartitionFromAllTablesForValue(database, NullLogger.Instance, "smalla",
527+
CancellationToken.None);
528+
529+
// The survivor keeps its partition, its bound, and — the part that used to be lost — its rows.
530+
(await partitionValues("teams", "shared_bucket")).ShouldBe(["smallb"]);
531+
(await teamNames()).ShouldBe(["Tigers"]);
532+
533+
// and can still write afterwards
534+
await insertTeam("Bears", "smallb");
535+
(await teamNames()).ShouldBe(["Bears", "Tigers"]);
536+
537+
// The departed value is genuinely deregistered, not merely unrouted.
538+
database.Partitions.ForceReload();
539+
await database.Partitions.InitializeAsync(database, CancellationToken.None);
540+
database.Partitions.Partitions.ShouldNotContainKey("smalla");
541+
database.Partitions.Partitions.ShouldContainKey("smallb");
542+
}
543+
finally
544+
{
545+
await dropManagedListsSchema();
546+
}
547+
}
548+
549+
[Fact]
550+
public async Task the_partition_is_released_only_with_the_last_bucket_member()
551+
{
552+
await dropManagedListsSchema();
553+
var database = new ManagedListDatabase();
554+
await database.Partitions.ResetValues(database, new Dictionary<string, string>(), CancellationToken.None);
555+
await database.ApplyAllConfiguredChangesToDatabaseAsync();
556+
557+
await database.Partitions.AddPartitionToAllTables(database, "smalla", "shared_bucket", CancellationToken.None);
558+
await database.Partitions.AddPartitionToAllTables(database, "smallb", "shared_bucket", CancellationToken.None);
559+
560+
await database.Partitions.DropPartitionFromAllTablesForValue(database, NullLogger.Instance, "smalla",
561+
CancellationToken.None);
562+
563+
// Still one member left, so the physical partition survives.
564+
(await partitionExists("teams", "shared_bucket")).ShouldBeTrue();
565+
566+
await database.Partitions.DropPartitionFromAllTablesForValue(database, NullLogger.Instance, "smallb",
567+
CancellationToken.None);
568+
569+
// Last member gone — now the partition is released on every managed table.
570+
(await partitionExists("teams", "shared_bucket")).ShouldBeFalse();
571+
(await partitionExists("players", "shared_bucket")).ShouldBeFalse();
572+
}
573+
574+
private static async Task insertTeam(string name, string color)
575+
{
576+
await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString);
577+
await conn.OpenAsync();
578+
await conn.CreateCommand("insert into managed_lists.teams (name, color) values (:name, :color)")
579+
.With("name", name)
580+
.With("color", color)
581+
.ExecuteNonQueryAsync();
582+
}
583+
584+
private static async Task<string[]> teamNames()
585+
{
586+
await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString);
587+
await conn.OpenAsync();
588+
await using var reader = await conn.CreateCommand("select name from managed_lists.teams order by name")
589+
.ExecuteReaderAsync();
590+
591+
var names = new List<string>();
592+
while (await reader.ReadAsync())
593+
{
594+
names.Add(reader.GetString(0));
595+
}
596+
597+
return names.ToArray();
598+
}
599+
600+
private static async Task<string?> partitionOfTeam(string name)
601+
{
602+
await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString);
603+
await conn.OpenAsync();
604+
return await conn.CreateCommand("select tableoid::regclass::text from managed_lists.teams where name = :name")
605+
.With("name", name)
606+
.ExecuteScalarAsync() as string;
607+
}
608+
609+
/// <summary>
610+
/// The values a partition's bound actually covers, sorted so the assertion does not depend on the order
611+
/// PostgreSQL happens to echo them back in.
612+
/// </summary>
613+
private static async Task<string[]> partitionValues(string parent, string suffix)
614+
{
615+
await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString);
616+
await conn.OpenAsync();
617+
var bound = await conn.CreateCommand(
618+
"select pg_get_expr(c.relpartbound, c.oid) from pg_class c join pg_namespace n on n.oid = c.relnamespace where n.nspname = 'managed_lists' and c.relname = :name")
619+
.With("name", $"{parent}_{suffix}")
620+
.ExecuteScalarAsync() as string;
621+
622+
if (bound == null) return [];
623+
624+
var inner = bound.Substring(bound.IndexOf('(') + 1, bound.LastIndexOf(')') - bound.IndexOf('(') - 1);
625+
return inner.Split(',').Select(x => x.Trim().Trim('\'')).OrderBy(x => x).ToArray();
626+
}
627+
417628
private static async Task dropManagedListsSchema()
418629
{
419630
await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString);

0 commit comments

Comments
 (0)