Merge #58275 #58862

58275: roachprod: Support AWS GP3 drives. r=petermattis a=miretskiy Add support for spinning up VMs with GP3 drives. Make it possible to specify multiple, possibly different EBS volumes for each aws instance. Release Notes: None 58862: server,cli: ensure the server identifiers are properly logged in SQL pds r=itsbilal a=knz This is a forward-port of a change (#58708) we've discovered was necessary in v20.2. Release note: None Co-authored-by: Yevgeniy Miretskiy <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
cockroachdb · Jan 15, 2021 · 95e22b5 · 95e22b5
3 parents b4dfbbe + fe89c58 + 3457bf7
commit 95e22b5
Show file tree

Hide file tree

Showing 13 changed files with 271 additions and 52 deletions.
diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go
@@ -6093,6 +6093,8 @@ func TestProtectedTimestampsFailDueToLimits(t *testing.T) {
 
 func TestPaginatedBackupTenant(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	defer jobs.TestingSetAdoptAndCancelIntervals(100*time.Millisecond, 100*time.Millisecond)()
 
 	const numAccounts = 1
@@ -6192,6 +6194,8 @@ func TestPaginatedBackupTenant(t *testing.T) {
 // Ensure that backing up and restoring tenants succeeds.
 func TestBackupRestoreTenant(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	defer jobs.TestingSetAdoptAndCancelIntervals(100*time.Millisecond, 100*time.Millisecond)()
 
 	const numAccounts = 1
@@ -6208,11 +6212,16 @@ func TestBackupRestoreTenant(t *testing.T) {
 	tenant10 := sqlutils.MakeSQLRunner(conn10)
 	tenant10.Exec(t, `CREATE DATABASE foo; CREATE TABLE foo.bar(i int primary key); INSERT INTO foo.bar VALUES (110), (210)`)
 
+	// Prevent a logging assertion that the server ID is initialized multiple times.
+	log.TestingClearServerIdentifiers()
+
 	_, conn11 := serverutils.StartTenant(t, srv, base.TestTenantArgs{TenantID: roachpb.MakeTenantID(11)})
 	defer conn11.Close()
 	tenant11 := sqlutils.MakeSQLRunner(conn11)
 	tenant11.Exec(t, `CREATE DATABASE foo; CREATE TABLE foo.baz(i int primary key); INSERT INTO foo.baz VALUES (111), (211)`)
 
+	log.TestingClearServerIdentifiers()
+
 	_, conn20 := serverutils.StartTenant(t, srv, base.TestTenantArgs{TenantID: roachpb.MakeTenantID(20)})
 	defer conn20.Close()
 	tenant20 := sqlutils.MakeSQLRunner(conn20)
@@ -6268,6 +6277,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 			[][]string{{`10`, `true`, `{"id": "10", "state": "ACTIVE"}`}},
 		)
 
+		log.TestingClearServerIdentifiers()
+
 		ten10Stopper := stop.NewStopper()
 		_, restoreConn10 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{
@@ -6302,6 +6313,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 			[][]string{{`10`, `true`, `{"id": "10", "state": "ACTIVE"}`}},
 		)
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn10 = serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true},
 		)
@@ -6326,6 +6339,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 			[][]string{{`10`, `true`, `{"id": "10", "state": "ACTIVE"}`}},
 		)
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn10 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true},
 		)
@@ -6355,6 +6370,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 			},
 		)
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn10 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true},
 		)
@@ -6364,6 +6381,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 		restoreTenant10.CheckQueryResults(t, `select * from foo.bar`, tenant10.QueryStr(t, `select * from foo.bar`))
 		restoreTenant10.CheckQueryResults(t, `select * from foo.bar2`, tenant10.QueryStr(t, `select * from foo.bar2`))
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn11 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(11), Existing: true},
 		)
@@ -6382,6 +6401,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 
 		restoreDB.Exec(t, `RESTORE TENANT 10 FROM 'nodelocal://1/t10' AS OF SYSTEM TIME `+ts1)
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn10 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true},
 		)
@@ -6400,6 +6421,8 @@ func TestBackupRestoreTenant(t *testing.T) {
 
 		restoreDB.Exec(t, `RESTORE TENANT 20 FROM 'nodelocal://1/t20'`)
 
+		log.TestingClearServerIdentifiers()
+
 		_, restoreConn20 := serverutils.StartTenant(
 			t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(20), Existing: true},
 		)

diff --git a/pkg/ccl/backupccl/create_scheduled_backup_test.go b/pkg/ccl/backupccl/create_scheduled_backup_test.go
@@ -35,6 +35,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	pbtypes "github.com/gogo/protobuf/types"
@@ -178,6 +179,8 @@ func (t userType) String() string {
 // itself with the actual scheduling and the execution of those backups.
 func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 
@@ -435,6 +438,8 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
 
 func TestScheduleBackup(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 
@@ -578,6 +583,8 @@ INSERT INTO t1 values (-1), (10), (-100);
 
 func TestCreateBackupScheduleRequiresAdminRole(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 
@@ -599,6 +606,8 @@ func TestCreateBackupScheduleRequiresAdminRole(t *testing.T) {
 
 func TestCreateBackupScheduleCollectionOverwrite(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 
@@ -619,6 +628,8 @@ func TestCreateBackupScheduleCollectionOverwrite(t *testing.T) {
 
 func TestCreateBackupScheduleInExplicitTxnRollback(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 
@@ -639,6 +650,8 @@ func TestCreateBackupScheduleInExplicitTxnRollback(t *testing.T) {
 // (eventually), even after the cluster has been down for a long period.
 func TestScheduleBackupRecoversFromClusterDown(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	th, cleanup := newTestHelper(t)
 	defer cleanup()
 

diff --git a/pkg/ccl/importccl/import_stmt_test.go b/pkg/ccl/importccl/import_stmt_test.go
@@ -6258,6 +6258,7 @@ func TestDisallowsInvalidFormatOptions(t *testing.T) {
 
 func TestImportInTenant(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
 	baseDir := filepath.Join("testdata")
@@ -6272,6 +6273,9 @@ func TestImportInTenant(t *testing.T) {
 	defer conn10.Close()
 	t10 := sqlutils.MakeSQLRunner(conn10)
 
+	// Prevent a logging assertion that the server ID is initialized multiple times.
+	log.TestingClearServerIdentifiers()
+
 	// Setup a few tenants, each with a different table.
 	_, conn11 := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(11)})
 	defer conn11.Close()

diff --git a/pkg/cli/interactive_tests/test_log_config_msg.tcl b/pkg/cli/interactive_tests/test_log_config_msg.tcl
@@ -7,9 +7,10 @@ source [file join [file dirname $argv0] common.tcl]
 
 start_server $argv
 
-start_test "Check that the cluster ID is reported at the start of the first log file."
+start_test "Check that the cluster and node ID is reported at the start of the first log file."
 spawn tail -n 1000 -F logs/db/logs/cockroach.log
 eexpect "\\\[config\\\] clusterID:"
+eexpect "\\\[config\\\] nodeID:"
 eexpect "node startup completed"
 end_test
 
@@ -22,7 +23,7 @@ system "$argv start-single-node --insecure --pid-file=server_pid --background -s
 # Stop the server, which also flushes and closes the log files.
 stop_server $argv
 
-start_test "Check that the cluster ID is reported at the start of new log files."
+start_test "Check that the cluster and node ID is reported at the start of new log files."
 # Verify that the string "restarted pre-existing node" can be found
 # somewhere. This ensures that if this string ever changes, the test
 # below won't report a false negative.
@@ -32,4 +33,6 @@ system "grep -q 'restarted pre-existing node' logs/db/logs/*.log"
 system "if grep -q 'restarted pre-existing node' logs/db/logs/cockroach.log; then false; fi"
 # Verify that the last log file does contain the cluster ID.
 system "grep -qF '\[config\] clusterID:' logs/db/logs/cockroach.log"
+system "grep -qF '\[config\] nodeID:' logs/db/logs/cockroach.log"
 end_test
+
diff --git a/pkg/cli/mt_start_sql.go b/pkg/cli/mt_start_sql.go
@@ -98,7 +98,7 @@ func runStartSQL(cmd *cobra.Command, args []string) error {
 		tempStorageMaxSizeBytes,
 	)
 
-	sqlServer, addr, httpAddr, instanceID, err := server.StartTenant(
+	sqlServer, addr, httpAddr, err := server.StartTenant(
 		ctx,
 		stopper,
 		clusterName,
@@ -116,11 +116,6 @@ func runStartSQL(cmd *cobra.Command, args []string) error {
 		sqlServer.StartDiagnostics(ctx)
 	}
 
-	// Register the server's identifiers so that log events are
-	// decorated with the server's identity. This helps when gathering
-	// log events from multiple servers into the same log collector.
-	log.SetTenantIDs(serverCfg.SQLConfig.TenantID.String(), int32(instanceID))
-
 	log.Infof(ctx, "SQL server for tenant %s listening at %s, http at %s", serverCfg.SQLConfig.TenantID, addr, httpAddr)
 
 	// TODO(tbg): make the other goodies in `./cockroach start` reusable, such as