Merge #151076 #151185 #151201

151076: server: skip TestCheckRestartSafe under duress r=dhartunian a=dhartunian

These tests do not need to be run under race or deadlock since they involve a lot of moving parts and a full server.

Resolves: #150811
Resolves: #150810
Resolves: #150365

Release note: None

151185: cli: explicitly set disallow_full_table_scans for debug.zips r=ZhouXing19 a=michae2

Fixes: #151183

Release note (bug fix): Fix a bug where debug.zips collected from clusters with disallow_full_table_scans set would have missing system table data.

151201: clusterversion: move WriteInitialTruncStateBeforeSplitApplication to 25.4 r=arulajmani a=arulajmani

This cluster version was intended for v25.3, but [we raced with branch cut](#149494 (comment)) and this didn't make it to the release branch. The least risky thing to do is to move this to 25.4 instead of a late stage backport, so let's do that.

Epic: none

Release note: None

Co-authored-by: David Hartunian <davidh@cockroachlabs.com>
Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>
Co-authored-by: Arul Ajmani <arulajmani@gmail.com>

Author: 4 people

docs/generated/settings/settings-for-tenants.txt

@@ -418,4 +418,4 @@ trace.zipkin.collector	string		the address of a Zipkin instance to receive trace
 ui.database_locality_metadata.enabled	boolean	true	if enabled shows extended locality data about databases and tables in DB Console which can be expensive to compute	application
 ui.default_timezone	string		the default timezone used to format timestamps in the ui	application
 ui.display_timezone	enumeration	etc/utc	the timezone used to format timestamps in the ui. This setting is deprecatedand will be removed in a future version. Use the 'ui.default_timezone' setting instead. 'ui.default_timezone' takes precedence over this setting. [etc/utc = 0, america/new_york = 1]	application
-version	version	1000025.3-upgrading-to-1000025.4-step-002	set the active cluster version in the format '<major>.<minor>'	application
+version	version	1000025.3-upgrading-to-1000025.4-step-004	set the active cluster version in the format '<major>.<minor>'	application

docs/generated/settings/settings.html

@@ -376,6 +376,6 @@
 <tr><td><div id="setting-ui-database-locality-metadata-enabled" class="anchored"><code>ui.database_locality_metadata.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if enabled shows extended locality data about databases and tables in DB Console which can be expensive to compute</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-default-timezone" class="anchored"><code>ui.default_timezone</code></div></td><td>string</td><td><code></code></td><td>the default timezone used to format timestamps in the ui</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-display-timezone" class="anchored"><code>ui.display_timezone</code></div></td><td>enumeration</td><td><code>etc/utc</code></td><td>the timezone used to format timestamps in the ui. This setting is deprecatedand will be removed in a future version. Use the &#39;ui.default_timezone&#39; setting instead. &#39;ui.default_timezone&#39; takes precedence over this setting. [etc/utc = 0, america/new_york = 1]</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000025.3-upgrading-to-1000025.4-step-002</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000025.3-upgrading-to-1000025.4-step-004</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/cli/testdata/zip/testzip_disallow_full_scans

@@ -0,0 +1,45 @@
+zip
+----
+debug zip --concurrency=1 --cpu-profile-duration=0 /dev/null
+[cluster] discovering virtual clusters... done
+[cluster] creating output file /dev/null... done
+[cluster] establishing RPC connection to ...
+[cluster] using SQL address: ...
+[cluster] requesting data for debug/events... received response... writing JSON output: debug/events.json... done
+[cluster] requesting data for debug/rangelog... received response... writing JSON output: debug/rangelog.json... done
+[cluster] requesting data for debug/settings... received response... writing JSON output: debug/settings.json... done
+[cluster] requesting data for debug/reports/problemranges... received response... writing JSON output: debug/reports/problemranges.json... done
+<dumping SQL tables>
+[cluster] requesting nodes... received response... writing JSON output: debug/nodes.json... done
+[cluster] requesting liveness... received response... writing JSON output: debug/liveness.json... done
+[cluster] requesting tenant ranges... received response...
+[cluster] requesting tenant ranges: last request failed: rpc error: ...
+[cluster] requesting tenant ranges: creating error output: debug/tenant_ranges.err.txt... done
+[cluster] collecting the inflight traces for jobs... received response... done
+[node 1] node status... writing JSON output: debug/nodes/1/status.json... done
+[node 1] using SQL connection URL: postgresql://...
+<dumping SQL tables>
+[node 1] requesting data for debug/nodes/1/details... received response... writing JSON output: debug/nodes/1/details.json... done
+[node 1] requesting data for debug/nodes/1/gossip... received response... writing JSON output: debug/nodes/1/gossip.json... done
+[node 1] requesting stacks... received response... writing binary output: debug/nodes/1/stacks.txt... done
+[node 1] requesting stacks with labels... received response... writing binary output: debug/nodes/1/stacks_with_labels.txt... done
+[node 1] requesting heap profile... received response... writing binary output: debug/nodes/1/heap.pprof... done
+[node 1] requesting engine stats... received response... writing binary output: debug/nodes/1/lsm.txt... done
+[node 1] requesting heap profile list... received response... done
+[node ?] ? heap profiles found
+[node 1] requesting goroutine dump list... received response... done
+[node ?] ? goroutine dumps found
+[node 1] requesting cpu profile list... received response... done
+[node ?] ? cpu profiles found
+[node 1] requesting execution trace list... received response... done
+[node ?] ? execution traces found
+[node 1] requesting log files list... received response... done
+[node ?] ? log files found
+[node 1] requesting ranges... received response... done
+[node 1] writing ranges... writing JSON output: debug/nodes/1/ranges.json... done
+[cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
+[cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
+[cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] capture debug zip flags... writing binary output: debug/debug_zip_command_flags.txt... done
+[cluster] The generated file /dev/null is corrupt. Please retry debug zip generation. error: zip: not a valid zip file
+ERROR: zip: not a valid zip file

pkg/cli/zip.go

@@ -594,6 +594,12 @@ func (zc *debugZipContext) dumpTableDataForZip(
 				return err
 			}
 
+			// Many of the table data queries use full scans, so allow them.
+			err = conn.Exec(ctx, `SET disallow_full_table_scans = off`)
+			if err != nil {
+				return err
+			}
+
 			w, err := zc.z.createLocked(name, time.Time{})
 			if err != nil {
 				return err

pkg/cli/zip_test.go

@@ -807,6 +807,45 @@ func TestPartialZip(t *testing.T) {
 		})
 }
 
+// TestZipDisallowFullScans tests that we still can dump full SQL tables for
+// debug.zips when the cluster has disallow_full_table_scans enabled.
+func TestZipDisallowFullScans(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	skip.UnderShort(t)
+	skip.UnderRace(t)
+
+	dir, cleanupFn := testutils.TempDir(t)
+	defer cleanupFn()
+
+	c := NewCLITest(TestCLIParams{
+		StoreSpecs: []base.StoreSpec{{
+			Path: dir,
+		}},
+	})
+	defer c.Cleanup()
+
+	c.RunWithArgs([]string{"sql", "-e", `
+SET CLUSTER SETTING sql.defaults.disallow_full_table_scans.enabled = on;
+SET CLUSTER SETTING sql.defaults.large_full_scan_rows = 1;
+`})
+
+	out, err := c.RunWithCapture("debug zip --concurrency=1 --cpu-profile-duration=0 " + os.DevNull)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Strip any non-deterministic messages.
+	out = eraseNonDeterministicZipOutput(out)
+
+	// We use datadriven simply to read the golden output file; we don't actually
+	// run any commands. Using datadriven allows TESTFLAGS=-rewrite.
+	datadriven.RunTest(t, datapathutils.TestDataPath(t, "zip", "testzip_disallow_full_scans"), func(t *testing.T, td *datadriven.TestData) string {
+		return out
+	})
+}
+
 // This checks that SQL retry errors are properly handled.
 func TestZipRetries(t *testing.T) {
 	defer leaktest.AfterTest(t)()

pkg/clusterversion/cockroach_versions.go

@@ -199,17 +199,17 @@ const (
 
 	V25_3_AddHotRangeLoggerJob
 
-	// V25_3_WriteInitialTruncStateBeforeSplitApplication is the version above
-	// which we write the initial truncated state before applying a split. By
-	// extension, we no longer need to replicate the truncated state when
-	// constructing the split write batch.
-	V25_3_WriteInitialTruncStateBeforeSplitApplication
-
 	// V25_3 is CockroachDB v25.3. It's used for all v25.3.x patch releases.
 	V25_3
 
 	V25_4_Start
 
+	// V25_4_WriteInitialTruncStateBeforeSplitApplication is the version above
+	// which we write the initial truncated state before applying a split. By
+	// extension, we no longer need to replicate the truncated state when
+	// constructing the split write batch.
+	V25_4_WriteInitialTruncStateBeforeSplitApplication
+
 	// *************************************************
 	// Step (1) Add new versions above this comment.
 	// Do not add new versions to a patch release.
@@ -257,12 +257,13 @@ var versionTable = [numKeys]roachpb.Version{
 
 	V25_3_AddHotRangeLoggerJob: {Major: 25, Minor: 2, Internal: 8},
 
-	V25_3_WriteInitialTruncStateBeforeSplitApplication: {Major: 25, Minor: 2, Internal: 10},
-
 	V25_3: {Major: 25, Minor: 3, Internal: 0},
 
 	// v25.4 versions. Internal versions must be even.
 	V25_4_Start: {Major: 25, Minor: 3, Internal: 2},
+
+	V25_4_WriteInitialTruncStateBeforeSplitApplication: {Major: 25, Minor: 3, Internal: 4},
+
 	// *************************************************
 	// Step (2): Add new versions above this comment.
 	// Do not add new versions to a patch release.

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

@@ -1482,11 +1482,11 @@ func splitTriggerHelper(
 			return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to write initial Replica state")
 		}
 		// TODO(arulajmani): This can be removed once all nodes are past the
-		// V25_3_WriteInitialTruncStateBeforeSplitApplication cluster version.
+		// V25_4_WriteInitialTruncStateBeforeSplitApplication cluster version.
 		// At that point, we'll no longer need to replicate the truncated state
 		// as all replicas will be responsible for writing it locally before
 		// applying the split.
-		if !rec.ClusterSettings().Version.IsActive(ctx, clusterversion.V25_3_WriteInitialTruncStateBeforeSplitApplication) {
+		if !rec.ClusterSettings().Version.IsActive(ctx, clusterversion.V25_4_WriteInitialTruncStateBeforeSplitApplication) {
 			if err := stateloader.WriteInitialTruncState(ctx, batch, split.RightDesc.RangeID); err != nil {
 				return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to write initial Replica state")
 			}

pkg/server/api_v2_test.go

@@ -338,7 +338,7 @@ func TestCheckRestartSafe_Criticality(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 150810)
+	skip.UnderDuress(t)
 
 	ctx := context.Background()
 
@@ -436,7 +436,7 @@ func TestCheckRestartSafe_AllowMinimumQuorum_Pass(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 150365)
+	skip.UnderDuress(t)
 
 	ctx := context.Background()
 	var err error
@@ -477,7 +477,7 @@ func TestCheckRestartSafe_AllowMinimumQuorum_Fail(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 149534)
+	skip.UnderDuress(t)
 
 	ctx := context.Background()
 	var err error
@@ -512,7 +512,7 @@ func TestCheckRestartSafe_Integration(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 150811)
+	skip.UnderDuress(t)
 
 	ctx := context.Background()
 	var err error