Skip to content

Commit

Permalink
Merge #91881
Browse files Browse the repository at this point in the history 
91881: logictest: support mixed binary version tests with upgrades r=ZhouXing19 a=rafiss

fixes #72451

The logictest framework can now use the cockroach-go testserver to test a multi-node cluster in which each node is running a different binary version.

Previously, only mixed logical versions could be tested.

Release note: None

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
  • Loading branch information
@rafiss
craig[bot] and rafiss committed Nov 24, 2022
2 parents 6475cce + acdc9ba commit 4ae94a5
Show file tree
Hide file tree
Showing 22 changed files with 543 additions and 345 deletions.
6 changes: 3 additions & 3 deletions DEPS.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1342,10 +1342,10 @@ def go_deps():
name = "com_github_cockroachdb_cockroach_go_v2",
build_file_proto_mode = "disable_global",
importpath = "github.com/cockroachdb/cockroach-go/v2",
sha256 = "714648a107b3d7e81cc69106fe95d4619592f63f9d6e556cedf83057084dfa32",
strip_prefix = "github.com/cockroachdb/cockroach-go/v2@v2.2.18",
sha256 = "54ce3a52f7971a1f5b69a766a45688d77fd9118f815ac5839a4f2477df498b06",
strip_prefix = "github.com/cockroachdb/cockroach-go/v2@v2.2.19",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.18.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.19.zip",
],
)
go_repository(
Expand Down
2 changes: 1 addition & 1 deletion build/bazelutil/distdir_files.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/apd/v3/com_github_cockroachdb_apd_v3-v3.1.2.zip": "dde4e1e0861ab1276363eb60a6f1ac6b9f70e1a5baea1e7c7d3bd2a0b9cffad5",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/circuitbreaker/com_github_cockroachdb_circuitbreaker-v2.2.2-0.20190114160014-a614b14ccf63+incompatible.zip": "52fdb5ba6a60e9a2f1db42d5b3c4c13cc5bb3947d5ce7f1bba9b0a14de71813a",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cmux/com_github_cockroachdb_cmux-v0.0.0-20170110192607-30d10be49292.zip": "88f6f9cf33eb535658540b46f6222f029398e590a3ff9cc873d7d561ac6debf0",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.18.zip": "714648a107b3d7e81cc69106fe95d4619592f63f9d6e556cedf83057084dfa32",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.19.zip": "54ce3a52f7971a1f5b69a766a45688d77fd9118f815ac5839a4f2477df498b06",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlfmt/com_github_cockroachdb_crlfmt-v0.0.0-20210128092314-b3eff0b87c79.zip": "452219ca74191eedc6f44a5088b5d64e6f75168b438ac9eafe2256b0db8dbcad",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.1.zip": "0f8fe6199b1c7c44cbd4ce9ac0f3f9e1d2f02263f409361e49445fad4a410ab8",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/errors/com_github_cockroachdb_errors-v1.9.0.zip": "ff3814544271799c80da14dadfe408efc4f66e02cbdf17b73e81614ed9f7ae43",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ require (
github.com/cockroachdb/apd/v3 v3.1.2
github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible
github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292
github.com/cockroachdb/cockroach-go/v2 v2.2.18
github.com/cockroachdb/cockroach-go/v2 v2.2.19
github.com/cockroachdb/crlfmt v0.0.0-20210128092314-b3eff0b87c79
github.com/cockroachdb/datadriven v1.0.1
github.com/cockroachdb/errors v1.9.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,8 @@ github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incom
github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible/go.mod h1:v3T8+rm/HmCL0D1BwDcGaHHAQDuFPW7EsnYs2nBRqUo=
github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292 h1:dzj1/xcivGjNPwwifh/dWTczkwcuqsXXFHY1X/TZMtw=
github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292/go.mod h1:qRiX68mZX1lGBkTWyp3CLcenw9I94W2dLeRvMzcn9N4=
github.com/cockroachdb/cockroach-go/v2 v2.2.18 h1:bRNZWqzRSZesVYFjDAl1jgFb1jhIFIEreWIC2kPbcdY=
github.com/cockroachdb/cockroach-go/v2 v2.2.18/go.mod h1:mzlIDDBALQfEjv/7DU12fb2AfQ/MUYTlychcMpWp9QI=
github.com/cockroachdb/cockroach-go/v2 v2.2.19 h1:YIHyz17jZumBeXPuoZKq/0nrITsqDoDD8/KQt3/xiyc=
github.com/cockroachdb/cockroach-go/v2 v2.2.19/go.mod h1:mzlIDDBALQfEjv/7DU12fb2AfQ/MUYTlychcMpWp9QI=
github.com/cockroachdb/crlfmt v0.0.0-20210128092314-b3eff0b87c79 h1:4s0GWs4NXFK4JEeUc0Q1pRbL4oMbqh1DK70qeQ+viOA=
github.com/cockroachdb/crlfmt v0.0.0-20210128092314-b3eff0b87c79/go.mod h1:EOI6rrXIdP+4EXwM8837kmmb6IJesf7k7W6bUu8BDOg=
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
Expand Down
6 changes: 6 additions & 0 deletions pkg/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ ALL_TESTS = [
"//pkg/sql/lexbase:lexbase_test",
"//pkg/sql/logictest/tests/5node-disk:5node-disk_test",
"//pkg/sql/logictest/tests/5node:5node_test",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.1-22.2:cockroach-go-testserver-22_1-22_2_test",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.2-master:cockroach-go-testserver-22_2-master_test",
"//pkg/sql/logictest/tests/fakedist-disk:fakedist-disk_test",
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
Expand Down Expand Up @@ -1542,6 +1544,8 @@ GO_TARGETS = [
"//pkg/sql/logictest/logictestbase:logictestbase",
"//pkg/sql/logictest/tests/5node-disk:5node-disk_test",
"//pkg/sql/logictest/tests/5node:5node_test",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.1-22.2:cockroach-go-testserver-22_1-22_2_test",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.2-master:cockroach-go-testserver-22_2-master_test",
"//pkg/sql/logictest/tests/fakedist-disk:fakedist-disk_test",
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
Expand Down Expand Up @@ -2721,6 +2725,8 @@ GET_X_DATA_TARGETS = [
"//pkg/sql/logictest/logictestbase:get_x_data",
"//pkg/sql/logictest/tests/5node:get_x_data",
"//pkg/sql/logictest/tests/5node-disk:get_x_data",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.1-22.2:get_x_data",
"//pkg/sql/logictest/tests/cockroach-go-testserver-22.2-master:get_x_data",
"//pkg/sql/logictest/tests/fakedist:get_x_data",
"//pkg/sql/logictest/tests/fakedist-disk:get_x_data",
"//pkg/sql/logictest/tests/fakedist-vec-off:get_x_data",
Expand Down
16 changes: 5 additions & 11 deletions pkg/ccl/backupccl/restore_job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuppb"
"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/cloud/cloudpb"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/jobs/joberror"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
Expand Down Expand Up @@ -2741,24 +2740,19 @@ func (r *restoreResumer) restoreSystemUsers(
return err
}

insertUser := `INSERT INTO system.users ("username", "hashedPassword", "isRole") VALUES ($1, $2, $3)`
if r.execCfg.Settings.Version.IsActive(ctx, clusterversion.V22_2AddSystemUserIDColumn) {
insertUser = `INSERT INTO system.users ("username", "hashedPassword", "isRole", "user_id") VALUES ($1, $2, $3, $4)`
}
insertUser := `INSERT INTO system.users ("username", "hashedPassword", "isRole", "user_id") VALUES ($1, $2, $3, $4)`
newUsernames := make(map[string]bool)
args := make([]interface{}, 4)
for _, user := range users {
newUsernames[user[0].String()] = true
args[0] = user[0]
args[1] = user[1]
args[2] = user[2]
if r.execCfg.Settings.Version.IsActive(ctx, clusterversion.V22_2AddSystemUserIDColumn) {
id, err := descidgen.GenerateUniqueRoleID(ctx, r.execCfg.DB, r.execCfg.Codec)
if err != nil {
return err
}
args[3] = id
id, err := descidgen.GenerateUniqueRoleID(ctx, r.execCfg.DB, r.execCfg.Codec)
if err != nil {
return err
}
args[3] = id
if _, err = executor.Exec(ctx, "insert-non-existent-users", txn, insertUser,
args...); err != nil {
return err
Expand Down
28 changes: 9 additions & 19 deletions pkg/ccl/backupccl/system_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,6 @@ func roleOptionsRestoreFunc(
txn *kv.Txn,
systemTableName, tempTableName string,
) error {
if !execCfg.Settings.Version.IsActive(ctx, clusterversion.V22_2AddSystemUserIDColumn) {
return defaultSystemTableRestoreFunc(
ctx, execCfg, txn, systemTableName, tempTableName,
)
}

executor := execCfg.InternalExecutor
hasIDColumnQuery := fmt.Sprintf(
`SELECT EXISTS (SELECT 1 FROM [SHOW COLUMNS FROM %s] WHERE column_name = 'user_id')`, tempTableName)
Expand Down Expand Up @@ -357,20 +351,16 @@ func roleIDSeqRestoreFunc(
txn *kv.Txn,
systemTableName, tempTableName string,
) error {
if execCfg.Settings.Version.IsActive(ctx, clusterversion.V22_2AddSystemUserIDColumn) {
datums, err := execCfg.InternalExecutor.QueryRowEx(
ctx, "role-id-seq-custom-restore", txn,
sessiondata.NodeUserSessionDataOverride,
`SELECT max(user_id) FROM system.users`,
)
if err != nil {
return err
}
max := tree.MustBeDOid(datums[0])
return execCfg.DB.Put(ctx, execCfg.Codec.SequenceKey(keys.RoleIDSequenceID), max.Oid+1)
datums, err := execCfg.InternalExecutor.QueryRowEx(
ctx, "role-id-seq-custom-restore", txn,
sessiondata.NodeUserSessionDataOverride,
`SELECT max(user_id) FROM system.users`,
)
if err != nil {
return err
}
// Nothing to be done since no user ids have been assigned.
return nil
max := tree.MustBeDOid(datums[0])
return execCfg.DB.Put(ctx, execCfg.Codec.SequenceKey(keys.RoleIDSequenceID), max.Oid+1)
}

// systemTableBackupConfiguration is a map from every systemTable present in the
Expand Down
2 changes: 2 additions & 0 deletions pkg/cmd/generate-logictest/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type testFileTemplateConfig struct {
CclLogicTest bool
ExecBuildLogicTest bool
SqliteLogicTest bool
CockroachGoTestserverTest bool
Ccl bool
ForceProductionValues bool
Package, TestRuleName, RelDir string
Expand Down Expand Up @@ -160,6 +161,7 @@ func (t *testdir) dump() error {
tplCfg.Package = strings.ReplaceAll(strings.ReplaceAll(cfg.Name, "-", "_"), ".", "")
tplCfg.RelDir = t.relPathToParent
tplCfg.TestCount = testCount
tplCfg.CockroachGoTestserverTest = cfg.UseCockroachGoTestserver
// The NumCPU calculation is a guess pulled out of thin air to
// allocate the tests which use 3-node clusters 2 vCPUs, and
// the ones which use more a bit more.
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/generate-logictest/templates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ go_test(
args = ["-test.timeout=3595s"],{{ end }}
data = [
"//c-deps:libgeos", # keep{{ if .SqliteLogicTest }}
"@com_github_cockroachdb_sqllogictest//:testfiles", # keep{{ end }}{{ if .CclLogicTest }}
"@com_github_cockroachdb_sqllogictest//:testfiles", # keep{{ end }}{{ if .CockroachGoTestserverTest }}
"//pkg/cmd/cockroach-short", # keep{{ end }}{{ if .CclLogicTest }}
"//pkg/ccl/logictestccl:testdata", # keep{{ end }}{{ if .LogicTest }}
"//pkg/sql/logictest:testdata", # keep{{ end }}{{ if .ExecBuildLogicTest }}
"//pkg/sql/opt/exec/execbuilder:testdata", # keep{{ end }}
Expand Down
21 changes: 6 additions & 15 deletions pkg/sql/create_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,15 @@ func (n *CreateRoleNode) startExec(params runParams) error {
}

// TODO(richardjcai): move hashedPassword column to system.role_options.
stmt := fmt.Sprintf("INSERT INTO %s VALUES ($1, $2, $3)", sessioninit.UsersTableName)
args := append(make([]interface{}, 0, 4), n.roleName, hashedPassword, n.isRole)
if params.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.V22_2AddSystemUserIDColumn) {
stmt = fmt.Sprintf("INSERT INTO %s VALUES ($1, $2, $3, $4)", sessioninit.UsersTableName)
roleID, err := descidgen.GenerateUniqueRoleID(params.ctx, params.ExecCfg().DB, params.ExecCfg().Codec)
if err != nil {
return err
}
args = append(args, roleID)
stmt := fmt.Sprintf("INSERT INTO %s VALUES ($1, $2, $3, $4)", sessioninit.UsersTableName)
roleID, err := descidgen.GenerateUniqueRoleID(params.ctx, params.ExecCfg().DB, params.ExecCfg().Codec)
if err != nil {
return err
}
rowsAffected, err := params.extendedEvalCtx.ExecCfg.InternalExecutor.Exec(
params.ctx,
opName,
params.p.txn,
stmt,
args...,
params.ctx, opName, params.p.txn, stmt,
n.roleName, hashedPassword, n.isRole, roleID,
)

if err != nil {
return err
} else if rowsAffected != 1 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/logictest/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/build/bazel",
"//pkg/cloud/externalconn/providers",
"//pkg/clusterversion",
"//pkg/col/coldata",
Expand Down Expand Up @@ -60,6 +61,7 @@ go_library(
"//pkg/testutils/sqlutils",
"//pkg/upgrade/upgradebase",
"//pkg/util",
"//pkg/util/binfetcher",
"//pkg/util/envutil",
"//pkg/util/log",
"//pkg/util/randutil",
Expand Down
Loading

0 comments on commit 4ae94a5

Please sign in to comment.