Merge #75191 #75210

75191: cli/demo: fix connection string for shared DB server r=otan,knz a=rafiss fixes #74162 This is needed since the workload fixtures are not installed on the shared server, so it only has the default databases. Release note: None 75210: sql: deflake TestTelemetry r=rytaft a=rytaft This commit deflakes TestTelemetry by adding a more precise feature-allowlist. Fixes #75190 Release note: None Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>
cockroachdb · Jan 20, 2022 · 96779a6 · 96779a6
3 parents e8a0b75 + 982e6aa + 4af0f77
commit 96779a6
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 9 deletions.
diff --git 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,19 +1362,25 @@ 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,
 		u: pgurl.New().
 			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
@@ -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"

diff --git a/pkg/sql/testdata/telemetry/planning b/pkg/sql/testdata/telemetry/planning
@@ -180,14 +180,18 @@ 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
 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)
 ----