Skip to content

Commit 1367800

Browse files
cemililikclaude
andcommitted
fix(tenancy): detach what the database refused; bound the flights
EF keeps a failed entry in the state it had, so an Added row a uniqueness violation refused stays Added. A caller that turns the conflict into Result.Fail and carries on writing therefore has the rejected INSERT still queued, and the next SaveChanges on the same context re-sends it — the row is gone from the database, and the tracker is a claim that outlived its subject. Reachable through nesting, which ADR-0040 permits: an outer handler may absorb an inner failure and keep going on the same scope, and the scope is one DbContext. Added entries only — a Modified entry's original values are what the database still holds, so detaching it would discard a change the caller may retry. The resolver's coalescing remark now says what bounds the flights, because the dictionary does not: a flight is retired in the read's finally, so its size is lookups in flight rather than hosts ever seen, and the ceiling is the Npgsql pool. A distributed flood of novel hostnames degrades into queueing there — the rate limiter bounds one peer, the negative cache bounds repeats, neither bounds first sight — and the admission gate that would is named rather than built, because its trigger is a measurement nobody has taken. Module: Tenancy ADR: 0040 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6172a8b commit 1367800

3 files changed

Lines changed: 89 additions & 0 deletions

File tree

‎backend/src/LearnStack.Infrastructure/MultiTenancy/CachedHostToTenantResolver.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,17 @@ public sealed class CachedHostToTenantResolver(
119119
/// a round trip the next request would repeat. An earlier version of this sentence
120120
/// said the negative cache is "populated only by a request that survived its own
121121
/// lookup", which the publication block below contradicts in the same file.
122+
///
123+
/// <b>What bounds the flights.</b> The dictionary does not: a flight is registered
124+
/// per host and retired in the read's <c>finally</c>, so its size is the number of
125+
/// lookups in flight, never the number of hosts ever seen. The ceiling is the Npgsql
126+
/// pool — each cold lookup takes a connection, and past the pool maximum the rest
127+
/// queue in <c>OpenConnectionAsync</c> rather than opening more. A distributed flood
128+
/// of novel hostnames therefore degrades into queueing on that pool: the anonymous
129+
/// rate limiter bounds one peer, the negative cache bounds repeats, and neither
130+
/// bounds first sight. A process-wide admission gate for cold lookups is the remedy
131+
/// if that shows up; it is not built, because the trigger for it is a measurement
132+
/// nobody has taken.
122133
/// </remarks>
123134
private async Task<HostResolution?> ReadCoalescedAsync(
124135
string host, string key, CancellationToken cancellationToken)

‎backend/src/Modules/Tenancy/LearnStack.Modules.Tenancy.Infrastructure/Persistence/TenancyWriteStores.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,28 @@ internal static async Task SaveTranslatingConflictsAsync(
125125
catch (DbUpdateException failure)
126126
when (failure.InnerException is PostgresException { SqlState: "23505" } conflict)
127127
{
128+
// Detach what the database refused, before the exception leaves. EF keeps a
129+
// failed entry in the state it had — an Added row stays Added — so a caller
130+
// that turns this into Result.Fail and carries on writing has the rejected
131+
// INSERT still queued, and the NEXT SaveChanges on this context re-sends it.
132+
//
133+
// Reachable through nesting, which is the shape ADR-0040 permits: an outer
134+
// handler may absorb an inner failure and keep going on the same scope, and
135+
// the scope is one DbContext. The row is gone from the database either way —
136+
// the statement was refused — so the tracker holding it is a claim that
137+
// outlived its subject.
138+
//
139+
// Added only. A Modified entry's original values are what the database still
140+
// holds, so leaving it tracked is correct; detaching it would discard a change
141+
// the caller may legitimately retry.
142+
foreach (var entry in failure.Entries)
143+
{
144+
if (entry.State == EntityState.Added)
145+
{
146+
entry.State = EntityState.Detached;
147+
}
148+
}
149+
128150
throw new AggregateConflictException(
129151
conflict.MessageText, conflict.ConstraintName, failure);
130152
}

‎backend/tests/LearnStack.Tests.Integration/Database/TenantProvisioningTests.cs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using LearnStack.Modules.Tenancy.Domain;
99
using LearnStack.Modules.Tenancy.Infrastructure.Persistence;
1010
using LearnStack.SharedKernel.Identifiers;
11+
using LearnStack.SharedKernel.Errors;
1112
using LearnStack.SharedKernel.Persistence;
1213
using LearnStack.SharedKernel.Results;
1314
using LearnStack.SharedKernel.Tenancy;
@@ -417,6 +418,61 @@ public async Task Updating_a_detached_aggregate_is_refused_rather_than_guessed_a
417418
await unitOfWork.RollbackAsync();
418419
}
419420

421+
[Fact]
422+
public async Task A_refused_write_does_not_ride_along_on_the_next_save()
423+
{
424+
// EF keeps a failed entry in the state it had, so an Added row the database
425+
// refused stays Added. A caller that turns the conflict into Result.Fail and
426+
// carries on writing therefore has the rejected INSERT still queued, and the next
427+
// SaveChanges on the same context re-sends it — the row is gone from the database,
428+
// and the tracker is a claim that outlived its subject.
429+
//
430+
// Reachable through nesting, which ADR-0040 permits: an outer handler may absorb
431+
// an inner failure and keep going on the same scope, and the scope is one
432+
// DbContext. Driven directly here, because what is under test is the store's
433+
// cleanup and not the pipeline that would carry it.
434+
await using var provider = BuildProvider();
435+
await using var scope = provider.CreateAsyncScope();
436+
var services = scope.ServiceProvider;
437+
438+
services.GetRequiredService<ITenantContextAccessor>().Current =
439+
new ResolvedContext(SchemaFixture.TenantA, SchemaFixture.OrgA1);
440+
441+
var unitOfWork = services.GetRequiredService<IUnitOfWork>();
442+
await unitOfWork.BeginTransactionAsync();
443+
await unitOfWork.SetTenantContextAsync(services.GetRequiredService<ITenantContext>());
444+
445+
var organizations = services.GetRequiredService<IOrganizationWriteStore>();
446+
447+
// Refused: OrgA1 already exists under this tenant.
448+
var refused = Organization.Create(
449+
OrganizationId.From(SchemaFixture.OrgA1),
450+
TenantId.From(SchemaFixture.TenantA),
451+
"collides",
452+
"Collides",
453+
Clock,
454+
UserId.SystemActor);
455+
456+
var conflict = async () => await organizations.AddAsync(refused);
457+
await conflict.Should().ThrowAsync<AggregateConflictException>();
458+
459+
// The caller absorbs it and writes something else on the same context.
460+
var accepted = Organization.Create(
461+
OrganizationId.From(Guid.CreateVersion7()),
462+
TenantId.From(SchemaFixture.TenantA),
463+
$"after-{Guid.CreateVersion7():N}"[..20],
464+
"After",
465+
Clock,
466+
UserId.SystemActor);
467+
468+
var second = async () => await organizations.AddAsync(accepted);
469+
470+
await second.Should().NotThrowAsync(
471+
"the refused row was detached, so the second save carries only the new one");
472+
473+
await unitOfWork.RollbackAsync();
474+
}
475+
420476
// ── Harness ──────────────────────────────────────────────────────────────
421477

422478
/// <summary>

0 commit comments

Comments
 (0)