Skip to content

Commit

Permalink
runs Postgres tests with all 3 latest versions
Browse files Browse the repository at this point in the history 
The reasoning is that we are considering Postgres 15
for it's better query planner index selection, which
would be needed for GC queries moving forward
  • Loading branch information
@vroldanbet
vroldanbet committed Sep 27, 2023
1 parent b172ca9 commit 983978c
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 44 deletions.
96 changes: 58 additions & 38 deletions internal/datastore/postgres/postgres_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/authzed/spicedb/internal/datastore/common"
pgcommon "github.com/authzed/spicedb/internal/datastore/postgres/common"
pgversion "github.com/authzed/spicedb/internal/datastore/postgres/version"
"github.com/authzed/spicedb/internal/testfixtures"
testdatastore "github.com/authzed/spicedb/internal/testserver/datastore"
"github.com/authzed/spicedb/pkg/datastore"
Expand All @@ -40,17 +41,34 @@ func (pgd *pgDatastore) ExampleRetryableError() error {
}
}

// the global OTel tracer is used everywhere, so we synchronize tests over a global test tracer
var otelMutex = sync.Mutex{}
var testTraceProvider *trace.TracerProvider

func init() {
testTraceProvider = trace.NewTracerProvider(
trace.WithSampler(trace.AlwaysSample()),
)
otel.SetTracerProvider(testTraceProvider)
}

func TestPostgresDatastore(t *testing.T) {
t.Parallel()

for _, config := range []struct {
targetMigration string
migrationPhase string
pgVersion string
}{
{"head", ""},
{"head", "", pgversion.MinimumSupportedPostgresVersion},
{"head", "", "14"},
{"head", "", "15"},
{"head", "", "16"},
} {
config := config
t.Run(fmt.Sprintf("%s-%s", config.targetMigration, config.migrationPhase), func(t *testing.T) {
t.Run(fmt.Sprintf("postgres-%s-%s-%s", config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) {
t.Parallel()
b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration)
b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion)

test.All(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore {
Expand Down Expand Up @@ -109,11 +127,11 @@ func TestPostgresDatastore(t *testing.T) {
})

t.Run("WatchNotEnabled", func(t *testing.T) {
WatchNotEnabledTest(t, b)
WatchNotEnabledTest(t, b, config.pgVersion)
})

t.Run("GCQueriesServedByExpectedIndexes", func(t *testing.T) {
GCQueriesServedByExpectedIndexes(t, b)
GCQueriesServedByExpectedIndexes(t, b, config.pgVersion)
})

if config.migrationPhase == "" {
Expand Down Expand Up @@ -167,23 +185,32 @@ func TestPostgresDatastore(t *testing.T) {
}

func TestPostgresDatastoreWithoutCommitTimestamps(t *testing.T) {
b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false)
t.Parallel()

// NOTE: watch API requires the commit timestamps, so we skip those tests here.
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(uri,
RevisionQuantization(revisionQuantization),
GCWindow(gcWindow),
GCInterval(gcInterval),
WatchBufferLength(watchBufferLength),
DebugAnalyzeBeforeStatistics(),
)
require.NoError(t, err)
return ds
for _, pgVersion := range []string{pgversion.MinimumSupportedPostgresVersion, "14", "15", "16"} {
pgVersion := pgVersion
t.Run(fmt.Sprintf("postgres-%s", pgVersion), func(t *testing.T) {
t.Parallel()

b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion)

// NOTE: watch API requires the commit timestamps, so we skip those tests here.
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(uri,
RevisionQuantization(revisionQuantization),
GCWindow(gcWindow),
GCInterval(gcInterval),
WatchBufferLength(watchBufferLength),
DebugAnalyzeBeforeStatistics(),
)
require.NoError(t, err)
return ds
})
return ds, nil
}), test.WithCategories(test.WatchCategory))
})
return ds, nil
}), test.WithCategories(test.WatchCategory))
}
}

type datastoreTestFunc func(t *testing.T, ds datastore.Datastore)
Expand Down Expand Up @@ -1067,25 +1094,18 @@ func RevisionInversionTest(t *testing.T, ds datastore.Datastore) {
}

func OTelTracingTest(t *testing.T, ds datastore.Datastore) {
otelMutex.Lock()
defer otelMutex.Unlock()

require := require.New(t)

ctx := context.Background()
r, err := ds.ReadyState(ctx)
require.NoError(err)
require.True(r.IsReady)

// Setup OTel recording
defaultProvider := otel.GetTracerProvider()

provider := trace.NewTracerProvider(
trace.WithSampler(trace.AlwaysSample()),
)
spanrecorder := tracetest.NewSpanRecorder()
provider.RegisterSpanProcessor(spanrecorder)
otel.SetTracerProvider(provider)
defer func() {
otel.SetTracerProvider(defaultProvider)
}()
testTraceProvider.RegisterSpanProcessor(spanrecorder)

// Perform basic operation
_, err = ds.ReadWriteTx(ctx, func(rwt datastore.ReadWriteTransaction) error {
Expand All @@ -1103,10 +1123,10 @@ func OTelTracingTest(t *testing.T, ds datastore.Datastore) {
require.True(present, "missing trace for Streaming gRPC call")
}

func WatchNotEnabledTest(t *testing.T, _ testdatastore.RunningEngineForTest) {
func WatchNotEnabledTest(t *testing.T, _ testdatastore.RunningEngineForTest, pgVersion string) {
require := require.New(t)

ds := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", migrate.Head, false).NewDatastore(t, func(engine, uri string) datastore.Datastore {
ds := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", migrate.Head, false, pgVersion).NewDatastore(t, func(engine, uri string) datastore.Datastore {
ds, err := newPostgresDatastore(uri,
RevisionQuantization(0),
GCWindow(time.Millisecond*1),
Expand All @@ -1130,7 +1150,7 @@ func WatchNotEnabledTest(t *testing.T, _ testdatastore.RunningEngineForTest) {
func BenchmarkPostgresQuery(b *testing.B) {
req := require.New(b)

ds := testdatastore.RunPostgresForTesting(b, "", migrate.Head).NewDatastore(b, func(engine, uri string) datastore.Datastore {
ds := testdatastore.RunPostgresForTesting(b, "", migrate.Head, pgversion.MinimumSupportedPostgresVersion).NewDatastore(b, func(engine, uri string) datastore.Datastore {
ds, err := newPostgresDatastore(uri,
RevisionQuantization(0),
GCWindow(time.Millisecond*1),
Expand Down Expand Up @@ -1161,10 +1181,10 @@ func BenchmarkPostgresQuery(b *testing.B) {
})
}

func datastoreWithInterceptorAndTestData(t *testing.T, interceptor pgcommon.QueryInterceptor) datastore.Datastore {
func datastoreWithInterceptorAndTestData(t *testing.T, interceptor pgcommon.QueryInterceptor, pgVersion string) datastore.Datastore {
require := require.New(t)

ds := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", migrate.Head, false).NewDatastore(t, func(engine, uri string) datastore.Datastore {
ds := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", migrate.Head, false, pgVersion).NewDatastore(t, func(engine, uri string) datastore.Datastore {
ds, err := newPostgresDatastore(uri,
RevisionQuantization(0),
GCWindow(time.Millisecond*1),
Expand Down Expand Up @@ -1306,10 +1326,10 @@ func datastoreWithInterceptorAndTestData(t *testing.T, interceptor pgcommon.Quer
return ds
}

func GCQueriesServedByExpectedIndexes(t *testing.T, _ testdatastore.RunningEngineForTest) {
func GCQueriesServedByExpectedIndexes(t *testing.T, _ testdatastore.RunningEngineForTest, pgVersion string) {
require := require.New(t)
interceptor := &withQueryInterceptor{explanations: make(map[string]string, 0)}
ds := datastoreWithInterceptorAndTestData(t, interceptor)
ds := datastoreWithInterceptorAndTestData(t, interceptor, pgVersion)

// Get the head revision.
ctx := context.Background()
Expand Down
3 changes: 2 additions & 1 deletion internal/testserver/datastore/datastore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/authzed/spicedb/internal/datastore/postgres/version"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/migrate"
)
Expand Down Expand Up @@ -62,7 +63,7 @@ func RunDatastoreEngineWithBridge(t testing.TB, engine string, bridgeNetworkName
case "cockroachdb":
return RunCRDBForTesting(t, bridgeNetworkName)
case "postgres":
return RunPostgresForTesting(t, bridgeNetworkName, migrate.Head)
return RunPostgresForTesting(t, bridgeNetworkName, migrate.Head, version.MinimumSupportedPostgresVersion)
case "mysql":
return RunMySQLForTesting(t, bridgeNetworkName)
case "spanner":
Expand Down
9 changes: 4 additions & 5 deletions internal/testserver/datastore/postgres.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/stretchr/testify/require"

pgmigrations "github.com/authzed/spicedb/internal/datastore/postgres/migrations"
pgversion "github.com/authzed/spicedb/internal/datastore/postgres/version"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/migrate"
"github.com/authzed/spicedb/pkg/secrets"
Expand All @@ -29,11 +28,11 @@ type postgresTester struct {
}

// RunPostgresForTesting returns a RunningEngineForTest for postgres
func RunPostgresForTesting(t testing.TB, bridgeNetworkName string, targetMigration string) RunningEngineForTest {
return RunPostgresForTestingWithCommitTimestamps(t, bridgeNetworkName, targetMigration, true)
func RunPostgresForTesting(t testing.TB, bridgeNetworkName string, targetMigration string, pgVersion string) RunningEngineForTest {
return RunPostgresForTestingWithCommitTimestamps(t, bridgeNetworkName, targetMigration, true, pgVersion)
}

func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName string, targetMigration string, withCommitTimestamps bool) RunningEngineForTest {
func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName string, targetMigration string, withCommitTimestamps bool, pgVersion string) RunningEngineForTest {
pool, err := dockertest.NewPool("")
require.NoError(t, err)

Expand All @@ -47,7 +46,7 @@ func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName s
resource, err := pool.RunWithOptions(&dockertest.RunOptions{
Name: name,
Repository: "postgres",
Tag: pgversion.MinimumSupportedPostgresVersion,
Tag: pgVersion,
Env: []string{"POSTGRES_PASSWORD=secret", "POSTGRES_DB=defaultdb"},
ExposedPorts: []string{"5432/tcp"},
NetworkID: bridgeNetworkName,
Expand Down

0 comments on commit 983978c

Please sign in to comment.