Merge #148514 #148914 #149784 #149820

148514: roachprod: gc ibm cloud r=DarrylWong,herkolategan a=golgeek

The roachprod GC job had not been updated since we started running automated tests on IBM Cloud. This patch updates the job to the latest release and makes it aware of IBM Cloud credentials.

Epic: CRDB-21133
Fixes: #143789
Release note: None

148914: sql/ttl: centralize row-level TTL progress updates at coordinator r=spilchen a=spilchen

Previously, each processor in a row-level TTL job directly updated job progress, leading to fan-out issues in large clusters and performance hits during restarts when many spans had no expired rows. Gating logic was previously added to mitigate this.

This change delegates all progress updates to the coordinator. Each processor now sends its progress to the coordinator, which performs the job progress update. The original gating logic remains but is now enforced by the coordinator.

Closes: #135227

Epic: none
Release note: none

149784: cli/debug: strip comments in statement bundle env.sql file r=mgartner a=mgartner

SQL comments in statement bundle `env.sql` files are now ignored with
the `cockroach debug sb recreate` command.

Release note: None


149820: opt: fix sysbench-update-non-index benchmark r=mgartner a=mgartner

The `sysbench-update-non-index` optimizer benchmark would previously
fail with the error `variable sub-expressions are not allowed in...`
because the placeholder string value was not correctly quoted. This has
been fixed.

Release note: None


Co-authored-by: Ludovic Leroux <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

Author: 4 people

pkg/cli/statement_bundle.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -54,8 +55,11 @@ and stats in an unzipped statement bundle directory.
 	Args: cobra.ExactArgs(1),
 }
 
-var placeholderPairs []string
-var explainPrefix string
+var (
+	placeholderPairs []string
+	explainPrefix    string
+	commentPattern   = regexp.MustCompile(`^\s*--`)
+)
 
 func init() {
 	statementBundleRecreateCmd.RunE = clierrorplus.MaybeDecorateError(runBundleRecreate)
@@ -169,8 +173,10 @@ func runBundleRecreate(cmd *cobra.Command, args []string) (resErr error) {
 	return runDemoInternal(cmd, nil /* gen */, func(ctx context.Context, conn clisqlclient.Conn) error {
 		// SET CLUSTER SETTING statements cannot be executed in multi-statement
 		// implicit transaction, so we need to separate them out into their own
-		// implicit transactions.
-		initStmts := strings.Split(string(bundle.env), "SET CLUSTER SETTING")
+		// implicit transactions. Comments are stripped from the env file first.
+		lines := strings.Split(string(bundle.env), "\n")
+		lines = slices.DeleteFunc(lines, commentPattern.MatchString)
+		initStmts := strings.Split(strings.Join(lines, "\n"), "SET CLUSTER SETTING")
 		for i := 1; i < len(initStmts); i++ {
 			initStmts[i] = "SET CLUSTER SETTING " + initStmts[i]
 		}

pkg/jobs/ingeststopped/ingesting_checker.go

@@ -92,7 +92,7 @@ func checkAllNodesForIngestingJob(
 	)
 	sql.FinalizePlan(ctx, planCtx, p)
 
-	res := sql.NewMetadataOnlyMetadataCallbackWriter()
+	res := sql.NewMetadataOnlyMetadataCallbackWriter(func(context.Context, *execinfrapb.ProducerMetadata) error { return nil })
 
 	recv := sql.MakeDistSQLReceiver(
 		ctx,

pkg/jobs/jobspb/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "//pkg/util/iterutil",
         "//pkg/util/tracing/tracingpb",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_gogo_protobuf//jsonpb",
     ],
 )

pkg/jobs/jobspb/jobs.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/iterutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
+	"github.com/cockroachdb/redact"
 )
 
 // JobID is the ID of a job.
@@ -164,3 +165,10 @@ func (b *BackupEncryptionOptions) IsEncrypted() bool {
 	// For dumb reasons, there are two ways to represent no encryption.
 	return !(b == nil || b.Mode == EncryptionMode_None)
 }
+
+var _ redact.SafeFormatter = (*RowLevelTTLProcessorProgress)(nil)
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (r *RowLevelTTLProcessorProgress) SafeFormat(p redact.SafePrinter, _ rune) {
+	p.SafeString(redact.SafeString(r.String()))
+}

pkg/jobs/jobspb/jobs.proto

@@ -1336,14 +1336,19 @@ message RowLevelTTLProcessorProgress {
     (gogoproto.nullable) = false
   ];
 
-  // ProcessorRowCount is the row count of the DistSQL processor.
-  int64 processor_row_count = 3;
+  // DeletedRowCount is the number of rows deleted by this DistSQL processor.
+  int64 deleted_row_count = 3;
 
-  // ProcessorSpanCount is the number of spans of the DistSQL processor;
-  int64 processor_span_count = 4;
+  // TotalSpanCount is the total number of spans assigned to the DistSQL processor.
+  int64 total_span_count = 4;
+
+  // ProcessedSpanCount is the number of spans already processed.
+  int64 processed_span_count = 6;
 
   // ProcessorConcurrency is the number parallel tasks the processor will do at once.
   int64 processor_concurrency = 5;
+
+  // NEXT ID: 7
 }
 
 message SchemaTelemetryDetails {

pkg/roachprod/k8s/roachprod-gc.yaml

@@ -31,7 +31,7 @@ spec:
         spec:
           containers:
             - name: roachprod-gc-cronjob
-              image: gcr.io/cockroach-dev-inf/cockroachlabs/roachprod:13afd784242
+              image: gcr.io/cockroach-dev-inf/cockroachlabs/roachprod:fd18c1bd63d
               args:
                 - gc
                 - --gce-project=cockroach-ephemeral,cockroach-roachstress
@@ -40,6 +40,11 @@ spec:
                 - --aws-account-ids=541263489771,337380398238
                 - --azure-subscription-names=e2e-adhoc,e2e-infra,Microsoft Azure Sponsorship
               env:
+                - name: IBM_APIKEY
+                  valueFrom:
+                    secretKeyRef:
+                      name: roachprod-gc-cronjob-creds
+                      key: ibm_apikey
                 - name: SLACK_TOKEN
                   valueFrom:
                     secretKeyRef:

pkg/sql/distsql_running.go

@@ -18,6 +18,7 @@ import (
 	"unsafe"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/col/coldata"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangecache"
@@ -456,9 +457,13 @@ func (dsp *DistSQLPlanner) setupFlows(
 	if len(statementSQL) > setupFlowRequestStmtMaxLength {
 		statementSQL = statementSQL[:setupFlowRequestStmtMaxLength]
 	}
+	v := execversion.V25_2
+	if dsp.st.Version.IsActive(ctx, clusterversion.V25_4) {
+		v = execversion.V25_4
+	}
 	setupReq := execinfrapb.SetupFlowRequest{
 		LeafTxnInputState: leafInputState,
-		Version:           execversion.V25_2,
+		Version:           v,
 		TraceKV:           recv.tracing.KVTracingEnabled(),
 		CollectStats:      planCtx.collectExecStats,
 		StatementSQL:      statementSQL,
@@ -1163,12 +1168,12 @@ func NewMetadataCallbackWriter(
 // NewMetadataOnlyMetadataCallbackWriter creates a new MetadataCallbackWriter
 // that uses errOnlyResultWriter and only supports receiving
 // execinfrapb.ProducerMetadata.
-func NewMetadataOnlyMetadataCallbackWriter() *MetadataCallbackWriter {
+func NewMetadataOnlyMetadataCallbackWriter(
+	metaFn func(ctx context.Context, meta *execinfrapb.ProducerMetadata) error,
+) *MetadataCallbackWriter {
 	return NewMetadataCallbackWriter(
 		&errOnlyResultWriter{},
-		func(ctx context.Context, meta *execinfrapb.ProducerMetadata) error {
-			return nil
-		},
+		metaFn,
 	)
 }
 

pkg/sql/execversion/version.go

@@ -21,12 +21,16 @@ type V uint32
 // only be used by the flows once the cluster has upgraded to 25.2.
 const V25_2 = V(73)
 
+// V25_4 is the exec version of all binaries of 25.4 cockroach versions. It can
+// only be used by the flows once the cluster has upgraded to 25.4.
+const V25_4 = V(74)
+
 // MinAccepted is the oldest version that the server is compatible with. A
 // server will not accept flows with older versions.
 const MinAccepted = V25_2
 
 // Latest is the latest exec version supported by this binary.
-const Latest = V25_2
+const Latest = V25_4
 
 var contextVersionKey = ctxutil.RegisterFastValueKey()
 

pkg/sql/inspect/inspect_job.go

@@ -164,7 +164,8 @@ func (c *inspectResumer) runInspectPlan(
 ) error {
 	execCfg := jobExecCtx.ExecCfg()
 
-	metadataCallbackWriter := sql.NewMetadataOnlyMetadataCallbackWriter()
+	metadataCallbackWriter := sql.NewMetadataOnlyMetadataCallbackWriter(
+		func(context.Context, *execinfrapb.ProducerMetadata) error { return nil })
 
 	distSQLReceiver := sql.MakeDistSQLReceiver(
 		ctx,

pkg/sql/opt/bench/bench_test.go

@@ -407,7 +407,7 @@ var queries = [...]benchQuery{
 	{
 		name:  "sysbench-update-non-index",
 		query: `UPDATE sbtest SET c=$2 WHERE id=$1`,
-		args:  []interface{}{10, "foo"},
+		args:  []interface{}{10, "'foo'"},
 	},
 
 	// 1. Table with many columns.