From 7a6d0c17e37bb737abe91ef233ec479153e2f35f Mon Sep 17 00:00:00 2001 From: Nick Travers Date: Mon, 27 Feb 2023 16:04:04 -0800 Subject: [PATCH] clusterversion,storage: use range tombstones unconditionally in 23.1 Currently, MVCC range tombstones are only enabled when the `storage.mvcc.range_tombstones.enabled` cluster setting is `true`. Originally, the plan for 23.1 was to remove this cluster setting entirely, which would enable the feature unconditionally. However, it was decided that for 23.1, the setting would remain, along with the utility methods that determine whether or not the feature is enabled. As there is still an expectation that the feature is enabled in 23.1, add an internal cluster version that is used as a feature gate. If the cluster is at least of version `V23_1_MVCCRangeTombstonesUnconditionallyEnabled`, MVCC range tombstones are _unconditionally_ enabled, irrespective of whether the cluster setting is enabled or not. Fix #91147. Release note: None. --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/clusterversion/cockroach_versions.go | 13 +++++++++++++ pkg/storage/mvcc.go | 18 +++++++++++++----- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 5024cb438c76..f2f0679df820 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -297,4 +297,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 1000022.2-56 set the active cluster version in the format '.' +version version 1000022.2-58 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index f0ce6b518a95..72681ff886fe 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -238,6 +238,6 @@
trace.opentelemetry.collector
stringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
trace.span_registry.enabled
booleantrueif set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector
stringthe address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used. -
version
version1000022.2-56set the active cluster version in the format '<major>.<minor>' +
version
version1000022.2-58set the active cluster version in the format '<major>.<minor>' diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 985502b8bd35..17cf6f4afbd6 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -444,6 +444,15 @@ const ( // backfilled. V23_1DatabaseRoleSettingsRoleIDColumnBackfilled + // V23_1_MVCCRangeTombstonesUnconditionallyEnabled is a version gate at and + // after which Cockroach will always write MVCC Range Tombstones, regardless + // of the value of the storage.mvcc.range_tombstones.enabled cluster setting. + // Prior to this version, it was possible for a cluster to be writing MVCC + // Range Tombstones, but only if the cluster had been opted in manually, under + // a specific set of circumstances (i.e. appropriate 22.2.x version, Cockroach + // Cloud cluster, etc.). + V23_1_MVCCRangeTombstonesUnconditionallyEnabled + // ************************************************* // Step (1): Add new versions here. // Do not add new versions to a patch release. @@ -769,6 +778,10 @@ var rawVersionsSingleton = keyedVersions{ Key: V23_1DatabaseRoleSettingsRoleIDColumnBackfilled, Version: roachpb.Version{Major: 22, Minor: 2, Internal: 56}, }, + { + Key: V23_1_MVCCRangeTombstonesUnconditionallyEnabled, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 58}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index 76df4a2b279e..63eb0e03eb09 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -22,6 +22,7 @@ import ( "sync" "time" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv/kvnemesis/kvnemesisutil" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" @@ -83,18 +84,25 @@ var minWALSyncInterval = settings.RegisterDurationSetting( // If disabled after being enabled, this will prevent new range tombstones from // being written, but already written tombstones will remain until GCed. The // above note on jobs also applies in this case. +// +// If the version of the cluster is at or beyond the version +// V23_1_MVCCRangeTombstonesUnconditionallyEnabled, the feature is +// unconditionally enabled. var MVCCRangeTombstonesEnabled = settings.RegisterBoolSetting( settings.TenantReadOnly, "storage.mvcc.range_tombstones.enabled", "enables the use of MVCC range tombstones", true) -// CanUseMVCCRangeTombstones returns true if the caller can begin writing -// MVCC range tombstones, by setting DeleteRangeRequest.UseRangeTombstone. -// It requires the MVCCRangeTombstones version gate to be active, and the -// setting storage.mvcc.range_tombstones.enabled to be enabled. +// CanUseMVCCRangeTombstones returns true if the caller can begin writing MVCC +// range tombstones, by setting DeleteRangeRequest.UseRangeTombstone. It +// requires the storage.mvcc.range_tombstones.enabled cluster setting to be +// enabled, OR the cluster version is at or beyond the +// V23_1_MVCCRangeTombstonesUnconditionallyEnabled version (i.e. in 23.1, the +// feature is unconditionally enabled). func CanUseMVCCRangeTombstones(ctx context.Context, st *cluster.Settings) bool { - return MVCCRangeTombstonesEnabled.Get(&st.SV) + return st.Version.IsActive(ctx, clusterversion.V23_1_MVCCRangeTombstonesUnconditionallyEnabled) || + MVCCRangeTombstonesEnabled.Get(&st.SV) } // MaxIntentsPerWriteIntentError sets maximum number of intents returned in