chore: add cockroachdb 26.1 to the matrix

Author: tstirrat15

.github/workflows/build-test.yaml

@@ -262,7 +262,11 @@ jobs:
       fail-fast: false
       matrix:
         datastore: ["crdb"]
-        crdbversion: ["25.2.0", "25.3.0"]
+        crdbversion:
+          # the version many folks are on
+          - "25.2.0"
+          # the most recent version
+          - "26.1.0"
     steps:
       - uses: "actions/checkout@v6"
         if: |
@@ -297,7 +301,11 @@ jobs:
       fail-fast: false
       matrix:
         datastore: ["crdb"]
-        crdbversion: ["25.2.0", "25.3.0"]
+        crdbversion:
+          # the version many folks are on
+          - "25.2.0"
+          # the most recent version
+          - "26.1.0"
     steps:
       - uses: "actions/checkout@v6"
         if: |

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Begin deprecation of library "github.com/dlmiddlecote/sqlstats" (https://github.com/authzed/spicedb/pull/2904).
   NOTE: in a future release, MySQL metrics will change.
+- Added support for CRDB 26.1 by fixing how version information is read from the cluster
 
 ### Fixed
 - enforce graceful shutdown on serve and serve-testing (https://github.com/authzed/spicedb/pull/2888)

internal/datastore/crdb/crdb_test.go

@@ -17,6 +17,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -112,6 +113,14 @@ func TestCRDBDatastoreWithoutIntegrity(t *testing.T) {
 		GCWindow(veryLargeGCWindow),
 		WithAcquireTimeout(5*time.Second),
 	))
+
+	t.Run("TestVersionReading", createDatastoreTest(
+		b,
+		VersionReadingTest,
+		RevisionQuantization(0),
+		GCWindow(veryLargeGCWindow),
+		WithAcquireTimeout(5*time.Second),
+	))
 }
 
 type datastoreTestFunc func(t *testing.T, ds datastore.Datastore)
@@ -960,3 +969,32 @@ func TestRegisterPrometheusCollectors(t *testing.T) {
 	require.NotNil(t, poolReadMetric)
 	require.Equal(t, float64(readMaxConns), poolReadMetric.GetGauge().GetValue()) //nolint:testifylint // we expect exact values
 }
+
+func VersionReadingTest(t *testing.T, _ datastore.Datastore) {
+	require := require.New(t)
+
+	expectedVersionList := strings.Split(crdbTestVersion(), ".")
+	expectedMajor := expectedVersionList[0]
+	expectedMinor := expectedVersionList[1]
+	expectedPatch := expectedVersionList[2]
+
+	const (
+		readMaxConns = 10
+	)
+	// Set up a raw connection to the DB
+	initPoolConfig, err := pgxpool.ParseConfig(fmt.Sprintf("postgres://db:password@pg.example.com:5432/mydb?pool_max_conns=%d", readMaxConns))
+	require.NoError(err)
+	initPool, err := pool.NewRetryPool(t.Context(), "read", initPoolConfig, nil, 18, 20)
+	require.NoError(err)
+	t.Cleanup(func() {
+		initPool.Close()
+	})
+
+	var version crdbVersion
+	err = queryServerVersion(t.Context(), initPool, &version)
+	require.NoError(err)
+
+	require.Equal(expectedMajor, version.Major)
+	require.Equal(expectedMinor, version.Minor)
+	require.Equal(expectedPatch, version.Patch)
+}

internal/datastore/crdb/version.go

@@ -2,48 +2,33 @@ package crdb
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"regexp"
 	"strconv"
 
 	"github.com/jackc/pgx/v5"
-	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/rs/zerolog"
 
 	pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common"
 )
 
 const (
-	queryVersionJSON = "SELECT crdb_internal.active_version()::jsonb;"
-	queryVersion     = "SELECT version();"
-
-	errFunctionDoesNotExist = "42883"
+	queryVersion = "SELECT version();"
 )
 
 var versionRegex = regexp.MustCompile(`v([0-9]+)\.([0-9]+)\.([0-9]+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+)?`)
 
+// queryServerVersion reads the version() string out of CRDB
+// and then parses it using a regex to determine the semver.
 func queryServerVersion(ctx context.Context, db pgxcommon.DBFuncQuerier, version *crdbVersion) error {
+	var versionStr string
 	if err := db.QueryRowFunc(ctx, func(ctx context.Context, row pgx.Row) error {
-		return row.Scan(version)
-	}, queryVersionJSON); err != nil {
-		var pgerr *pgconn.PgError
-		if !errors.As(err, &pgerr) || pgerr.Code != errFunctionDoesNotExist {
-			return err
-		}
-
-		// The crdb_internal.active_version() wasn't added until v22.1.X, try to parse the version
-		var versionStr string
-		if err := db.QueryRowFunc(ctx, func(ctx context.Context, row pgx.Row) error {
-			return row.Scan(&versionStr)
-		}, queryVersion); err != nil {
-			return err
-		}
-
-		return parseVersionStringInto(versionStr, version)
+		return row.Scan(&versionStr)
+	}, queryVersion); err != nil {
+		return err
 	}
 
-	return nil
+	return parseVersionStringInto(versionStr, version)
 }
 
 func parseVersionStringInto(versionStr string, version *crdbVersion) error {

internal/datastore/crdb/version/version.go

@@ -8,4 +8,4 @@ const MinimumSupportedCockroachDBVersion = "23.1.30"
 // LatestTestedCockroachDBVersion is the latest version of CockroachDB that has been tested with this driver.
 //
 // NOTE: must match a tag on DockerHub for the `cockroachdb/cockroach` image, without the `v` prefix.
-const LatestTestedCockroachDBVersion = "25.3.2"
+const LatestTestedCockroachDBVersion = "26.1.0"

pkg/datastore/test/revisions.go

@@ -20,7 +20,6 @@ import (
 
 // RevisionQuantizationTest tests whether or not the requirements for revisions hold
 // for a particular datastore.
-// TODO: rewrite using synctest
 func RevisionQuantizationTest(t *testing.T, tester DatastoreTester) {
 	testCases := []struct {
 		quantizationRange        time.Duration
@@ -178,6 +177,7 @@ func RevisionGCTest(t *testing.T, tester DatastoreTester) {
 	time.Sleep(gcWindow)
 
 	gcable, ok := ds.(common.GarbageCollectableDatastore)
+	// NOTE: CRDB and Spanner both do garbage collection with row-level TTLs
 	if ok {
 		// Run garbage collection.
 		gcable.ResetGCCompleted()
@@ -198,9 +198,14 @@ func RevisionGCTest(t *testing.T, tester DatastoreTester) {
 	// require.Error(err, "expected previously written schema to exist at out-of-GC window head")
 
 	// check freshly fetched head revision is valid after GC window elapsed
-	head, err = ds.HeadRevision(ctx)
 	require.NoError(err)
 
+	// assert that recent call to head revision is also valid, even after a GC window cycle without writes elapsed
+	require.NoError(ds.CheckRevision(ctx, head), "expected freshly obtained head revision to be valid")
+
+	// TODO: these reads are taking a significant amount of time on CRDB, on the order
+	// of 100ms for a row read. We need to ascertain whether this is a test artifact
+	// or a performance regression.
 	// check that we can read a caveat whose revision has been garbage collectged
 	_, _, err = ds.SnapshotReader(head).LegacyReadCaveatByName(ctx, testCaveat.Name)
 	require.NoError(err, "expected previously written caveat should exist at head")
@@ -213,9 +218,6 @@ func RevisionGCTest(t *testing.T, tester DatastoreTester) {
 	_, _, err = ds.SnapshotReader(head).LegacyReadNamespaceByName(ctx, "foo/bar")
 	require.NoError(err, "expected previously written schema to exist at head")
 
-	// and that recent call to head revision is also valid, even after a GC window cycle without writes elapsed
-	require.NoError(ds.CheckRevision(ctx, head), "expected freshly obtained head revision to be valid")
-
 	// write happens, we get a new head revision
 	newerRev, err := ds.ReadWriteTx(ctx, func(ctx context.Context, rwt datastore.ReadWriteTransaction) error {
 		return rwt.LegacyWriteNamespaces(ctx, testNamespace)