Merge #70464 #71962

70464: Re-introduce "kv,migration: rm code handling legacy raft truncated st… r=irfansharif a=irfansharif

…ate"

This reverts commit ef1dd6f. #70432
reverted #69887, as temporary stop-gap until we release the first 21.2
beta. See the discussion over on #70432 for why we want to queue up this
revert to the original revert; this should only be merged after #69826
lands.

Release note: None

71962: backup: mark some settings public r=dt a=dt

This marks some of BACKUP's cluster settings as public as they are
intended for user-tuning to match their desired workload / requirements,
such as the delay before invoking priority reads or the target file size.

Release note (ops change): Some existing settings related to BACKUP execution are now listed  by SHOW CLUSTER SETTINGS.

Fixes #71786.

Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: David Taylor <[email protected]>

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

@@ -2,6 +2,9 @@ Setting	Type	Default	Description
 admission.kv.enabled	boolean	false	when true, work performed by the KV layer is subject to admission control
 admission.sql_kv_response.enabled	boolean	false	when true, work performed by the SQL layer when receiving a KV response is subject to admission control
 admission.sql_sql_response.enabled	boolean	false	when true, work performed by the SQL layer when receiving a DistSQL response is subject to admission control
+bulkio.backup.file_size	byte size	128 MiB	target size for individual data files produced during BACKUP
+bulkio.backup.read_timeout	duration	5m0s	amount of time after which a read attempt is considered timed out, which causes the backup to fail
+bulkio.backup.read_with_priority_after	duration	1m0s	amount of time since the read-as-of time above which a BACKUP should use priority when retrying reads
 bulkio.stream_ingestion.minimum_flush_interval	duration	5s	the minimum timestamp between flushes; flushes may still occur if internal buffers fill up
 changefeed.node_throttle_config	string		specifies node level throttling configuration for all changefeeeds
 cloudstorage.http.custom_ca	string		custom root CA (appended to system's default CAs) for verifying certificates when interacting with HTTPS storage

docs/generated/settings/settings.html

@@ -4,6 +4,9 @@
 <tr><td><code>admission.kv.enabled</code></td><td>boolean</td><td><code>false</code></td><td>when true, work performed by the KV layer is subject to admission control</td></tr>
 <tr><td><code>admission.sql_kv_response.enabled</code></td><td>boolean</td><td><code>false</code></td><td>when true, work performed by the SQL layer when receiving a KV response is subject to admission control</td></tr>
 <tr><td><code>admission.sql_sql_response.enabled</code></td><td>boolean</td><td><code>false</code></td><td>when true, work performed by the SQL layer when receiving a DistSQL response is subject to admission control</td></tr>
+<tr><td><code>bulkio.backup.file_size</code></td><td>byte size</td><td><code>128 MiB</code></td><td>target size for individual data files produced during BACKUP</td></tr>
+<tr><td><code>bulkio.backup.read_timeout</code></td><td>duration</td><td><code>5m0s</code></td><td>amount of time after which a read attempt is considered timed out, which causes the backup to fail</td></tr>
+<tr><td><code>bulkio.backup.read_with_priority_after</code></td><td>duration</td><td><code>1m0s</code></td><td>amount of time since the read-as-of time above which a BACKUP should use priority when retrying reads</td></tr>
 <tr><td><code>bulkio.stream_ingestion.minimum_flush_interval</code></td><td>duration</td><td><code>5s</code></td><td>the minimum timestamp between flushes; flushes may still occur if internal buffers fill up</td></tr>
 <tr><td><code>changefeed.node_throttle_config</code></td><td>string</td><td><code></code></td><td>specifies node level throttling configuration for all changefeeeds</td></tr>
 <tr><td><code>cloudstorage.http.custom_ca</code></td><td>string</td><td><code></code></td><td>custom root CA (appended to system's default CAs) for verifying certificates when interacting with HTTPS storage</td></tr>

pkg/ccl/backupccl/backup_processor.go

@@ -54,10 +54,10 @@ var (
 	)
 	priorityAfter = settings.RegisterDurationSetting(
 		"bulkio.backup.read_with_priority_after",
-		"age of read-as-of time above which a BACKUP should read with priority",
+		"amount of time since the read-as-of time above which a BACKUP should use priority when retrying reads",
 		time.Minute,
 		settings.NonNegativeDuration,
-	)
+	).WithPublic()
 	delayPerAttmpt = settings.RegisterDurationSetting(
 		"bulkio.backup.read_retry_delay",
 		"amount of time since the read-as-of time, per-prior attempt, to wait before making another attempt",
@@ -66,22 +66,23 @@ var (
 	)
 	timeoutPerAttempt = settings.RegisterDurationSetting(
 		"bulkio.backup.read_timeout",
-		"amount of time after which a read attempt is considered timed out and is canceled. "+
-			"Hitting this timeout will cause the backup job to fail.",
+		"amount of time after which a read attempt is considered timed out, which causes the backup to fail",
 		time.Minute*5,
 		settings.NonNegativeDuration,
-	)
+	).WithPublic()
 	targetFileSize = settings.RegisterByteSizeSetting(
 		"bulkio.backup.file_size",
-		"target file size",
+		"target size for individual data files produced during BACKUP",
 		128<<20,
-	)
+	).WithPublic()
+
 	smallFileBuffer = settings.RegisterByteSizeSetting(
 		"bulkio.backup.merge_file_buffer_size",
 		"size limit used when buffering backup files before merging them",
 		16<<20,
 		settings.NonNegativeInt,
 	)
+
 	splitKeysOnTimestamps = settings.RegisterBoolSetting(
 		"bulkio.backup.split_keys_on_timestamps",
 		"split backup data on timestamps when writing revision history",

pkg/cli/debug_check_store.go

@@ -252,7 +252,7 @@ func checkStoreRaftState(
 					return err
 				}
 				getReplicaInfo(rangeID).committedIndex = hs.Commit
-			case bytes.Equal(suffix, keys.LocalRaftTruncatedStateLegacySuffix):
+			case bytes.Equal(suffix, keys.LocalRaftTruncatedStateSuffix):
 				var trunc roachpb.RaftTruncatedState
 				if err := kv.Value.GetProto(&trunc); err != nil {
 					return err
@@ -264,12 +264,6 @@ func checkStoreRaftState(
 					return err
 				}
 				getReplicaInfo(rangeID).appliedIndex = state.RaftAppliedIndex
-			case bytes.Equal(suffix, keys.LocalRaftAppliedIndexLegacySuffix):
-				idx, err := kv.Value.GetInt()
-				if err != nil {
-					return err
-				}
-				getReplicaInfo(rangeID).appliedIndex = uint64(idx)
 			case bytes.Equal(suffix, keys.LocalRaftLogSuffix):
 				_, index, err := encoding.DecodeUint64Ascending(detail)
 				if err != nil {

pkg/clusterversion/cockroach_versions.go

@@ -156,32 +156,6 @@ const (
 
 	// v21.1 versions.
 	//
-	// replacedTruncatedAndRangeAppliedStateMigration stands in for
-	// TruncatedAndRangeAppliedStateMigration which was	re-introduced after the
-	// migration job was introduced. This is necessary because the jobs
-	// infrastructure used to run this migration in v21.1 and its later alphas
-	// was introduced after this version was first introduced. Later code in the
-	// release relies on the job to run the migration but the job relies on
-	// its startup migrations having been run. Versions associated with long
-	// running migrations must follow deletedLongRunningMigrations.
-	replacedTruncatedAndRangeAppliedStateMigration
-	// replacedPostTruncatedAndRangeAppliedStateMigration is like the above
-	// version. See its comment.
-	replacedPostTruncatedAndRangeAppliedStateMigration
-	// TruncatedAndRangeAppliedStateMigration is part of the migration to stop
-	// using the legacy truncated state within KV. After the migration, we'll be
-	// using the unreplicated truncated state and the RangeAppliedState on all
-	// ranges. Callers that wish to assert on there no longer being any legacy
-	// will be able to do so after PostTruncatedAndRangeAppliedStateMigration is
-	// active. This lets remove any holdover code handling the possibility of
-	// replicated truncated state in 21.2.
-	//
-	// TODO(irfansharif): Do the above in 21.2.
-	TruncatedAndRangeAppliedStateMigration
-	// PostTruncatedAndRangeAppliedStateMigration is used to purge all replicas
-	// using the replicated legacy TruncatedState. It's also used in asserting
-	// that no replicated truncated state representation is found.
-	PostTruncatedAndRangeAppliedStateMigration
 	// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
 	//
 	// TODO(irfansharif): This can be removed as part of #71708 (bump
@@ -328,22 +302,6 @@ const (
 // to be added (i.e., when cutting the final release candidate).
 var versionsSingleton = keyedVersions{
 	// v21.1 versions.
-	{
-		Key:     replacedTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 14},
-	},
-	{
-		Key:     replacedPostTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 16},
-	},
-	{
-		Key:     TruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 22},
-	},
-	{
-		Key:     PostTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 24},
-	},
 	{
 		// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
 		Key:     V21_1,

pkg/keys/constants.go

@@ -80,17 +80,12 @@ var (
 	// LocalRangeAppliedStateSuffix is the suffix for the range applied state
 	// key.
 	LocalRangeAppliedStateSuffix = []byte("rask")
-	// LocalRaftAppliedIndexLegacySuffix is the suffix for the raft applied index.
-	LocalRaftAppliedIndexLegacySuffix = []byte("rfta")
-	// LocalRaftTruncatedStateLegacySuffix is the suffix for the legacy
-	// RaftTruncatedState. See VersionUnreplicatedRaftTruncatedState.
+	// LocalRaftTruncatedStateSuffix is the suffix for the
+	// RaftTruncatedState.
 	// Note: This suffix is also used for unreplicated Range-ID keys.
-	LocalRaftTruncatedStateLegacySuffix = []byte("rftt")
+	LocalRaftTruncatedStateSuffix = []byte("rftt")
 	// LocalRangeLeaseSuffix is the suffix for a range lease.
 	LocalRangeLeaseSuffix = []byte("rll-")
-	// LocalLeaseAppliedIndexLegacySuffix is the suffix for the applied lease
-	// index.
-	LocalLeaseAppliedIndexLegacySuffix = []byte("rlla")
 	// LocalRangePriorReadSummarySuffix is the suffix for a range's prior read
 	// summary.
 	LocalRangePriorReadSummarySuffix = []byte("rprs")

pkg/keys/doc.go

@@ -185,16 +185,12 @@ var _ = [...]interface{}{
 	//   range as a whole. Though they are replicated, they are unaddressable.
 	//   Typical examples are MVCC stats and the abort span. They all share
 	//   `LocalRangeIDPrefix` and `LocalRangeIDReplicatedInfix`.
-	AbortSpanKey,                // "abc-"
-	RangeGCThresholdKey,         // "lgc-"
-	RangeAppliedStateKey,        // "rask"
-	RaftAppliedIndexLegacyKey,   // "rfta"
-	RaftTruncatedStateLegacyKey, // "rftt"
-	RangeLeaseKey,               // "rll-"
-	LeaseAppliedIndexLegacyKey,  // "rlla"
-	RangePriorReadSummaryKey,    // "rprs"
-	RangeVersionKey,             // "rver"
-	RangeStatsLegacyKey,         // "stat"
+	AbortSpanKey,             // "abc-"
+	RangeGCThresholdKey,      // "lgc-"
+	RangeAppliedStateKey,     // "rask"
+	RangeLeaseKey,            // "rll-"
+	RangePriorReadSummaryKey, // "rprs"
+	RangeVersionKey,          // "rver"
 
 	//   2. Unreplicated range-ID local keys: These contain metadata that
 	//   pertain to just one replica of a range. They are unreplicated and