Merge #118293 #118827 #118850 #118865 #118905 #118914 #118928 #118960 #118970 #118975

118293: roachtest: add multi-region mixed-version test r=srosenberg,herkolategan a=renatolabs

This commit adds a `multi-region/mixed-version` test that creates a larger-sized cluster (4 regions, 10 nodes per region) and runs a constant background TPCC worload (with `tolerate-errors`), along with a short TPCC workload that runs in mixed-version state (without `tolerate-errors`).

With this test, we exercise our ability to perform several cluster upgrades in MR clusters. In the future, we plan to extend this test to include other kinds of randomized testing as well.

Fixes: #114803

Release note: None

**roachtest: change TPCC functions to take a logger instance**
This makes it easier to organize logs for a complex test by allowing
the caller to inject the logger instance that should be used in a
particular call to `runTPCC`. More immediately, we make use of this
change in the recently introduced `multi-region/mixed-version` test,
and pass the step's logger to those functions.

118827: roachpb: stop reading `Lease.DeprecatedStartStasis` r=erikgrinaker a=erikgrinaker

This will allow removing the field in a later version. We still have to populate the field for backwards compatibility with 23.2.

Epic: none
Release note: None

118850: server: enable continuous CPU profiler r=aadityasondhi a=aadityasondhi

This patch enables the CPU profile with a threshold of 75% and a max frequency of 20min.

The motivation for this change is that recently, I have noticed a few cases during investigations where CPU utilization spikes but we lack profiles after the fact. This hinders our ability to dig deeper into the source of high CPU usage. Having profiles can help inform us of future AC integrations that we may need, or other performance improvements we can do elsewhere.

Informs #97699.

Release note (ops change): CRDB will now automatically generate CPU profiles if there is an increase in CPU utilization. This can help investigate possible issues after the fact.

118865: technotes: SQL statistics r=maryliag a=maryliag

Tech notes on SQL Statistics.

Part Of [CRDB-35839](https://cockroachlabs.atlassian.net/browse/CRDB-35839)

Images of how the diagrams generated using mermaid will be rendered:

<img width="814" alt="Screenshot 2024-02-06 at 4 10 05 PM" src="https://github.com/cockroachdb/cockroach/assets/1017486/9dfbd2b1-4454-41f1-8840-dc39d41059e5">

<img width="285" alt="Screenshot 2024-02-06 at 4 42 39 PM" src="https://github.com/cockroachdb/cockroach/assets/1017486/bf570d55-1f5d-454b-b693-fbcf5e082a39">



Release note: None

118905: pgwire: increase timeout and add Error() call r=rafiss a=rickystewart

This test seems to deadlock sporadically under remote execution. I believe the connection is timing out, which due to the way this test was written causes it to deadlock. I'm increasing the timeout to attempt to increase reliability, and separately, we also add a call to `Error()` to mark the test as failed should the connection fail.

Closes #118741.

Epic: None
Release note: None

118914: roachtest: print issue number after test failure r=srosenberg a=renatolabs

This commit updates the GitHub issue poster so that information about
the issue is returned when an issue is created or a comment
added. The roachtest test runner uses this information in the TeamCity
output so that we can easily see the issue corresponding to a test
failure directly in the TC overview page.

Epic: none

Release note: None

118928: backupccl: skip TestDataDriven_multiregion under duress r=rickystewart a=msbutler

Fixes #118567

Release note: none

118960: cli: update docs url for sql shell r=lunevalex a=lunevalex

The SQL shell help function redirects the user to
use-the-built-in-sql-client.html this page no longer exists. Instead the SQL shell should point to cockroach-sql.html.

Epic: None

Release note (cli change): Change the SQL shell help URL to point to cockroach-sql.html.

118970: explain: fix overflow when printing estimated row count r=yuzefovich a=yuzefovich

Previously, when the optimizer estimated a very large row count (which is float64), we would first cast it to uint64 (which would work ok in case the count is large - we'd get `max int64 + 1`), and then we would cast it to int64 which can result in a negative number. This is now fixed by adding a check before casting to int64 to cap the value at max int64.

Epic: None

Release note: None

118975: changefeedccl: skip flaky tests for pulsar r=wenyihu6 a=wenyihu6

This patch skips flaky tests for pulsar sinks. We will add them back when pulsar
is fully supported.

Release note: none
Fixes: #118938, #118937, #118936, #118935

Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Aaditya Sondhi <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Alex Lunev <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Wenyi Hu <[email protected]>

Author: 10 people

docs/tech-notes/observability/sql_stats.md

@@ -512,6 +512,10 @@ func runTestDataDriven(t *testing.T, testFilePathFromWorkspace string) {
 			skip.WithIssue(t, issue)
 			return ""
 
+		case "skip-under-duress":
+			skip.UnderDuress(t)
+			return ""
+
 		case "reset":
 			ds.cleanup(ctx, t)
 			ds = newDatadrivenTestState()

pkg/ccl/backupccl/datadriven_test.go

@@ -1,5 +1,8 @@
 # disabled to run within tenant because multiregion primitives are not supported within tenant
 
+skip-under-duress
+----
+
 new-cluster name=s1 allow-implicit-access disable-tenant localities=us-east-1,us-west-1,eu-central-1
 ----
 

pkg/ccl/backupccl/testdata/backup-restore/multiregion

@@ -881,7 +881,7 @@ func randomSinkTypeWithOptions(options feedTestOptions) string {
 		"pubsub":       1,
 		"sinkless":     2,
 		"cloudstorage": 0,
-		"pulsar":       1,
+		"pulsar":       0,
 	}
 	if options.externalIODir != "" {
 		sinkWeights["cloudstorage"] = 3

pkg/ccl/changefeedccl/helpers_test.go

@@ -276,7 +276,7 @@ func (c *cliState) printCliHelp() {
 	fmt.Fprintf(c.iCtx.stdout, helpMessageFmt,
 		demoHelpStr,
 		docs.URL("sql-statements.html"),
-		docs.URL("use-the-built-in-sql-client.html"),
+		docs.URL("cockroach-sql.html"),
 	)
 	fmt.Fprintln(c.iCtx.stdout)
 }

pkg/cli/clisqlshell/sql.go

@@ -104,7 +104,8 @@ func DefaultIssueFilerFromFormatter(
 			}
 			req.ExtraParams["stress"] = "true"
 		}
-		return issues.Post(ctx, log.Default(), fmter, req, opts)
+		_, err := issues.Post(ctx, log.Default(), fmter, req, opts)
+		return err
 	}
 
 }

pkg/cmd/bazci/githubpost/githubpost.go

@@ -337,7 +337,34 @@ func buildIssueQueries(
 	return existingIssueQuery, relatedIssuesQuery
 }
 
-func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req PostRequest) error {
+type TestFailureType string
+
+const (
+	TestFailureNewIssue     = TestFailureType("new_issue")
+	TestFailureIssueComment = TestFailureType("comment")
+)
+
+// TestFailureIssue encapsulates data about an issue created or
+// changed in order to report a test failure.
+type TestFailureIssue struct {
+	Type TestFailureType
+	ID   int
+}
+
+func (tfi TestFailureIssue) String() string {
+	switch tfi.Type {
+	case TestFailureNewIssue:
+		return fmt.Sprintf("created new GitHub issue #%d", tfi.ID)
+	case TestFailureIssueComment:
+		return fmt.Sprintf("commented on existing GitHub issue #%d", tfi.ID)
+	default:
+		return fmt.Sprintf("[unrecognized test failure type %q, ID=%d]", tfi.Type, tfi.ID)
+	}
+}
+
+func (p *poster) post(
+	origCtx context.Context, formatter IssueFormatter, req PostRequest,
+) (*TestFailureIssue, error) {
 	ctx := &postCtx{Context: origCtx}
 	data := p.templateData(
 		ctx,
@@ -402,6 +429,7 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
 	createLabels := []string{RobotLabel}
 	createLabels = append(createLabels, req.labels()...)
 	createLabels = append(createLabels, releaseLabel(p.Branch))
+	var result TestFailureIssue
 	if foundIssue == nil {
 		issueRequest := github.IssueRequest{
 			Title:     &title,
@@ -411,11 +439,13 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
 		}
 		issue, _, err := p.createIssue(ctx, p.Org, p.Repo, &issueRequest)
 		if err != nil {
-			return errors.Wrapf(err, "failed to create GitHub issue %s",
+			return nil, errors.Wrapf(err, "failed to create GitHub issue %s",
 				github.Stringify(issueRequest))
 		}
 
-		p.l.Printf("created GitHub issue #%d", *issue.Number)
+		result.Type = TestFailureNewIssue
+		result.ID = *issue.Number
+		p.l.Printf("%s", result)
 		if req.ProjectColumnID != 0 {
 			_, _, err := p.createProjectCard(ctx, int64(req.ProjectColumnID), &github.ProjectCardOptions{
 				ContentID:   *issue.ID,
@@ -433,14 +463,16 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
 		comment := github.IssueComment{Body: github.String(body)}
 		if _, _, err := p.createComment(
 			ctx, p.Org, p.Repo, *foundIssue, &comment); err != nil {
-			return errors.Wrapf(err, "failed to update issue #%d with %s",
+			return nil, errors.Wrapf(err, "failed to update issue #%d with %s",
 				*foundIssue, github.Stringify(comment))
 		} else {
-			p.l.Printf("created comment on existing GitHub issue (#%d)", *foundIssue)
+			result.Type = TestFailureIssueComment
+			result.ID = *foundIssue
+			p.l.Printf("%s", result)
 		}
 	}
 
-	return nil
+	return &result, nil
 }
 
 func (p *poster) teamcityURL(tab, fragment string) *url.URL {
@@ -559,9 +591,9 @@ type Logger interface {
 // will be returned.
 func Post(
 	ctx context.Context, l Logger, formatter IssueFormatter, req PostRequest, opts *Options,
-) error {
+) (*TestFailureIssue, error) {
 	if !opts.CanPost() {
-		return errors.Newf("GITHUB_API_TOKEN env variable is not set; cannot post issue")
+		return nil, errors.Newf("GITHUB_API_TOKEN env variable is not set; cannot post issue")
 	}
 
 	client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource(

pkg/cmd/internal/issues/issues.go

@@ -404,15 +404,19 @@ test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTe
 				// Override the default.
 				req.Labels = []string{}
 			}
-			require.NoError(t, p.post(context.Background(), UnitTestFormatter, req))
+			issue, err := p.post(context.Background(), UnitTestFormatter, req)
+			require.NoError(t, err)
+			require.Equal(t, issueNumber, issue.ID)
 
 			switch foundIssue {
 			case foundNoIssue, foundOnlyRelatedIssue:
 				require.True(t, createdIssue)
 				require.False(t, createdComment)
+				require.Equal(t, TestFailureNewIssue, issue.Type)
 			case foundOnlyMatchingIssue, foundMatchingAndRelatedIssue:
 				require.False(t, createdIssue)
 				require.True(t, createdComment)
+				require.Equal(t, TestFailureIssueComment, issue.Type)
 			default:
 				t.Errorf("unhandled: %s", foundIssue)
 			}
@@ -460,7 +464,8 @@ func TestPostEndToEnd(t *testing.T) {
 		HelpCommand: UnitTestHelpCommand(""),
 	}
 
-	require.NoError(t, Post(context.Background(), log.Default(), UnitTestFormatter, req, opts))
+	_, err := Post(context.Background(), log.Default(), UnitTestFormatter, req, opts)
+	require.NoError(t, err)
 }
 
 // setEnv overrides the env variables corresponding to the input map. The

pkg/cmd/internal/issues/issues_test.go

@@ -34,7 +34,7 @@ type githubIssues struct {
 	disable      bool
 	cluster      *clusterImpl
 	vmCreateOpts *vm.CreateOpts
-	issuePoster  func(context.Context, issues.Logger, issues.IssueFormatter, issues.PostRequest, *issues.Options) error
+	issuePoster  func(context.Context, issues.Logger, issues.IssueFormatter, issues.PostRequest, *issues.Options) (*issues.TestFailureIssue, error)
 	teamLoader   func() (team.Map, error)
 }
 
@@ -301,11 +301,13 @@ func (g *githubIssues) createPostRequest(
 	}, nil
 }
 
-func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error {
+func (g *githubIssues) MaybePost(
+	t *testImpl, l *logger.Logger, message string,
+) (*issues.TestFailureIssue, error) {
 	doPost, skipReason := g.shouldPost(t)
 	if !doPost {
 		l.Printf("skipping GitHub issue posting (%s)", skipReason)
-		return nil
+		return nil, nil
 	}
 
 	var metamorphicBuild bool
@@ -319,7 +321,7 @@ func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string)
 	}
 	postRequest, err := g.createPostRequest(t.Name(), t.start, t.end, t.spec, t.failures(), message, metamorphicBuild, t.goCoverEnabled)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	opts := issues.DefaultOptionsFromEnv()
 

pkg/cmd/roachtest/github.go

@@ -750,7 +750,7 @@ func (r *testRunner) runWorker(
 		handleClusterCreationFailure := func(err error) {
 			t.Error(errClusterProvisioningFailed(err))
 
-			if err := github.MaybePost(t, l, t.failureMsg()); err != nil {
+			if _, err := github.MaybePost(t, l, t.failureMsg()); err != nil {
 				shout(ctx, l, stdout, "failed to post issue: %s", err)
 			}
 		}
@@ -1016,6 +1016,17 @@ func (r *testRunner) runTest(
 				}
 				output := fmt.Sprintf("%s\ntest artifacts and logs in: %s", failureMsg, t.ArtifactsDir())
 
+				issue, err := github.MaybePost(t, l, output)
+				if err != nil {
+					shout(ctx, l, stdout, "failed to post issue: %s", err)
+				}
+
+				// If an issue was created (or comment added) on GitHub,
+				// include that information in the output so that it can be
+				// easily inspected on the TeamCity overview page.
+				if issue != nil {
+					output += "\n" + issue.String()
+				}
 				if roachtestflags.TeamCity {
 					// If `##teamcity[testFailed ...]` is not present before `##teamCity[testFinished ...]`,
 					// TeamCity regards the test as successful.
@@ -1024,10 +1035,6 @@ func (r *testRunner) runTest(
 				}
 
 				shout(ctx, l, stdout, "--- FAIL: %s (%s)\n%s", testRunID, durationStr, output)
-
-				if err := github.MaybePost(t, l, output); err != nil {
-					shout(ctx, l, stdout, "failed to post issue: %s", err)
-				}
 			} else {
 				shout(ctx, l, stdout, "--- PASS: %s (%s)", testRunID, durationStr)
 			}