Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
139482: awsdms: Update replication instance r=Jeremyyang920 a=Jeremyyang920

Previously, we were using the dms.c4.large instance class. However that was recently silently deprecated and caused the test failure.

This commit upgrades the instances to the new c5 classes.

Fixes: #139394
Release note: None

139651: pgx: Continue migration of tests to v5 r=spilchen a=spilchen

This commit builds upon the pgx migration initiated in #139611, focusing on tests that could not be converted with a sed script. While the changes remain largely mechanical, several adjustments were required to ensure test compatibility with pgx v5:
- Additional packages were updated alongside the migration to `github.com/jackc/pgx/v5`.
- Error checks that relied on specific text were adjusted to account for additional context introduced in v5 error wrapping. Returning `pgconn.PgError` directly resolved these issues.
- Use the updated method to force use of the simple query protocol.
- The native support in v5 for scanning numerics directly into strings eliminated the need for previous conversion methods.

Epic: none
Release note: none

Informs #137595

139750: logictest: skip `alter_table` under race r=yuzefovich a=yuzefovich

We just saw a test run where `alter_table` took more than an hour under race. None of the metamorphic variables seem likely to be the cause of the slowdown, so probably just the sheer number of schema changes in this file are to blame.

Fixes: #139739.

Release note: None

Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
4 people committed Jan 24, 2025
4 parents 33c93cc + d427a9c + 8a322a2 + 8fe6d92 commit 6ff1a78
Show file tree
Hide file tree
Showing 20 changed files with 62 additions and 48 deletions.
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,6 @@ github.com/jackc/pgx/v5 v5.4.2/go.mod h1:q6iHT8uDNXWiFNOlRqJzBTaSH3+2xCXkokxHZC5
github.com/jackc/puddle v0.0.0-20190413234325-e4ced69a3a2b/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v0.0.0-20190608224051-11cab39313c9/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v1.1.3/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v1.3.0 h1:eHK/5clGOatcjX3oWGBO/MpxpbHzSwud5EWTSCI+MX0=
github.com/jackc/puddle v1.3.0/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle/v2 v2.2.0 h1:RdcDk92EJBuBS55nQMMYFXTxwstHug4jkhT5pq8VxPk=
github.com/jackc/puddle/v2 v2.2.0/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
Expand Down
2 changes: 1 addition & 1 deletion pkg/backup/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ go_test(
"@com_github_cockroachdb_pebble//vfs",
"@com_github_cockroachdb_redact//:redact",
"@com_github_gogo_protobuf//types",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_kr_pretty//:pretty",
"@com_github_lib_pq//:pq",
"@com_github_robfig_cron_v3//:cron",
Expand Down
8 changes: 6 additions & 2 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/oserror"
"github.com/cockroachdb/redact"
pgx "github.com/jackc/pgx/v4"
pgx "github.com/jackc/pgx/v5"
"github.com/kr/pretty"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -7393,6 +7393,8 @@ func TestClientDisconnect(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

skip.UnderRace(t, "may cause connection delays, leading to a context deadline exceeded error")

const restoreDB = "restoredb"

testCases := []struct {
Expand Down Expand Up @@ -7485,7 +7487,9 @@ func TestClientDisconnect(t *testing.T) {
assert.NoError(t, err)
defer func() { assert.NoError(t, db.Close(ctx)) }()
_, err = db.Exec(ctxToCancel, command)
assert.Equal(t, context.Canceled, errors.Unwrap(err))
// Check the root cause of the error, as pgx v5 may wrap additional
// errors around a context-canceled error.
assert.Equal(t, context.Canceled, errors.Cause(err))
}(testCase.jobCommand)

// Wait for the job to start.
Expand Down
3 changes: 2 additions & 1 deletion pkg/ccl/sqlproxyccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ go_test(
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgproto3_v2//:pgproto3",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_pires_go_proxyproto//:go-proxyproto",
"@com_github_pires_go_proxyproto//tlvparse",
"@com_github_stretchr_testify//assert",
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/sqlproxyccl/proxy_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/jackc/pgconn"
pgproto3 "github.com/jackc/pgproto3/v2"
pgx "github.com/jackc/pgx/v4"
pgx "github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
proxyproto "github.com/pires/go-proxyproto"
"github.com/pires/go-proxyproto/tlvparse"
"github.com/stretchr/testify/assert"
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/testccl/authccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ go_test(
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//stdstrings",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_stretchr_testify//require",
],
)
12 changes: 10 additions & 2 deletions pkg/ccl/testccl/authccl/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ import (
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/stdstrings"
"github.com/jackc/pgconn"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -406,6 +406,14 @@ func authCCLRunTest(t *testing.T, insecure bool) {
defer func() { _ = dbSQL.Close(ctx) }()
}
if err != nil {
// If the error is a PgError, return that directly instead of the
// wrapped error. The wrapped error includes additional contextual
// information that complicates checking for the expected error
// string in tests.
pgErr := new(pgconn.PgError)
if errors.As(err, &pgErr) {
return "", pgErr
}
return "", err
}
row := dbSQL.QueryRow(ctx, "SELECT current_catalog")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/awsdms.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ func setupDMSReplicationInstance(
createReplOut, err := dmsCli.CreateReplicationInstance(
ctx,
&dms.CreateReplicationInstanceInput{
ReplicationInstanceClass: proto.String("dms.c4.large"),
ReplicationInstanceClass: proto.String("dms.c5.large"),
ReplicationInstanceIdentifier: proto.String(awsdmsRoachtestDMSReplicationInstanceName(t.BuildVersion())),
},
)
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -952,10 +952,9 @@ go_test(
"@com_github_cockroachdb_redact//:redact",
"@com_github_gogo_protobuf//proto",
"@com_github_gogo_protobuf//types",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgtype//:pgtype",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_klauspost_compress//zip",
"@com_github_lib_pq//:pq",
"@com_github_lib_pq//oid",
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/conn_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
"github.com/cockroachdb/redact"
"github.com/jackc/pgconn"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/lib/pq"
"github.com/pmezard/go-difflib/difflib"
"github.com/stretchr/testify/require"
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# We've seen this file take more than an hour under race.
skip under race

statement ok
CREATE TABLE other (b INT PRIMARY KEY)

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/pgrepl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ go_test(
"//pkg/util/log",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_jackc_pgx_v5//pgproto3",
Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/pgrepl/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/datadriven"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -70,7 +70,7 @@ func TestDataDriven(t *testing.T) {

switch d.Cmd {
case "simple_query":
rows, err := conn.Query(ctx, d.Input, pgx.QuerySimpleProtocol(true))
rows, err := conn.Query(ctx, d.Input, pgx.QueryExecModeSimpleProtocol)
if expectError {
require.Error(t, err)
return err.Error()
Expand All @@ -81,7 +81,7 @@ func TestDataDriven(t *testing.T) {
return out
case "identify_system":
// IDENTIFY_SYSTEM needs some redaction to be deterministic.
rows, err := conn.Query(ctx, "IDENTIFY_SYSTEM", pgx.QuerySimpleProtocol(true))
rows, err := conn.Query(ctx, "IDENTIFY_SYSTEM", pgx.QueryExecModeSimpleProtocol)
require.NoError(t, err)
var sb strings.Builder
for rows.Next() {
Expand All @@ -91,13 +91,13 @@ func TestDataDriven(t *testing.T) {
if i > 0 {
sb.WriteRune('\n')
}
switch string(rows.FieldDescriptions()[i].Name) {
switch rows.FieldDescriptions()[i].Name {
case "systemid":
val = "some_cluster_id"
case "xlogpos":
val = "some_lsn"
}
sb.Write(rows.FieldDescriptions()[i].Name)
sb.WriteString(rows.FieldDescriptions()[i].Name)
sb.WriteString(": ")
sb.WriteString(fmt.Sprintf("%v", val))
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/pgwire/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ go_test(
"@com_github_jackc_pgproto3_v2//:pgproto3",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_lib_pq//:pq",
"@com_github_lib_pq//oid",
"@com_github_stretchr_testify//require",
Expand Down
12 changes: 10 additions & 2 deletions pkg/sql/pgwire/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/stdstrings"
"github.com/cockroachdb/redact"
"github.com/jackc/pgconn"
pgx "github.com/jackc/pgx/v4"
pgx "github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -586,6 +586,14 @@ func hbaRunTest(t *testing.T, insecure bool) {
defer func() { _ = dbSQL.Close(ctx) }()
}
if err != nil {
// If the error is a PgError, return that directly instead of the
// wrapped error. The wrapped error includes additional contextual
// information that complicates checking for the expected error
// string in tests.
pgErr := new(pgconn.PgError)
if errors.As(err, &pgErr) {
return "", pgErr
}
return "", err
}
row := dbSQL.QueryRow(ctx, "SELECT current_catalog")
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/session_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/datadriven"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -54,7 +54,7 @@ func TestSessionMigration(t *testing.T) {

config, err := pgx.ParseConfig(pgURL.String())
require.NoError(t, err)
config.PreferSimpleProtocol = true
config.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol
conn, err := pgx.ConnectConfig(ctx, config)
require.NoError(t, err)

Expand Down Expand Up @@ -83,7 +83,7 @@ func TestSessionMigration(t *testing.T) {

config, err := pgx.ParseConfig(pgURL.String())
require.NoError(t, err)
config.PreferSimpleProtocol = true
config.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol
conn, err := pgx.ConnectConfig(ctx, config)
require.NoError(t, err)

Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,13 @@ go_test(
"//pkg/util/tracing/tracingpb",
"//pkg/util/uuid",
"@com_github_cockroachdb_cockroach_go_v2//crdb",
"@com_github_cockroachdb_cockroach_go_v2//crdb/crdbpgx",
"@com_github_cockroachdb_cockroach_go_v2//crdb/crdbpgxv5",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_google_btree//:btree",
"@com_github_google_pprof//profile",
"@com_github_jackc_pgconn//:pgconn",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_jackc_pgx_v5//pgconn",
"@com_github_kr_pretty//:pretty",
"@com_github_lib_pq//:pq",
"@com_github_spf13_pflag//:pflag",
Expand Down
25 changes: 9 additions & 16 deletions pkg/sql/tests/show_commit_timestamp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import (
"context"
gosql "database/sql"
"fmt"
"math/big"
"testing"

"github.com/cockroachdb/cockroach-go/v2/crdb"
"github.com/cockroachdb/cockroach-go/v2/crdb/crdbpgx"
crdbpgx "github.com/cockroachdb/cockroach-go/v2/crdb/crdbpgxv5"
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/testutils"
Expand All @@ -22,8 +21,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
"github.com/jackc/pgconn"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -76,7 +75,7 @@ CREATE TABLE foo (i INT PRIMARY KEY)`)
defer cleanup()
conf, err := pgx.ParseConfig(pgURL.String())
require.NoError(t, err)
conf.PreferSimpleProtocol = simple
conf.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol
conn, err := pgx.ConnectConfig(ctx, conf)
require.NoError(t, err)
defer func() { require.NoError(t, conn.Close(ctx)) }()
Expand Down Expand Up @@ -111,11 +110,9 @@ CREATE TABLE foo (i INT PRIMARY KEY)`)
_, err = res.Exec()
require.NoError(t, err)
} else {
// Support for scanning numerics into strings was not added until
// a later version of pgx than was in use at the time of writing.
var r big.Rat
var r string
require.NoError(t, res.QueryRow().Scan(&r))
commitTimestamps = append(commitTimestamps, r.FloatString(10))
commitTimestamps = append(commitTimestamps, r)
}
}
require.NoError(t, res.Close())
Expand All @@ -128,7 +125,7 @@ CREATE TABLE foo (i INT PRIMARY KEY)`)
defer cleanup()
conf, err := pgx.ParseConfig(pgURL.String())
require.NoError(t, err)
conf.PreferSimpleProtocol = simple
conf.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol
conn, err := pgx.ConnectConfig(ctx, conf)
require.NoError(t, err)
defer func() { require.NoError(t, conn.Close(ctx)) }()
Expand All @@ -150,9 +147,7 @@ CREATE TABLE foo (i INT PRIMARY KEY)`)
}
var ts string
{
var tsRat big.Rat
require.NoError(t, conn.QueryRow(ctx, showCommitTimestamp).Scan(&tsRat))
ts = tsRat.FloatString(10)
require.NoError(t, conn.QueryRow(ctx, showCommitTimestamp).Scan(&ts))
}
checkResults(t, []string{ts}, 0)
var txTs string
Expand All @@ -163,11 +158,9 @@ CREATE TABLE foo (i INT PRIMARY KEY)`)
if _, err = tx.Exec(ctx, "insert into foo values (3)"); err != nil {
return err
}
var tsRat big.Rat
if err = tx.QueryRow(ctx, showCommitTimestamp).Scan(&tsRat); err != nil {
if err = tx.QueryRow(ctx, showCommitTimestamp).Scan(&txTs); err != nil {
return err
}
txTs = tsRat.FloatString(10)
return nil
}))

Expand Down
2 changes: 1 addition & 1 deletion pkg/testutils/sqlutils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ go_library(
"//pkg/util/timeutil",
"@com_github_cockroachdb_cockroach_go_v2//crdb",
"@com_github_cockroachdb_errors//:errors",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v5//:pgx",
"@com_github_lib_pq//:pq",
"@com_github_stretchr_testify//require",
],
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutils/sqlutils/rows.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"strings"
"unicode/utf8"

"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v5"
)

// RowsToDataDrivenOutput converts a gosql.Rows object into an appropriate
Expand Down

0 comments on commit 6ff1a78

Please sign in to comment.