diff --git a/internal/datastore/postgres/postgres_shared_test.go b/internal/datastore/postgres/postgres_shared_test.go index a343b5e6a8..fb6b4b2e8f 100644 --- a/internal/datastore/postgres/postgres_shared_test.go +++ b/internal/datastore/postgres/postgres_shared_test.go @@ -81,12 +81,35 @@ func testPostgresDatastore(t *testing.T, pc []postgresConfig) { if config.pgbouncer { pgbouncerStr = "pgbouncer-" } + + t.Run(fmt.Sprintf("%spostgres-%s-%s-%s-gc", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) { + b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer) + ctx := context.Background() + + // NOTE: gc tests take exclusive locks, so they are run under non-parallel. + test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(gcInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + MigrationPhase(config.migrationPhase), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), false) + }) + t.Run(fmt.Sprintf("%spostgres-%s-%s-%s", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) { t.Parallel() b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer) ctx := context.Background() - test.All(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, RevisionQuantization(revisionQuantization), @@ -100,7 +123,7 @@ func testPostgresDatastore(t *testing.T, pc []postgresConfig) { return ds }) return ds, nil - }), false) + }), test.WithCategories(test.GCCategory), false) t.Run("TransactionTimestamps", createDatastoreTest( b, @@ -261,6 +284,7 @@ func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, pc []postgresCon b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer) // NOTE: watch API requires the commit timestamps, so we skip those tests here. + // NOTE: gc tests take exclusive locks, so they are run under non-parallel. test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, @@ -274,7 +298,26 @@ func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, pc []postgresCon return ds }) return ds, nil - }), test.WithCategories(test.WatchCategory), false) + }), test.WithCategories(test.WatchCategory, test.GCCategory), false) + }) + + t.Run(fmt.Sprintf("postgres-%s-gc", pgVersion), func(t *testing.T) { + ctx := context.Background() + b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer) + test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(gcInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), false) }) } } diff --git a/pkg/datastore/test/datastore.go b/pkg/datastore/test/datastore.go index 40b34c01a3..fc3802bb86 100644 --- a/pkg/datastore/test/datastore.go +++ b/pkg/datastore/test/datastore.go @@ -152,8 +152,7 @@ func AllWithExceptions(t *testing.T, tester DatastoreTester, except Categories, t.Run("TestCheckRevisions", runner(tester, CheckRevisionsTest)) if !except.GC() { - t.Run("TestRevisionGC", runner(tester, RevisionGCTest)) - t.Run("TestInvalidReads", runner(tester, InvalidReadsTest)) + OnlyGCTests(t, tester, concurrent) } t.Run("TestBulkUpload", runner(tester, BulkUploadTest)) @@ -201,6 +200,16 @@ func AllWithExceptions(t *testing.T, tester DatastoreTester, except Categories, t.Run("TestRelationshipCounterOverExpired", runner(tester, RelationshipCounterOverExpiredTest)) } +func OnlyGCTests(t *testing.T, tester DatastoreTester, concurrent bool) { + runner := serial + if concurrent { + runner = parallel + } + + t.Run("TestRevisionGC", runner(tester, RevisionGCTest)) + t.Run("TestInvalidReads", runner(tester, InvalidReadsTest)) +} + // All runs all generic datastore tests on a DatastoreTester. func All(t *testing.T, tester DatastoreTester, concurrent bool) { AllWithExceptions(t, tester, noException, concurrent) diff --git a/pkg/datastore/test/revisions.go b/pkg/datastore/test/revisions.go index 0354329997..63dbd6f4fc 100644 --- a/pkg/datastore/test/revisions.go +++ b/pkg/datastore/test/revisions.go @@ -131,7 +131,7 @@ func RevisionGCTest(t *testing.T, tester DatastoreTester) { gcable, ok := ds.(common.GarbageCollector) if ok { gcable.ResetGCCompleted() - require.Eventually(func() bool { return gcable.HasGCRun() }, 5*time.Second, 50*time.Millisecond, "GC was never run as expected") + require.Eventually(func() bool { return gcable.HasGCRun() }, 10*time.Second, 100*time.Millisecond, "GC was never run as expected") } // FIXME currently the various datastores behave differently when a revision was requested and GC Window elapses.