From 982e6aa17b7df98151220c4b0599ee2b996c1d2d Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 19 Jan 2022 22:21:35 -0500 Subject: [PATCH 1/2] cli/demo: fix connection string for shared DB server This is needed since the workload fixtures are not installed on the shared server, so it only has the default databases. Release note: None --- pkg/cli/democluster/demo_cluster.go | 26 +++++++++++++++++++------ pkg/cli/interactive_tests/test_demo.tcl | 20 +++++++++++++++++-- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/pkg/cli/democluster/demo_cluster.go b/pkg/cli/democluster/demo_cluster.go index 9002be87bf71..54a3320ba487 100644 --- a/pkg/cli/democluster/demo_cluster.go +++ b/pkg/cli/democluster/demo_cluster.go @@ -523,7 +523,7 @@ func (c *transientCluster) createAndAddNode( } nodeID := roachpb.NodeID(idx + 1) args := c.demoCtx.testServerArgsForTransientCluster( - c.sockForServer(nodeID), nodeID, joinAddr, c.demoDir, + c.sockForServer(nodeID, "" /* databaseNameOverride */), nodeID, joinAddr, c.demoDir, c.sqlFirstPort, c.httpFirstPort, c.stickyEngineRegistry, @@ -944,7 +944,7 @@ func (c *transientCluster) RestartNode(ctx context.Context, nodeID int32) error return errors.Errorf("restarting nodes is not supported in --%s configurations", cliflags.Global.Name) } args := c.demoCtx.testServerArgsForTransientCluster( - c.sockForServer(roachpb.NodeID(nodeID)), + c.sockForServer(roachpb.NodeID(nodeID), "" /* databaseNameOverride */), roachpb.NodeID(nodeID), c.firstServer.ServingRPCAddr(), c.demoDir, c.sqlFirstPort, c.httpFirstPort, c.stickyEngineRegistry) @@ -1137,13 +1137,17 @@ func (c *transientCluster) getNetworkURLForServer( } } sqlAddr := c.servers[serverIdx].ServingSQLAddr() + database := c.defaultDB if isTenant { sqlAddr = c.tenantServers[serverIdx].SQLAddr() } + if !isTenant && c.demoCtx.Multitenant { + database = catalogkeys.DefaultDatabaseName + } host, port, _ := addr.SplitHostPort(sqlAddr, "") u. WithNet(pgurl.NetTCP(host, port)). - WithDatabase(c.defaultDB) + WithDatabase(database) // For a demo cluster we don't use client TLS certs and instead use // password-based authentication with the password pre-filled in the @@ -1358,11 +1362,17 @@ func (c *transientCluster) AcquireDemoLicense(ctx context.Context) (chan error, // sockForServer generates the metadata for a unix socket for the given node. // For example, node 1 gets socket /tmpdemodir/.s.PGSQL.26267, // node 2 gets socket /tmpdemodir/.s.PGSQL.26268, etc. -func (c *transientCluster) sockForServer(nodeID roachpb.NodeID) unixSocketDetails { +func (c *transientCluster) sockForServer( + nodeID roachpb.NodeID, databaseNameOverride string, +) unixSocketDetails { if !c.useSockets { return unixSocketDetails{} } port := strconv.Itoa(c.sqlFirstPort + int(nodeID) - 1) + databaseName := c.defaultDB + if databaseNameOverride != "" { + databaseName = databaseNameOverride + } return unixSocketDetails{ socketDir: c.demoDir, port: port, @@ -1370,7 +1380,7 @@ func (c *transientCluster) sockForServer(nodeID roachpb.NodeID) unixSocketDetail WithNet(pgurl.NetUnix(c.demoDir, port)). WithUsername(c.adminUser.Normalized()). WithAuthn(pgurl.AuthnPassword(true, c.adminPassword)). - WithDatabase(c.defaultDB), + WithDatabase(databaseName), } } @@ -1450,7 +1460,11 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne bool) { } // Print unix socket if defined. if c.useSockets { - sock := c.sockForServer(nodeID) + databaseNameOverride := "" + if c.demoCtx.Multitenant { + databaseNameOverride = catalogkeys.DefaultDatabaseName + } + sock := c.sockForServer(nodeID, databaseNameOverride) fmt.Fprintln(w, " (sql/unix)", sock) } fmt.Fprintln(w) diff --git a/pkg/cli/interactive_tests/test_demo.tcl b/pkg/cli/interactive_tests/test_demo.tcl index 773d55c23a91..7319569bafc7 100644 --- a/pkg/cli/interactive_tests/test_demo.tcl +++ b/pkg/cli/interactive_tests/test_demo.tcl @@ -8,9 +8,25 @@ spawn $argv demo --insecure=true eexpect "Welcome" # Warn the user that they won't get persistence. eexpect "your changes to data stored in the demo session will not be saved!" -# Inform the necessary URL. + +# Verify the URLs for both shared and tenant server. +eexpect "system tenant" eexpect "(webui)" -eexpect "http:" +eexpect "http://" +eexpect ":8081" +eexpect "(sql)" +eexpect "root" +eexpect ":26258/defaultdb" +eexpect "sslmode=disable" +eexpect "(sql/unix)" +eexpect "root:unused@/defaultdb" +eexpect "=26258" +eexpect "tenant 1" +eexpect "(sql)" +eexpect "root" +eexpect ":26257/movr" +eexpect "sslmode=disable" + # Ensure same messages as cockroach sql. eexpect "Server version" eexpect "Cluster ID" From 4af0f77d956f5f5fcf51a9bf0e19c95c1d2ee1cb Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Thu, 20 Jan 2022 08:09:43 -0600 Subject: [PATCH 2/2] sql: deflake TestTelemetry This commit deflakes TestTelemetry by adding a more precise feature-allowlist. Fixes #75190 Release note: None --- pkg/sql/testdata/telemetry/planning | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/sql/testdata/telemetry/planning b/pkg/sql/testdata/telemetry/planning index 33152e22cc66..13a5d2f61d34 100644 --- a/pkg/sql/testdata/telemetry/planning +++ b/pkg/sql/testdata/telemetry/planning @@ -180,7 +180,7 @@ SELECT * FROM (VALUES (1), (2)) AS a(x) JOIN LATERAL (SELECT a.x+1 AS x) AS b ON sql.plan.lateral-join feature-allowlist -sql.plan.subquery.* +sql.plan.subquery ---- feature-usage @@ -188,6 +188,10 @@ SELECT 1 = (SELECT a FROM x LIMIT 1) ---- sql.plan.subquery +feature-allowlist +sql.plan.subquery.correlated +---- + feature-usage SELECT x FROM (VALUES (1)) AS b(x) WHERE EXISTS(SELECT * FROM (VALUES (1)) AS a(x) WHERE a.x = b.x) ----