Merge #148795 #148868

148795: kvserver: Remove SnapshotReservationTimeoutError r=Abhinav1299 a=Abhinav1299

SnapshotReservationTimeoutError is a part of the gRPC response error that was getting overly redacted. The overall gRPC response should be handled to fix the over redaction instead just the error part. This patch reverts the #147395.

Epic: CRDB-37533
Part of: CRDB-44885
Release note: None

148868: concurrency: disallow lock flushing in production builds r=tbg a=stevendanna

This is an undocumented setting that is still under development. While it is undocumented, this additional check will avoid any accidental use.

Epic: none
Release note: None

Co-authored-by: Abhinav1299 <[email protected]>
Co-authored-by: Steven Danna <[email protected]>

Author: 3 people

pkg/kv/kvpb/errors.go

@@ -1828,34 +1828,6 @@ func NewKeyCollisionError(key roachpb.Key, value []byte) error {
 	return ret
 }
 
-// snapshotReservationTimeoutError represents an error that occurs when
-// giving up during snapshot reservation due to cluster setting timeout.
-type SnapshotReservationTimeoutError struct {
-	cause       error
-	settingName string
-}
-
-// Error implements the error interface.
-func (e *SnapshotReservationTimeoutError) Error() string {
-	return redact.Sprint(e).StripMarkers()
-}
-
-// SafeFormatError implements errors.SafeFormatter.
-func (e *SnapshotReservationTimeoutError) SafeFormatError(p errors.Printer) (next error) {
-	p.Printf("giving up during snapshot reservation due to cluster setting %q: %v", redact.SafeString(e.settingName), redact.SafeString(e.cause.Error()))
-	return nil
-}
-
-// NewSnapshotReservationTimeoutError creates a new SnapshotReservationTimeoutError.
-func NewSnapshotReservationTimeoutError(
-	cause error, settingName string,
-) *SnapshotReservationTimeoutError {
-	return &SnapshotReservationTimeoutError{
-		cause:       cause,
-		settingName: settingName,
-	}
-}
-
 // NewExclusionViolationError creates a new ExclusionViolationError. This error
 // is returned by requests that encounter an existing value written at a
 // timestamp at which they expected to have an exclusive lock on the key. This
@@ -1960,5 +1932,4 @@ var _ errors.SafeFormatter = &UnhandledRetryableError{}
 var _ errors.SafeFormatter = &ReplicaUnavailableError{}
 var _ errors.SafeFormatter = &ProxyFailedError{}
 var _ errors.SafeFormatter = &KeyCollisionError{}
-var _ errors.SafeFormatter = &SnapshotReservationTimeoutError{}
 var _ errors.SafeFormatter = &ExclusionViolationError{}

pkg/kv/kvserver/concurrency/concurrency_manager.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/debugutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -132,15 +133,27 @@ var UnreplicatedLockReliabilityLeaseTransfer = settings.RegisterBoolSetting(
 	"kv.lock_table.unreplicated_lock_reliability.lease_transfer.enabled",
 	"whether the replica should attempt to keep unreplicated locks during lease transfers",
 	metamorphic.ConstantWithTestBool("kv.lock_table.unreplicated_lock_reliability.lease_transfer.enabled", false),
+	settings.WithValidateBool(func(_ *settings.Values, enabled bool) error {
+		if enabled && !buildutil.CrdbTestBuild {
+			return errors.Newf("kv.lock_table.unreplicated_lock_reliability.lease_transfer.enabled is not supported in production builds")
+		}
+		return nil
+	}),
 )
 
-// UnreplicatedLockReliabilityMerge controls whether the replica will
-// attempt to keep unreplicated locks during range merge operations.
+// UnreplicatedLockReliabilityMerge controls whether the replica will attempt to
+// keep unreplicated locks during range merge operations.
 var UnreplicatedLockReliabilityMerge = settings.RegisterBoolSetting(
 	settings.SystemOnly,
 	"kv.lock_table.unreplicated_lock_reliability.merge.enabled",
 	"whether the replica should attempt to keep unreplicated locks during range merges",
 	metamorphic.ConstantWithTestBool("kv.lock_table.unreplicated_lock_reliability.merge.enabled", false),
+	settings.WithValidateBool(func(_ *settings.Values, enabled bool) error {
+		if enabled && !buildutil.CrdbTestBuild {
+			return errors.Newf("kv.lock_table.unreplicated_lock_reliability.merge.enabled is not supported in production builds")
+		}
+		return nil
+	}),
 )
 
 var MaxLockFlushSize = settings.RegisterByteSizeSetting(

pkg/kv/kvserver/store_snapshot.go

@@ -273,8 +273,10 @@ func (s *Store) throttleSnapshot(
 			if err := ctx.Err(); err != nil {
 				return nil, errors.Wrap(err, "acquiring snapshot reservation")
 			}
-			return nil, kvpb.NewSnapshotReservationTimeoutError(
-				queueCtx.Err(), string(snapshotReservationQueueTimeoutFraction.Name()),
+			return nil, errors.Wrapf(
+				queueCtx.Err(),
+				"giving up during snapshot reservation due to cluster setting %q",
+				snapshotReservationQueueTimeoutFraction.Name(),
 			)
 		case <-s.stopper.ShouldQuiesce():
 			return nil, errors.Errorf("stopped")

pkg/kv/kvserver/store_test.go

@@ -3471,11 +3471,6 @@ func TestReserveSnapshotQueueTimeoutAvoidsStarvation(t *testing.T) {
 						if errors.Is(err, context.DeadlineExceeded) {
 							return nil
 						}
-						// Also handle the new SnapshotReservationTimeoutError as a timeout condition
-						var snapshotTimeoutErr *kvpb.SnapshotReservationTimeoutError
-						if errors.As(err, &snapshotTimeoutErr) {
-							return nil
-						}
 						return err
 					}
 					defer cleanup()