Merge #107474 #107489 #107664 #107666 #107673 #107714 #107716

107474: cli/zip: emit SQL table data using TSV by default r=abarganier a=knz

Fixes #107473.
Epic: CRDB-28893

This is a partial revert of 35738d4.

It changes the default value of the `--format` flag back from JSON to TSV.

Release note (backward-incompatible change):
THIS RELEASE NOTE CANCELS THE CORRESPONDING PREVIOUS BACKWARD-INCOMPATIBLE CHANGE. New behavior, compatible with previous versions of CockroachDB: the command `cockroach debug zip` stores data retrieved from SQL tables in the remote cluster using the TSV format by default.

Release note (cli change): The default value of the `--format` parameter to `cockroach debug zip` is `tsv`, like other CLI commands that can extract SQL data.

107489: sql: Delete invalid TestDropColumnAfterMutations test r=rafiss a=rimadeodhar

This test checks the functionality for the following sequence of events:
1. A txn adds a constraint to a column on a table.
2. A separate txn drops the column.

However, this interaction between the two txns has been made explicit by the PR #92289. Since this PR, step (2) will fail if the constraint in step (1) is in the process of being added. As a result, the elaborate set up and sequence of events being tested in TestDropColumnAfterMutations is no longer necessary.

Release note: none
Epic: none
Fixes: #76843

107664: authors: add Angela Dietz to authors r=angeladietz a=angeladietz

Release note: None
Epic: None

107666: server: fix a race in tenant creation r=knz a=lidorcarmel

Previously, scanTenantsForRunnableServices() was not holding the mutex when SELECTing for the existing tenant names, which means that the following may happen:
- scanTenantsForRunnableServices() sees that only the system tenant exists
- createServerEntryLocked() then adds another tenant while holding the mutex
- scanTenantsForRunnableServices() takes the lock and stops the tenant that was just created because only the system tenant should be alive (which is wrong)

This patch changes scanTenantsForRunnableServices() to take the mutex before SELECTing for the existing tenants in order to avoid the race.

Epic: none
Fixes: #107434
Fixes: #107343
Fixes: #107154

Release note: None

107673: opt: remove Metadata.AllUserDefinedFunctions r=mgartner a=mgartner

The metadata method `AllUserDefinedFunctions` has been replaced with a
new function `HasUserDefinedFunctions` which provides a simpler API
without exposing the underlying UDF dependency map. The map is still
available outside of the opt package via the `TestingUDFDeps` method
which is designed for testing use only.

Epic: None

Release note: None


107714: roachtest: add warning to redacted github issue r=mgartner a=mgartner

Epic: None

Release note: None

107716: ui: extend search logic on insights page r=koorosh a=koorosh

This change extends the number of fields where
search is applied (instead of single transaction/
statement execution ID field).
It makes possible to search for any available ID
in Txn or statement insight.

Release note (ui change): search is performed on all ID fields of transaction and statement insights.

Resolves: #107253

Demo:

https://github.com/cockroachdb/cockroach/assets/3106437/7fb56720-3ab2-4be4-9500-457707f6f01d



Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: rimadeodhar <rima@cockroachlabs.com>
Co-authored-by: Angela Dietz <dietz@cockroachlabs.com>
Co-authored-by: Lidor Carmel <lidor@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>

AUTHORS

@@ -73,7 +73,7 @@ Andy Kimball <andyk@cockroachlabs.com> <kimball.andy@gmail.com> <32096062+andy-k
 Andy Woods <andy@cockroachlabs.com> Andrew Woods <andy@cockroachlabs.com>
 Andy Yang <yang@cockroachlabs.com> <ayang@cockroachlabs.com> andyyang890 <42660547+andyyang890@users.noreply.github.com>
 Angela Chang <angelachang27@gmail.com> changangela <angelachang27@gmail.com> <angela@cockroachlabs.com>
-Angela Dietz <angela.dietz@cockroachlabs.com>
+Angela Dietz <angela.dietz@cockroachlabs.com> <dietz@cockroachlabs.com>
 Angela Wen <angelaw@cockroachlabs.com> angelapwen <angelaw@cockroachlabs.com>
 Angela Xu <angelax@cockroachlabs.com> angelazxu <angelazxu@berkeley.edu>
 Anne Zhu <anne.zhu@cockroachlabs.com>

pkg/cli/testdata/zip/specialnames

@@ -1,6 +1,6 @@
 zip
 ----
-[cluster] retrieving SQL data for crdb_internal.cluster_database_privileges... writing output: debug/crdb_internal.cluster_database_privileges.json... done
-[cluster] retrieving SQL data for crdb_internal.table_indexes... writing output: debug/crdb_internal.table_indexes.json... done
-[cluster] retrieving SQL data for system.database_role_settings... writing output: debug/system.database_role_settings.json... done
-[cluster] retrieving SQL data for system.table_statistics... writing output: debug/system.table_statistics.json... done
+[cluster] retrieving SQL data for crdb_internal.cluster_database_privileges... writing output: debug/crdb_internal.cluster_database_privileges.txt... done
+[cluster] retrieving SQL data for crdb_internal.table_indexes... writing output: debug/crdb_internal.table_indexes.txt... done
+[cluster] retrieving SQL data for system.database_role_settings... writing output: debug/system.database_role_settings.txt... done
+[cluster] retrieving SQL data for system.table_statistics... writing output: debug/system.table_statistics.txt... done

pkg/cli/zip.go

@@ -245,7 +245,7 @@ func runDebugZip(cmd *cobra.Command, args []string) (retErr error) {
 
 			if !cmd.Flags().Changed(cliflags.TableDisplayFormat.Name) {
 				// Use a streaming format to avoid accumulating all rows in RAM.
-				sqlExecCtx.TableDisplayFormat = clisqlexec.TableDisplayJSON
+				sqlExecCtx.TableDisplayFormat = clisqlexec.TableDisplayTSV
 			}
 
 			zr.sqlOutputFilenameExtension = computeSQLOutputFilenameExtension(sqlExecCtx.TableDisplayFormat)

pkg/cli/zip_tenant_test.go

@@ -21,12 +21,15 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/datadriven"
 )
 
 // TestTenantZip tests the operation of zip for a tenant server.
 func TestTenantZip(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	skip.UnderRace(t, "test too slow under race")
 
 	tenants := []struct {

pkg/cli/zip_test.go

@@ -373,8 +373,8 @@ func eraseNonDeterministicZipOutput(out string) string {
 	out = re.ReplaceAllString(out, `postgresql://...`)
 	re = regexp.MustCompile(`(?m)SQL address: .*$`)
 	out = re.ReplaceAllString(out, `SQL address: ...`)
-	re = regexp.MustCompile(`(?m)log file:.*$`)
-	out = re.ReplaceAllString(out, `log file: ...`)
+	re = regexp.MustCompile(`(?m)^\[node \d+\] \[log file:.*$` + "\n")
+	out = re.ReplaceAllString(out, ``)
 	re = regexp.MustCompile(`(?m)RPC connection to .*$`)
 	out = re.ReplaceAllString(out, `RPC connection to ...`)
 	re = regexp.MustCompile(`(?m)dial tcp .*$`)

pkg/cmd/roachtest/github.go

@@ -181,7 +181,8 @@ func (g *githubIssues) createPostRequest(
 
 	issueMessage := messagePrefix + message
 	if spec.RedactResults {
-		issueMessage = "The details about this test failure have been omitted; consult the log for more details"
+		issueMessage = "The details about this test failure may contain sensitive information; " +
+			"consult the logs for details. WARNING: DO NOT COPY UNREDACTED ARTIFACTS TO THIS ISSUE."
 	}
 	return issues.PostRequest{
 		MentionOnCreate: mention,

pkg/server/server_controller_orchestration.go

@@ -96,6 +96,9 @@ func (c *serverController) startInitialSecondaryTenantServers(
 func (c *serverController) scanTenantsForRunnableServices(
 	ctx context.Context, ie isql.Executor,
 ) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	// The list of tenants that should have a running server.
 	reqTenants, err := c.getExpectedRunningTenants(ctx, ie)
 	if err != nil {
@@ -108,9 +111,6 @@ func (c *serverController) scanTenantsForRunnableServices(
 		nameLookup[name] = struct{}{}
 	}
 
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
 	// First check if there are any servers that shouldn't be running
 	// right now.
 	for name, srv := range c.mu.servers {

pkg/sql/explain_bundle.go

@@ -483,7 +483,7 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
 	if err := c.PrintCreateEnum(&buf, b.flags.RedactValues); err != nil {
 		b.printError(fmt.Sprintf("-- error getting schema for enums: %v", err), &buf)
 	}
-	if len(mem.Metadata().AllUserDefinedFunctions()) != 0 {
+	if mem.Metadata().HasUserDefinedFunctions() {
 		// Get all relevant user-defined functions.
 		blankLine()
 		if err := c.PrintRelevantCreateUdf(&buf, strings.ToLower(b.stmt), b.flags.RedactValues, &b.errorStrings); err != nil {

pkg/sql/opt/metadata.go

@@ -540,9 +540,9 @@ func (md *Metadata) AllUserDefinedTypes() []*types.T {
 	return md.userDefinedTypesSlice
 }
 
-// AllUserDefinedFunctions returns all user defined functions used in this query.
-func (md *Metadata) AllUserDefinedFunctions() map[cat.StableID]*tree.Overload {
-	return md.udfDeps
+// HasUserDefinedFunctions returns true if the query references a UDF.
+func (md *Metadata) HasUserDefinedFunctions() bool {
+	return len(md.udfDeps) > 0
 }
 
 // AddUserDefinedFunction adds a user-defined function to the metadata for this