Merge #155555 #155563 #155624 #155626

155555: roachtest: increase server.sqlliveness.ttl in copyfrom r=yuzefovich a=yuzefovich

`copyfrom/atomic` often fails with `RETRY_COMMIT_DEADLINE_EXCEEDED` error. Based on some guidance from KV folks, it sounds like (apart from the closed TS system that has already been adjusted) the most likely explanation is that the lease on a descriptor has expired while the txn was running. Before 24.1 we used expiry based leasing, then we introduced session based leasing, and the migration to only use that has been completed in the beginning of the year. If I'm reading the code right, then the lease duration depends on the SQL liveness TTL, so this commit bumps the relevant cluster setting from 40s to 5m to give enough time for atomic COPY to finish.

I did 50 runs of the test, and all of them passed.

Fixes: #155300.
Release note: None

155563: importer: unskip all tests with test tenants r=yuzefovich a=yuzefovich

This mostly involved ensuring that the right ApplicationLayerInterface is being used, or the test just worked. Also a couple of tests explicitly control the tenants, so mark those accordingly.

My hypothesis is that some tests have been fixed long time ago in 4fd9f70 (`tc.ServerConn` now returns the right connection), but I didn't verify it.

Only `TestImportIntoCSV` needs a special callout. In that test we have some expected errors, and some of them when encountered by the tenants can be retried. In order to speed up the test we reduce the retry duration from 2 minutes to 2 seconds.

Additionally remove some SQLMemoryPoolSize overrides that now equal the default of 256MiB (which increased from 128MiB a couple years ago).

Fixes: #107141.
Fixes: #142803.
Release note: None

155624: backup: move backup index logic to backupinfo pkg r=kev-cao a=msbutler

The backup index is part of the backup metadata, whose logic lives in the backupinfo package. This refactor also enables subsequent refactor to unify backup metadata writing for regular backups and compacted backups.

Epic: none

Release note: none

155626: dev-inf: Fix jq command to handle JSON array format r=rickystewart a=ajstorm

The execution file is a JSON array, not JSONL format. Updated all
extraction steps to use '.[]' to iterate over the array before
filtering by type.

Changed from: jq -r 'select(.type == "result") | .result'
Changed to: jq -r '.[] | select(.type == "result") | .result'

This fixes the error: "Cannot index array with string 'type'"

Tested locally with both array and JSONL formats to verify the fix works.

Epic: none
Release note: none

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Michael Butler <butler@cockroachlabs.com>
Co-authored-by: Adam Storm <storm@cockroachlabs.com>

Author: 4 people

.github/workflows/pr-analyzer-threestage.yml

@@ -66,7 +66,7 @@ jobs:
         id: stage1_result
         if: steps.stage1.conclusion == 'success'
         run: |
-          RESULT=$(jq -r 'select(.type == "result") | .result' "${{ steps.stage1.outputs.execution_file }}")
+          RESULT=$(jq -r '.[] | select(.type == "result") | .result' "${{ steps.stage1.outputs.execution_file }}")
           {
             echo 'result<<EOF'
             echo "$RESULT"
@@ -116,7 +116,7 @@ jobs:
         id: stage2_result
         if: steps.stage2.conclusion == 'success'
         run: |
-          RESULT=$(jq -r 'select(.type == "result") | .result' "${{ steps.stage2.outputs.execution_file }}")
+          RESULT=$(jq -r '.[] | select(.type == "result") | .result' "${{ steps.stage2.outputs.execution_file }}")
           {
             echo 'result<<EOF'
             echo "$RESULT"
@@ -181,7 +181,7 @@ jobs:
         id: stage3_result
         if: steps.stage3.conclusion == 'success'
         run: |
-          RESULT=$(jq -r 'select(.type == "result") | .result' "${{ steps.stage3.outputs.execution_file }}")
+          RESULT=$(jq -r '.[] | select(.type == "result") | .result' "${{ steps.stage3.outputs.execution_file }}")
           {
             echo 'result<<EOF'
             echo "$RESULT"

pkg/backup/backup_job.go

@@ -444,7 +444,7 @@ func backup(
 		return roachpb.RowCount{}, 0, err
 	}
 
-	if err := backupdest.WriteBackupIndexMetadata(
+	if err := backupinfo.WriteBackupIndexMetadata(
 		ctx,
 		execCtx.ExecCfg(),
 		execCtx.User(),

pkg/backup/backupdest/BUILD.bazel

@@ -4,7 +4,6 @@ go_library(
     name = "backupdest",
     srcs = [
         "backup_destination.go",
-        "backup_index.go",
         "incrementals.go",
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/backup/backupdest",
@@ -17,24 +16,19 @@ go_library(
         "//pkg/backup/backuputils",
         "//pkg/cloud",
         "//pkg/cloud/cloudpb",
-        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/roachpb",
         "//pkg/security/username",
-        "//pkg/settings",
         "//pkg/settings/cluster",
         "//pkg/sql",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/util",
-        "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
         "//pkg/util/hlc",
         "//pkg/util/ioctx",
         "//pkg/util/log",
-        "//pkg/util/metamorphic",
         "//pkg/util/mon",
-        "//pkg/util/protoutil",
         "//pkg/util/timeutil",
         "//pkg/util/tracing",
         "@com_github_cockroachdb_errors//:errors",
@@ -46,7 +40,6 @@ go_test(
     name = "backupdest_test",
     srcs = [
         "backup_destination_test.go",
-        "backup_index_test.go",
         "incrementals_test.go",
         "main_test.go",
     ],
@@ -57,35 +50,29 @@ go_test(
     }),
     deps = [
         "//pkg/backup/backupbase",
+        "//pkg/backup/backupinfo",
         "//pkg/backup/backuppb",
         "//pkg/backup/backuptestutils",
         "//pkg/backup/backuputils",
         "//pkg/base",
-        "//pkg/blobs",
         "//pkg/ccl",
         "//pkg/cloud",
-        "//pkg/cloud/cloudpb",
         "//pkg/cloud/impl:cloudimpl",
-        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",
         "//pkg/security/username",
         "//pkg/server",
-        "//pkg/settings/cluster",
         "//pkg/sql",
-        "//pkg/testutils",
         "//pkg/testutils/jobutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/testcluster",
         "//pkg/util",
         "//pkg/util/hlc",
-        "//pkg/util/ioctx",
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/protoutil",
         "//pkg/util/randutil",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",
     ],
 )

pkg/backup/backupdest/backup_destination.go

@@ -237,7 +237,7 @@ func ResolveDest(
 		}
 	}
 
-	partName := ConstructDateBasedIncrementalFolderName(startTime.GoTime(), endTime.GoTime())
+	partName := backupinfo.ConstructDateBasedIncrementalFolderName(startTime.GoTime(), endTime.GoTime())
 	defaultIncrementalsURI, urisByLocalityKV, err := GetURIsByLocalityKV(fullyResolvedIncrementalsLocation, partName)
 	if err != nil {
 		return ResolvedDestination{}, err
@@ -478,7 +478,7 @@ func ListFullBackupsInCollection(
 	ctx context.Context, store cloud.ExternalStorage, useIndex bool,
 ) ([]string, error) {
 	if useIndex {
-		return ListSubdirsFromIndex(ctx, store)
+		return backupinfo.ListSubdirsFromIndex(ctx, store)
 	}
 
 	var backupPaths []string
@@ -539,15 +539,15 @@ func ResolveBackupManifests(
 	}
 	defer rootStore.Close()
 
-	if !ReadBackupIndexEnabled.Get(&execCfg.Settings.SV) || isCustomIncLocation {
+	if !backupinfo.ReadBackupIndexEnabled.Get(&execCfg.Settings.SV) || isCustomIncLocation {
 		return legacyResolveBackupManifests(
 			ctx, execCfg, mem, defaultCollectionURI, mkStore,
 			resolvedSubdir, fullyResolvedBaseDirectory, fullyResolvedIncrementalsDirectory,
 			endTime, encryption, kmsEnv, user, includeSkipped, includeCompacted,
 		)
 	}
 
-	exists, err := IndexExists(ctx, rootStore, resolvedSubdir)
+	exists, err := backupinfo.IndexExists(ctx, rootStore, resolvedSubdir)
 	if err != nil {
 		return nil, nil, nil, 0, err
 	}
@@ -759,7 +759,7 @@ func indexedResolveBackupManifests(
 		}
 	}()
 
-	indexes, err := GetBackupTreeIndexMetadata(
+	indexes, err := backupinfo.GetBackupTreeIndexMetadata(
 		ctx, rootStores[0], resolvedSubdir,
 	)
 	if err != nil {

pkg/backup/backupdest/incrementals.go

@@ -7,14 +7,13 @@ package backupdest
 
 import (
 	"context"
-	"fmt"
 	"path"
 	"regexp"
 	"sort"
 	"strings"
-	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
+	"github.com/cockroachdb/cockroach/pkg/backup/backupinfo"
 	"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -92,11 +91,11 @@ func FindAllIncrementalPaths(
 	defer sp.Finish()
 
 	// Backup indexes do not support custom incremental locations.
-	if customIncLocation || !ReadBackupIndexEnabled.Get(&execCfg.Settings.SV) {
+	if customIncLocation || !backupinfo.ReadBackupIndexEnabled.Get(&execCfg.Settings.SV) {
 		return LegacyFindPriorBackups(ctx, incStore, OmitManifest)
 	}
 
-	indexes, err := ListIndexes(ctx, rootStore, subdir)
+	indexes, err := backupinfo.ListIndexes(ctx, rootStore, subdir)
 	if err != nil {
 		return nil, err
 	}
@@ -110,7 +109,7 @@ func FindAllIncrementalPaths(
 	return util.MapE(
 		indexes[1:], // We skip the full backup
 		func(indexFilename string) (string, error) {
-			return parseBackupFilePathFromIndexFileName(subdir, indexFilename)
+			return backupinfo.ParseBackupFilePathFromIndexFileName(subdir, indexFilename)
 		},
 	)
 }
@@ -317,18 +316,3 @@ func ResolveDefaultBaseIncrementalStorageLocation(
 
 	return defaultURI, nil
 }
-
-// ConstructDateBasedIncrementalFolderName constructs the name of a date-based
-// incremental backup folder relative to the full subdirectory it belongs to.
-//
-// /2025/07/30-120000.00/20250730/130000.00-20250730-120000.00
-//
-//	 	                 └─────────────────────────────────────┘
-//										               returns this
-func ConstructDateBasedIncrementalFolderName(start, end time.Time) string {
-	return fmt.Sprintf(
-		"%s-%s",
-		end.Format(backupbase.DateBasedIncFolderName),
-		start.Format(backupbase.DateBasedIncFolderNameSuffix),
-	)
-}

pkg/backup/backupdest/incrementals_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
 	"github.com/cockroachdb/cockroach/pkg/backup/backupdest"
+	"github.com/cockroachdb/cockroach/pkg/backup/backupinfo"
 	"github.com/cockroachdb/cockroach/pkg/backup/backuptestutils"
 	"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
@@ -250,7 +251,7 @@ func TestFindAllIncrementalPaths(t *testing.T) {
 
 			var expectedPaths []string
 			for _, b := range targetChain[1:] {
-				expectedPaths = append(expectedPaths, backupdest.ConstructDateBasedIncrementalFolderName(
+				expectedPaths = append(expectedPaths, backupinfo.ConstructDateBasedIncrementalFolderName(
 					toTime(b.start), toTime(b.end),
 				))
 			}
@@ -439,7 +440,7 @@ func writeEmptyBackupManifest(
 		}
 		backupPath = backuputils.JoinURLPath(
 			subdir,
-			backupdest.ConstructDateBasedIncrementalFolderName(start, end),
+			backupinfo.ConstructDateBasedIncrementalFolderName(start, end),
 		)
 	}
 
@@ -486,7 +487,7 @@ func writeEmptyBackupManifest(
 
 	require.NoError(
 		t,
-		backupdest.WriteBackupIndexMetadata(
+		backupinfo.WriteBackupIndexMetadata(
 			context.Background(), execCfg, username.RootUserName(),
 			execCfg.DistSQLSrv.ExternalStorageFromURI, backupDetails, hlc.Timestamp{},
 		),

pkg/backup/backupinfo/BUILD.bazel

@@ -4,6 +4,7 @@ load("//pkg/testutils:buildutil/buildutil.bzl", "disallowed_imports_test")
 go_library(
     name = "backupinfo",
     srcs = [
+        "backup_index.go",
         "desc_sst.go",
         "external_sst_util.go",
         "file_sst.go",
@@ -19,11 +20,13 @@ go_library(
         "//pkg/ccl/storageccl",
         "//pkg/cloud",
         "//pkg/cloud/cloudpb",
+        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/keys",
         "//pkg/kv/kvpb",
         "//pkg/roachpb",
         "//pkg/security/username",
+        "//pkg/settings",
         "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/dbdesc",
@@ -43,6 +46,7 @@ go_library(
         "//pkg/util/hlc",
         "//pkg/util/ioctx",
         "//pkg/util/log",
+        "//pkg/util/metamorphic",
         "//pkg/util/mon",
         "//pkg/util/protoutil",
         "//pkg/util/syncutil",
@@ -57,17 +61,22 @@ go_library(
 go_test(
     name = "backupinfo_test",
     srcs = [
+        "backup_index_test.go",
         "desc_sst_test.go",
         "main_test.go",
         "manifest_handling_test.go",
     ],
     embed = [":backupinfo"],
     deps = [
+        "//pkg/backup/backupbase",
         "//pkg/backup/backuppb",
+        "//pkg/backup/backuptestutils",
         "//pkg/base",
         "//pkg/blobs",
         "//pkg/ccl",
         "//pkg/cloud",
+        "//pkg/cloud/cloudpb",
+        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/keys",
         "//pkg/multitenant/mtinfopb",
@@ -76,15 +85,20 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/security/username",
         "//pkg/server",
+        "//pkg/settings/cluster",
+        "//pkg/sql",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/isql",
+        "//pkg/testutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/testcluster",
         "//pkg/util",
         "//pkg/util/bulk",
         "//pkg/util/hlc",
+        "//pkg/util/ioctx",
         "//pkg/util/leaktest",
         "//pkg/util/log",
+        "//pkg/util/protoutil",
         "//pkg/util/randutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",

pkg/backup/backupdest/backup_index.go renamed to pkg/backup/backupinfo/backup_index.go

@@ -3,7 +3,7 @@
 // Use of this software is governed by the CockroachDB Software License
 // included in the /LICENSE file.
 
-package backupdest
+package backupinfo
 
 import (
 	"bytes"
@@ -279,7 +279,7 @@ func GetBackupTreeIndexMetadata(
 // Note: While the path is stored in the index file, we can take a shortcut here
 // and derive it from the filename solely because backup paths are
 // millisecond-precise and so are the timestamps encoded in the filename.
-func parseBackupFilePathFromIndexFileName(subdir, basename string) (string, error) {
+func ParseBackupFilePathFromIndexFileName(subdir, basename string) (string, error) {
 	start, end, err := parseIndexFilename(basename)
 	if err != nil {
 		return "", err

pkg/backup/backupdest/backup_index_test.go renamed to pkg/backup/backupinfo/backup_index_test.go

@@ -3,7 +3,7 @@
 // Use of this software is governed by the CockroachDB Software License
 // included in the /LICENSE file.
 
-package backupdest
+package backupinfo
 
 import (
 	"bufio"

pkg/backup/backupinfo/manifest_handling.go

@@ -16,6 +16,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
 	"github.com/cockroachdb/cockroach/pkg/backup/backupencryption"
@@ -1760,3 +1761,18 @@ func GetBackupManifestIterFactories(
 
 	return layerToFileIterFactory, nil
 }
+
+// ConstructDateBasedIncrementalFolderName constructs the name of a date-based
+// incremental backup folder relative to the full subdirectory it belongs to.
+//
+// /2025/07/30-120000.00/20250730/130000.00-20250730-120000.00
+//
+//	 	                 └─────────────────────────────────────┘
+//										               returns this
+func ConstructDateBasedIncrementalFolderName(start, end time.Time) string {
+	return fmt.Sprintf(
+		"%s-%s",
+		end.Format(backupbase.DateBasedIncFolderName),
+		start.Format(backupbase.DateBasedIncFolderNameSuffix),
+	)
+}