From fefdc43db7a2c288096850399888d5dab6639edd Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 04:35:14 +0000 Subject: [PATCH 1/2] test(auth): add a 50-caller stress variant of the bootstrap-race integration test TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress fires 50 barrier-synchronized CreateAdminIfNone callers per round for 10 rounds and requires exactly one winner, no errors and exactly one Administrators-group row after each round. Removing the pg_advisory_xact_lock call, or switching it to pg_try_advisory_xact_lock, makes it fail with 2 to 10 winners in a round. The two-caller test and the stress variant share one helper. Workers only send outcomes over a channel and every assertion runs on the test goroutine, after all workers have finished. The stress variant takes its database from the shared container (testhelpers.RequireMigratedDatabase), and a new integration-only TestMain stops that container, as in internal/config. Closes #64 Co-Authored-By: Claude Opus 5.5 --- internal/auth/main_integration_test.go | 27 +++++++ internal/auth/store_postgres_db_test.go | 102 +++++++++++++++++------- 2 files changed, 102 insertions(+), 27 deletions(-) create mode 100644 internal/auth/main_integration_test.go diff --git a/internal/auth/main_integration_test.go b/internal/auth/main_integration_test.go new file mode 100644 index 000000000..566abb466 --- /dev/null +++ b/internal/auth/main_integration_test.go @@ -0,0 +1,27 @@ +//go:build integration +// +build integration + +package auth + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container shared by the integration tests in +// this package that take their database from testhelpers.RequireMigratedDatabase +// (issue #982). Each such test still gets a freshly migrated database of its +// own; only the container and the migrated template it is cloned from outlive +// a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/auth/store_postgres_db_test.go b/internal/auth/store_postgres_db_test.go index 8326e5ff6..46982ebf3 100644 --- a/internal/auth/store_postgres_db_test.go +++ b/internal/auth/store_postgres_db_test.go @@ -8,7 +8,8 @@ package auth // These run the real SQL against a migrated PostgreSQL testcontainer, // proving the 19-column bootstrap INSERT, the NOT EXISTS guard and its // agreement with AdminExists (active-membership semantics), and the -// insert-once guarantee under two concurrent CreateAdminIfNone calls. +// insert-once guarantee under concurrent CreateAdminIfNone calls (two +// callers, and a 50-caller stress variant). import ( "context" @@ -165,48 +166,95 @@ func TestIntegration_CreateAdminIfNone_Bootstrap(t *testing.T) { // TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce proves the // insert-once semantics under two concurrent CreateAdminIfNone calls: exactly // one caller wins and exactly one admin row exists afterwards, across -// repeated barrier-synchronized attempts. +// repeated barrier-synchronized attempts. Two callers is the production case +// (two operators bootstrapping at once) and keeps this the cheap fast path. func TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce(t *testing.T) { - db := setupAuthTestDB(t) - store := NewPostgresStore(db) - ctx := context.Background() + requireConcurrentBootstrapOnce(t, setupAuthTestDB(t), 2, 25) +} + +// TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress is the +// high-fanout variant (issue #64): 50 barrier-synchronized callers per +// iteration. Fifty callers queue on the test pool's 10 connections as well as +// on the bootstrap advisory lock, so a change that weakens the lock (dropping +// it, or switching to pg_try_advisory_xact_lock and ignoring the result) shows +// up as several winners in a round rather than depending on two callers +// happening to interleave, and the test shows the lock holds when every pool +// connection is taken by a contender. +// +// It takes its database from the binary's shared container +// (testhelpers.RequireMigratedDatabase, stopped by TestMain) rather than +// starting a container of its own. +func TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() - const iterations = 25 + db := testhelpers.RequireMigratedDatabase(ctx, t, getAuthTestMigrationsPath()).DB + requireConcurrentBootstrapOnce(t, db, 50, 10) +} + +// bootstrapOutcome is what each concurrent CreateAdminIfNone caller reports +// back to the test goroutine. Workers only send these; every assertion runs on +// the test goroutine. +type bootstrapOutcome struct { + created bool + err error +} + +// requireConcurrentBootstrapOnce runs iterations rounds of callers concurrent +// CreateAdminIfNone calls, released together by a barrier against an empty +// users table, and requires exactly one winner, no errors, and exactly one +// admin row after each round. +func requireConcurrentBootstrapOnce(t *testing.T, db *database.Connection, callers, iterations int) { + t.Helper() + + store := NewPostgresStore(db) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() for i := 0; i < iterations; i++ { resetUsers(t, db) - type outcome struct { - created bool - err error - } - results := make(chan outcome, 2) - start := make(chan struct{}) - - for n := 0; n < 2; n++ { - user := newBootstrapCandidate(fmt.Sprintf("admin-%d-%d@example.com", i, n)) - go func(u *User) { - <-start - created, err := store.CreateAdminIfNone(ctx, u) - results <- outcome{created: created, err: err} - }(user) - } - close(start) + outcomes := runBootstrapRound(ctx, store, callers, i) winners := 0 - for n := 0; n < 2; n++ { - r := <-results - require.NoError(t, r.err, "iteration %d", i) + for n, r := range outcomes { + require.NoError(t, r.err, "iteration %d, caller %d", i, n) if r.created { winners++ } } require.Equal(t, 1, winners, - "iteration %d: exactly one of two concurrent bootstrap calls must win", i) + "iteration %d: exactly one of %d concurrent bootstrap calls must win", i, callers) count, err := store.CountGroupMembers(ctx, DefaultAdminGroupID) require.NoError(t, err) require.Equal(t, 1, count, - "iteration %d: exactly one admin row must exist after concurrent bootstrap", i) + "iteration %d: exactly one admin row must exist after %d concurrent bootstrap calls", i, callers) + } +} + +// runBootstrapRound starts callers goroutines that each wait on a shared +// barrier and then call CreateAdminIfNone with a distinct email, and returns +// every caller's outcome once all of them have finished. Collecting all of +// them before asserting means no worker is still using the pool when the test +// fails and its cleanup closes the connection. +func runBootstrapRound(ctx context.Context, store *PostgresStore, callers, iteration int) []bootstrapOutcome { + results := make(chan bootstrapOutcome, callers) + start := make(chan struct{}) + + for n := 0; n < callers; n++ { + user := newBootstrapCandidate(fmt.Sprintf("admin-%d-%d@example.com", iteration, n)) + go func(u *User) { + <-start + created, err := store.CreateAdminIfNone(ctx, u) + results <- bootstrapOutcome{created: created, err: err} + }(user) + } + close(start) + + outcomes := make([]bootstrapOutcome, 0, callers) + for n := 0; n < callers; n++ { + outcomes = append(outcomes, <-results) } + return outcomes } From d94b15d4832994a0ec072007a75b6c1af200c0cb Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 16:28:42 +0000 Subject: [PATCH 2/2] fix(auth): set up the bootstrap stress test with setupAuthTestDB Main (#1400) moved setupAuthTestDB onto the shared Postgres server (testhelpers.RequireMigratedDatabase), which is what the stress variant did by hand to avoid starting its own container. Use the shared helper and drop the comment that contrasted the two. Co-Authored-By: Claude Opus 5.5 --- internal/auth/store_postgres_db_test.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/auth/store_postgres_db_test.go b/internal/auth/store_postgres_db_test.go index 2c3364cbc..6c580231d 100644 --- a/internal/auth/store_postgres_db_test.go +++ b/internal/auth/store_postgres_db_test.go @@ -174,16 +174,8 @@ func TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce(t *testing.T) { // up as several winners in a round rather than depending on two callers // happening to interleave, and the test shows the lock holds when every pool // connection is taken by a contender. -// -// It takes its database from the binary's shared container -// (testhelpers.RequireMigratedDatabase, stopped by TestMain) rather than -// starting a container of its own. func TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancel() - - db := testhelpers.RequireMigratedDatabase(ctx, t, getAuthTestMigrationsPath()).DB - requireConcurrentBootstrapOnce(t, db, 50, 10) + requireConcurrentBootstrapOnce(t, setupAuthTestDB(t), 50, 10) } // bootstrapOutcome is what each concurrent CreateAdminIfNone caller reports