Merge #144515 #145405 #145445

144515: backup: update compaction to have its own manifest creation code r=msbutler a=kev-cao

Previously, the compaction logic would rely on the `createBackupManifest` from the existing backup code, which caused some issues as outlined in #144512. This patch updates compaction logic to instead have its own manifest creation code that does not rely on existing cluster state.

Fixes: #144512

Release note: None

145405: vecindex: add Level field to metadata KV key encoding r=drewkimball,michae2 a=andy-kimball

Previously, the Metadata KV Key for a vector index partition had these fields:

│Index Prefix│Prefix Columns│PartitionKey│Family ID 0│

This commit adds a Level field, which always has the value of zero:

│Index Prefix│Prefix Columns│PartitionKey│Level 0│Family ID 0│

This is needed because the KV splits do not handle cases where one KV key is a prefix of other keys. It can cause an unsplittable range to form. By adding the Level field with value zero, we guarantee that the Metadata KV key always sorts before all Vector KV keys in the partition, and yet isn't a prefix of them.

Epic: CRDB-42943
Release note: Vector indexes created in beta releases have an encoding issue that may result in failed inserts. These indexes should be dropped and re-created before being used with later releases.

145445: ci: bump timeout for lint r=celiala a=rickystewart

... from 30m to 60m. This timed out in #145433.

Epic: none
Release note: None

Co-authored-by: Kevin Cao <[email protected]>
Co-authored-by: Andrew Kimball <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

Author: 4 people

build/github/lint.sh

@@ -28,7 +28,7 @@ bazel test \
   --sandbox_writable_path=$HOME \
   --test_env=GO_SDK=$(dirname $(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath))) \
   --test_env=COCKROACH_WORKSPACE=$WORKSPACE \
-  --test_timeout=1800 \
+  --test_timeout=3600 \
   --build_event_binary_file=bes.bin \
   --jobs 50 \
   --remote_download_minimal $(./build/github/engflow-args.sh)

go.mod

@@ -159,6 +159,7 @@ require (
 	github.com/elastic/gosigar v0.14.3
 	github.com/emicklei/dot v0.15.0
 	github.com/fatih/color v1.9.0
+	github.com/fatih/structs v1.1.0
 	github.com/felixge/fgprof v0.9.5
 	github.com/fsnotify/fsnotify v1.5.1
 	github.com/getsentry/sentry-go v0.27.0
@@ -326,7 +327,6 @@ require (
 	github.com/eapache/go-resiliency v1.6.0 // indirect
 	github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 // indirect
 	github.com/eapache/queue v1.1.0 // indirect
-	github.com/fatih/structs v1.1.0 // indirect
 	github.com/form3tech-oss/jwt-go v3.2.5+incompatible // indirect
 	github.com/gabriel-vasile/mimetype v1.4.2 // indirect
 	github.com/ghodss/yaml v1.0.0 // indirect

pkg/backup/BUILD.bazel

@@ -224,6 +224,7 @@ go_test(
         "//pkg/backup/backuputils",
         "//pkg/base",
         "//pkg/blobs",
+        "//pkg/build",
         "//pkg/build/bazel",
         "//pkg/ccl/kvccl",
         "//pkg/ccl/multiregionccl",
@@ -345,6 +346,7 @@ go_test(
         "@com_github_cockroachdb_pebble//vfs",
         "@com_github_cockroachdb_redact//:redact",
         "@com_github_cockroachdb_version//:version",
+        "@com_github_fatih_structs//:structs",
         "@com_github_gogo_protobuf//types",
         "@com_github_jackc_pgx_v5//:pgx",
         "@com_github_kr_pretty//:pretty",

pkg/backup/compaction_job.go

@@ -22,7 +22,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/joberror"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
-	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/scheduledjobs"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -256,12 +255,12 @@ func (b *backupResumer) ResumeCompaction(
 			return err
 		}
 		updatedDetails, err = updateCompactionBackupDetails(
-			ctx, compactChain, initialDetails, backupDest, baseEncryptionOpts, kmsEnv,
+			ctx, compactChain, initialDetails, backupDest, baseEncryptionOpts,
 		)
 		if err != nil {
 			return err
 		}
-		backupManifest, err = createCompactionManifest(ctx, execCtx, updatedDetails, compactChain)
+		backupManifest, err = compactChain.createCompactionManifest(ctx, updatedDetails)
 		if err != nil {
 			return err
 		}
@@ -407,7 +406,9 @@ type compactionChain struct {
 	// for a restore.
 	backupChain    []backuppb.BackupManifest
 	chainToCompact []backuppb.BackupManifest
-	// start refers to the start time of the first backup to be compacted.
+	// start refers to the start time of the first backup to be compacted. This
+	// will always be greater than 0 as a full backup is not a candidate for
+	// compaction.
 	// end refers to the end time of the last backup to be compacted.
 	start, end hlc.Timestamp
 	// Inclusive startIdx and exclusive endIdx of the sub-chain to compact.
@@ -485,7 +486,6 @@ func updateCompactionBackupDetails(
 	initialDetails jobspb.BackupDetails,
 	resolvedDest backupdest.ResolvedDestination,
 	baseEncryptOpts *jobspb.BackupEncryptionOptions,
-	kmsEnv cloud.KMSEnv,
 ) (jobspb.BackupDetails, error) {
 	if len(compactionChain.chainToCompact) == 0 {
 		return jobspb.BackupDetails{}, errors.New("no backup manifests to compact")
@@ -525,19 +525,50 @@ func updateCompactionBackupDetails(
 	return compactedDetails, nil
 }
 
-// compactIntroducedSpans takes a compacted backup manifest and the full chain of backups it belongs
-// to and computes the introduced spans for the compacted backup.
-func compactIntroducedSpans(
-	ctx context.Context, manifest backuppb.BackupManifest, chain compactionChain,
-) (roachpb.Spans, error) {
-	if err := checkCoverage(ctx, manifest.Spans, chain.backupChain); err != nil {
-		return roachpb.Spans{}, err
-	}
-	return filterSpans(
-			manifest.Spans,
-			chain.backupChain[chain.startIdx-1].Spans,
-		),
-		nil
+// createCompactionManifest creates a new manifest for a compaction job and its
+// compacted chain. The details should have its targets resolved.
+func (c compactionChain) createCompactionManifest(
+	ctx context.Context, details jobspb.BackupDetails,
+) (*backuppb.BackupManifest, error) {
+	// TODO (kev-cao): Will need to update the SSTSinkKeyWriter to support
+	// range keys.
+	lastBackup := c.lastBackup()
+	if len(lastBackup.Tenants) != 0 {
+		return nil, errors.New("backup compactions does not yet support range keys")
+	}
+	if err := checkCoverage(ctx, c.lastBackup().Spans, c.backupChain); err != nil {
+		return nil, err
+	}
+	// In compaction, any span that is not included in the backup *preceding* the
+	// compaction chain is considered introduced.
+	introducedSpans := filterSpans(
+		c.lastBackup().Spans,
+		c.backupChain[c.startIdx-1].Spans,
+	)
+	cManifest := lastBackup
+	cManifest.ID = uuid.MakeV4()
+	cManifest.StartTime = c.start
+	cManifest.Descriptors = details.ResolvedTargets
+	cManifest.IntroducedSpans = introducedSpans
+	cManifest.IsCompacted = true
+
+	cManifest.Dir = cloudpb.ExternalStorage{}
+	// As this manifest will be used prior to the manifest actually being written
+	// (and therefore its external files being created) we first set
+	// HasExternalManifestSSTs to false and allow it to be set to true later when
+	// it is written to storage.
+	cManifest.HasExternalManifestSSTs = false
+	// While we do not compaction on revision history backups, we still need to
+	// nil out DescriptorChanges to avoid the placeholder descriptor from the
+	// manifest of the last incremental.
+	cManifest.DescriptorChanges = nil
+	cManifest.Files = nil
+	cManifest.EntryCounts = roachpb.RowCount{}
+
+	// The StatisticsFileNames is inherited from the stats of the latest
+	// incremental we are compacting as the compacted manifest will have the same
+	// targets as the last incremental.
+	return &cManifest, nil
 }
 
 // resolveBackupSubdir returns the resolved base full backup subdirectory from a
@@ -590,50 +621,8 @@ func resolveBackupDirs(
 	return resolvedBaseDirs, resolvedIncDirs, resolvedSubdir, nil
 }
 
-// createCompactionManifest creates a new manifest for a compaction job and its
-// compacted chain.
-func createCompactionManifest(
-	ctx context.Context,
-	execCtx sql.JobExecContext,
-	details jobspb.BackupDetails,
-	compactChain compactionChain,
-) (*backuppb.BackupManifest, error) {
-	var tenantSpans []roachpb.Span
-	var tenantInfos []mtinfopb.TenantInfoWithUsage
-	if err := execCtx.ExecCfg().InternalDB.Txn(ctx, func(ctx context.Context, txn isql.Txn) error {
-		var err error
-		tenantSpans, tenantInfos, err = getTenantInfo(ctx, execCtx.ExecCfg().Codec, txn, details)
-		return err
-	}); err != nil {
-		return nil, err
-	}
-	// TODO (kev-cao): Will need to update the SSTSinkKeyWriter to support
-	// range keys.
-	if len(tenantSpans) != 0 || len(tenantInfos) != 0 {
-		return nil, errors.New("backup compactions does not yet support range keys")
-	}
-	m, err := createBackupManifest(
-		ctx,
-		execCtx.ExecCfg(),
-		tenantSpans,
-		tenantInfos,
-		details,
-		compactChain.backupChain,
-		compactChain.allIters,
-	)
-	if err != nil {
-		return nil, err
-	}
-	m.IsCompacted = true
-	m.IntroducedSpans, err = compactIntroducedSpans(ctx, m, compactChain)
-	if err != nil {
-		return nil, err
-	}
-	return &m, nil
-}
-
 // getBackupChain fetches the current shortest chain of backups (and its
-// associated info) required to restore the to the end time specified in the details.
+// associated info) required to restore to the end time specified in the details.
 func getBackupChain(
 	ctx context.Context,
 	execCfg *sql.ExecutorConfig,