Merge #148448 #148507 #148512 #148515

148448: roachtest: guarantee deleting user-table sst in OR recovery r=msbutler a=kev-cao

As illuminated in #148408, deleting an SST that backs a temporary system table before the download phase completes may still result in a successful download phase. Our current OR recovery roachtest deletes SSTs at random, so it is possible for the test to flake because of this. This updates the test to always delete the last SST in `SHOW BACKUP`, which should always cover a user table instead.

Epic: CRDB-51394

Release note: None

148507: roachtest: deflake gopg r=fqazi a=fqazi

Release note: None
Fixes: #147750

148512: sql/schemachangerccl: deflake multi-region backup tests r=fqazi a=fqazi

Increase the CPU cost per test so that fewer multi-region schema changer backup / restore tests run concurrently. This should reduce some flakes.

Fixes: #148192

Release note: None

148515: storage: set format major verison for v25.3 r=RaduBerinde a=jbowens

Set the storage engine's format major version for cluster version v25.3.

Epic: none
Release note: None

Co-authored-by: Kevin Cao <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

Author: 4 people

pkg/ccl/schemachangerccl/sctestbackupccl/BUILD.bazel

@@ -15,7 +15,7 @@ go_test(
     ],
     exec_properties = {"test.Pool": "heavy"},
     shard_count = 48,
-    tags = ["cpu:3"],
+    tags = ["cpu:4"],
     deps = [
         "//pkg/ccl",
         "//pkg/ccl/schemachangerccl",

pkg/cli/testdata/ear-list

@@ -8,13 +8,13 @@ list
 000004.log:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: f9 35 f1 b1 73 c8 bc 8f bd f3 c0 1b
-  counter: 3210085521
+  nonce: 31 d3 cd 5a 69 e2 13 64 21 53 57 64
+  counter: 3952287331
 000005.sst:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: 6e 34 f4 3c 11 43 1a f5 69 ce 33 f1
-  counter: 2398097086
+  nonce: 23 d9 b2 e1 39 b0 87 ed f9 6d 49 20
+  counter: 3481614039
 COCKROACHDB_DATA_KEYS_000001_monolith:
   env type: Store, AES128_CTR
   keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
@@ -35,11 +35,11 @@ marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith:
   keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
   nonce: 55 d7 d4 27 6c 97 9b dd f1 5d 40 c8
   counter: 467030050
-marker.format-version.000008.021:
+marker.format-version.000011.024:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: df 3a 47 28 5e fb 88 e3 9e b3 9b a3
-  counter: 1505393531
+  nonce: c3 6d b2 7b 3f 3e 67 b9 28 b9 81 b1
+  counter: 3050109376
 marker.manifest.000001.MANIFEST-000001:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef

pkg/cmd/roachtest/tests/backup_restore_roundtrip.go

@@ -545,7 +545,7 @@ func testOnlineRestoreRecovery(ctx context.Context, t test.Test, c cluster.Clust
 			return errors.Wrap(err, "failed to remove pausepoint for online restore")
 		}
 
-		if err := d.deleteRandomSST(ctx, t.L(), dbConn, collection); err != nil {
+		if err := d.deleteUserTableSST(ctx, t.L(), dbConn, collection); err != nil {
 			return err
 		}
 		if _, err := dbConn.ExecContext(ctx, "RESUME JOB $1", downloadJobID); err != nil {

pkg/cmd/roachtest/tests/gopg_blocklist.go

@@ -52,4 +52,6 @@ var gopgIgnoreList = blocklist{
 	`pg | ORM | relation with no results does not panic`:            "unknown",
 	// This test flakes sometimes because of connection reuse.
 	`v10.TestColumnReuse`: "unknown",
+	// This test is flaky sometimes due to the use of temp tables.
+	`pg | soft delete with int column nil model ForceDelete | deletes the model`: "unknown",
 }

pkg/cmd/roachtest/tests/mixed_version_backup.go

@@ -2316,24 +2316,39 @@ func (d *BackupRestoreTestDriver) createBackupCollection(
 	return d.saveContents(ctx, l, rng, &collection, fingerprintAOST)
 }
 
-// deleteRandomSSTs deletes an SST from the full backup in a collection. This is
-// used to test online restore recovery. We delete from the full backup
-// specifically to avoid deleting an SST from an incremental that is skipped due
-// to a compacted backup. This ensures that deleting an SST will cause the
-// download job to fail (assuming it hasn't already been downloaded).
-func (d *BackupRestoreTestDriver) deleteRandomSST(
+// deleteUserTableSST deletes an SST that covers a user table from the full
+// backup in a collection. This is used to test online restore recovery. We
+// delete from the full backup specifically to avoid deleting an SST from an
+// incremental that is skipped due to a compacted backup. This ensures that
+// deleting an SST will cause the download job to fail (assuming it hasn't
+// already been downloaded).
+// Note: We delete specifically a user-table SST as opposed to choosing a random
+// SST because the temporary system table SSTs may be GC'd before the download
+// phase attempts to download the SST. This would cause the download job to
+// succeed instead of failing as expected. See
+// https://github.com/cockroachdb/cockroach/issues/148408 for more details.
+// TODO (kev-cao): Investigate if blocking storage level compaction would
+// prevent GC of temporary system tables and allow us to delete a random SST
+// instead. Technically, it is still possible, but unlikely, for storage to
+// compact away a user-table SST before the download job attempts to download
+// it, which would result in false positives in the test.
+func (d *BackupRestoreTestDriver) deleteUserTableSST(
 	ctx context.Context, l *logger.Logger, db *gosql.DB, collection *backupCollection,
 ) error {
 	var sstPath string
 	if err := db.QueryRowContext(
 		ctx,
 		`SELECT path FROM
-		[SHOW BACKUP FILES FROM LATEST IN $1]
+		(
+			SELECT row_number() OVER (), * FROM
+			[SHOW BACKUP FILES FROM LATEST IN $1]
+		)
 		WHERE backup_type = 'full'
-		ORDER BY random() LIMIT 1 `,
+		ORDER BY row_number DESC
+		LIMIT 1`,
 		collection.uri(),
 	).Scan(&sstPath); err != nil {
-		return errors.Wrapf(err, "failed to get random SST path from %s", collection.uri())
+		return errors.Wrapf(err, "failed to get SST path from %s", collection.uri())
 	}
 	uri, err := url.Parse(collection.uri())
 	if err != nil {

pkg/storage/pebble.go

@@ -2168,6 +2168,7 @@ func (p *Pebble) CreateCheckpoint(dir string, spans []roachpb.Span) error {
 // named version, it can be assumed all *nodes* have ratcheted to the pebble
 // version associated with it, since they did so during the fence version.
 var pebbleFormatVersionMap = map[clusterversion.Key]pebble.FormatMajorVersion{
+	clusterversion.V25_3: pebble.FormatValueSeparation,
 	clusterversion.V25_2: pebble.FormatTableFormatV6,
 }