Merge cockroachdb#125644

125644: settings: code cleanup and improvements r=RaduBerinde a=RaduBerinde

#### setting: clean up DefaultString

It makes no sense for this method to return an error, especially when
the `String()` method doesn't. This was the consequence of a
round-about way of implementing it.

Epic: none
Release note: None

#### settings: add decodeAndSet/decodeAndSetDefaultOverride methods

Replace type switches with methods.

Epic: none
Release note: None

Co-authored-by: Radu Berinde <[email protected]>

pkg/settings/bool.go

@@ -34,6 +34,11 @@ func (b *BoolSetting) String(sv *Values) string {
 	return EncodeBool(b.Get(sv))
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (b *BoolSetting) DefaultString() string {
+	return EncodeBool(b.defaultValue)
+}
+
 // Encoded returns the encoded value of the current value of the setting.
 func (b *BoolSetting) Encoded(sv *Values) string {
 	return b.String(sv)
@@ -68,11 +73,6 @@ func (b *BoolSetting) Default() bool {
 	return b.defaultValue
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (b *BoolSetting) DefaultString() (string, error) {
-	return b.DecodeToString(b.EncodedDefault())
-}
-
 // Defeat the linter.
 var _ = (*BoolSetting).Default
 
@@ -94,6 +94,26 @@ func (b *BoolSetting) set(ctx context.Context, sv *Values, v bool) {
 	sv.setInt64(ctx, b.slot, vInt)
 }
 
+func (b *BoolSetting) decodeAndSet(ctx context.Context, sv *Values, encoded string) error {
+	v, err := strconv.ParseBool(encoded)
+	if err != nil {
+		return err
+	}
+	b.set(ctx, sv, v)
+	return nil
+}
+
+func (b *BoolSetting) decodeAndSetDefaultOverride(
+	ctx context.Context, sv *Values, encoded string,
+) error {
+	v, err := strconv.ParseBool(encoded)
+	if err != nil {
+		return err
+	}
+	sv.setDefaultOverride(b.slot, v)
+	return nil
+}
+
 func (b *BoolSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
 	if val := sv.getDefaultOverride(b.slot); val != nil {

pkg/settings/byte_size.go

@@ -22,7 +22,7 @@ type ByteSizeSetting struct {
 	IntSetting
 }
 
-var _ numericSetting = &ByteSizeSetting{}
+var _ internalSetting = &ByteSizeSetting{}
 
 // Typ returns the short (1 char) string denoting the type of setting.
 func (*ByteSizeSetting) Typ() string {
@@ -33,6 +33,11 @@ func (b *ByteSizeSetting) String(sv *Values) string {
 	return string(humanizeutil.IBytes(b.Get(sv)))
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (b *ByteSizeSetting) DefaultString() string {
+	return string(humanizeutil.IBytes(b.defaultValue))
+}
+
 // DecodeToString decodes and renders an encoded value.
 func (b *ByteSizeSetting) DecodeToString(encoded string) (string, error) {
 	iv, err := b.DecodeValue(encoded)
@@ -42,11 +47,6 @@ func (b *ByteSizeSetting) DecodeToString(encoded string) (string, error) {
 	return string(humanizeutil.IBytes(iv)), nil
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (b *ByteSizeSetting) DefaultString() (string, error) {
-	return b.DecodeToString(b.EncodedDefault())
-}
-
 // RegisterByteSizeSetting defines a new setting with type bytesize and any
 // supplied validation function(s). If no validation functions are given, then
 // the non-negative int validation is performed.

pkg/settings/common.go

@@ -156,8 +156,6 @@ type internalSetting interface {
 	init(class Class, key InternalKey, description string, slot slotIdx)
 	isRetired() bool
 	isSensitive() bool
-	setToDefault(ctx context.Context, sv *Values)
-
 	getSlot() slotIdx
 
 	// isReportable indicates whether the value of the setting can be
@@ -168,11 +166,14 @@ type internalSetting interface {
 	// it cannot be listed, but can be accessed with `SHOW CLUSTER
 	// SETTING enterprise.license` or SET CLUSTER SETTING.
 	isReportable() bool
-}
 
-// numericSetting is used for settings that can be set using an integer value.
-type numericSetting interface {
-	internalSetting
-	DecodeValue(value string) (int64, error)
-	set(ctx context.Context, sv *Values, value int64) error
+	setToDefault(ctx context.Context, sv *Values)
+
+	// decodeAndSet sets the setting after decoding the encoded value.
+	decodeAndSet(ctx context.Context, sv *Values, encoded string) error
+
+	// decodeAndOverrideDefault overrides the default value for the respective
+	// setting to newVal. It does not change the current value. Validation checks
+	// are not run against the decoded value.
+	decodeAndSetDefaultOverride(ctx context.Context, sv *Values, encoded string) error
 }

pkg/settings/duration.go

@@ -50,6 +50,11 @@ func (d *DurationSetting) String(sv *Values) string {
 	return EncodeDuration(d.Get(sv))
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (d *DurationSetting) DefaultString() string {
+	return EncodeDuration(d.defaultValue)
+}
+
 // Encoded returns the encoded value of the current value of the setting.
 func (d *DurationSetting) Encoded(sv *Values) string {
 	return d.String(sv)
@@ -66,7 +71,7 @@ func (d *DurationSetting) DecodeToString(encoded string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return EncodeDuration(v), nil
+	return v.String(), nil
 }
 
 // DecodeValue decodes the value into a float.
@@ -84,11 +89,6 @@ func (d *DurationSetting) Default() time.Duration {
 	return d.defaultValue
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (d *DurationSetting) DefaultString() (string, error) {
-	return d.DecodeToString(d.EncodedDefault())
-}
-
 // Defeat the linter.
 var _ = (*DurationSetting).Default
 
@@ -120,6 +120,25 @@ func (d *DurationSetting) set(ctx context.Context, sv *Values, v time.Duration)
 	return nil
 }
 
+func (d *DurationSetting) decodeAndSet(ctx context.Context, sv *Values, encoded string) error {
+	v, err := d.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	return d.set(ctx, sv, v)
+}
+
+func (d *DurationSetting) decodeAndSetDefaultOverride(
+	ctx context.Context, sv *Values, encoded string,
+) error {
+	v, err := d.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	sv.setDefaultOverride(d.slot, v)
+	return nil
+}
+
 func (d *DurationSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
 	if val := sv.getDefaultOverride(d.slot); val != nil {

pkg/settings/enum.go

@@ -12,13 +12,10 @@ package settings
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 	"sort"
 	"strconv"
 	"strings"
-
-	"github.com/cockroachdb/errors"
 )
 
 // EnumSetting is a StringSetting that restricts the values to be one of the `enumValues`
@@ -27,7 +24,7 @@ type EnumSetting struct {
 	enumValues map[int64]string
 }
 
-var _ numericSetting = &EnumSetting{}
+var _ internalSetting = &EnumSetting{}
 
 // Typ returns the short (1 char) string denoting the type of setting.
 func (e *EnumSetting) Typ() string {
@@ -44,8 +41,8 @@ func (e *EnumSetting) String(sv *Values) string {
 }
 
 // DefaultString returns the default value for the setting as a string.
-func (e *EnumSetting) DefaultString() (string, error) {
-	return e.DecodeToString(e.EncodedDefault())
+func (e *EnumSetting) DefaultString() string {
+	return e.enumValues[e.defaultValue]
 }
 
 // DecodeToString decodes and renders an encoded value.
@@ -113,13 +110,6 @@ func (e *EnumSetting) GetAvailableValues() []string {
 	return vals
 }
 
-func (e *EnumSetting) set(ctx context.Context, sv *Values, k int64) error {
-	if _, ok := e.enumValues[k]; !ok {
-		return errors.Errorf("unrecognized value %d", k)
-	}
-	return e.IntSetting.set(ctx, sv, k)
-}
-
 func enumValuesToDesc(enumValues map[int64]string) string {
 	var buffer bytes.Buffer
 	values := make([]int64, 0, len(enumValues))

pkg/settings/float.go

@@ -38,6 +38,11 @@ func (f *FloatSetting) String(sv *Values) string {
 	return EncodeFloat(f.Get(sv))
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (f *FloatSetting) DefaultString() string {
+	return EncodeFloat(f.defaultValue)
+}
+
 // Encoded returns the encoded value of the current value of the setting.
 func (f *FloatSetting) Encoded(sv *Values) string {
 	return f.String(sv)
@@ -72,11 +77,6 @@ func (f *FloatSetting) Default() float64 {
 	return f.defaultValue
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (f *FloatSetting) DefaultString() (string, error) {
-	return f.DecodeToString(f.EncodedDefault())
-}
-
 // Defeat the linter.
 var _ = (*FloatSetting).Default
 
@@ -110,6 +110,25 @@ func (f *FloatSetting) set(ctx context.Context, sv *Values, v float64) error {
 	return nil
 }
 
+func (f *FloatSetting) decodeAndSet(ctx context.Context, sv *Values, encoded string) error {
+	v, err := f.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	return f.set(ctx, sv, v)
+}
+
+func (f *FloatSetting) decodeAndSetDefaultOverride(
+	ctx context.Context, sv *Values, encoded string,
+) error {
+	v, err := f.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	sv.setDefaultOverride(f.slot, v)
+	return nil
+}
+
 func (f *FloatSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
 	if val := sv.getDefaultOverride(f.slot); val != nil {

pkg/settings/int.go

@@ -26,7 +26,7 @@ type IntSetting struct {
 	validateFn   func(int64) error
 }
 
-var _ numericSetting = &IntSetting{}
+var _ internalSetting = &IntSetting{}
 
 // Get retrieves the int value in the setting.
 func (i *IntSetting) Get(sv *Values) int64 {
@@ -37,6 +37,11 @@ func (i *IntSetting) String(sv *Values) string {
 	return EncodeInt(i.Get(sv))
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (i *IntSetting) DefaultString() string {
+	return EncodeInt(i.defaultValue)
+}
+
 // Encoded returns the encoded value of the current value of the setting.
 func (i *IntSetting) Encoded(sv *Values) string {
 	return i.String(sv)
@@ -71,11 +76,6 @@ func (i *IntSetting) Default() int64 {
 	return i.defaultValue
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (i *IntSetting) DefaultString() (string, error) {
-	return i.DecodeToString(i.EncodedDefault())
-}
-
 // Defeat the linter.
 var _ = (*IntSetting).Default
 
@@ -99,6 +99,29 @@ func (i *IntSetting) Override(ctx context.Context, sv *Values, v int64) {
 	sv.setDefaultOverride(i.slot, v)
 }
 
+func (i *IntSetting) decodeAndSet(ctx context.Context, sv *Values, encoded string) error {
+	v, err := strconv.ParseInt(encoded, 10, 64)
+	if err != nil {
+		return err
+	}
+	if err := i.Validate(v); err != nil {
+		return err
+	}
+	sv.setInt64(ctx, i.slot, v)
+	return nil
+}
+
+func (i *IntSetting) decodeAndSetDefaultOverride(
+	ctx context.Context, sv *Values, encoded string,
+) error {
+	v, err := strconv.ParseInt(encoded, 10, 64)
+	if err != nil {
+		return err
+	}
+	sv.setDefaultOverride(i.slot, v)
+	return nil
+}
+
 func (i *IntSetting) set(ctx context.Context, sv *Values, v int64) error {
 	if err := i.Validate(v); err != nil {
 		return err

pkg/settings/masked.go

@@ -52,8 +52,8 @@ func (s *MaskedSetting) String(sv *Values) string {
 }
 
 // DefaultString returns the default value for the setting as a string.
-func (s *MaskedSetting) DefaultString() (string, error) {
-	return s.setting.DecodeToString(s.setting.EncodedDefault())
+func (s *MaskedSetting) DefaultString() string {
+	return s.setting.DefaultString()
 }
 
 // Visibility returns the visibility setting for the underlying setting.

pkg/settings/protobuf.go

@@ -45,6 +45,15 @@ func (s *ProtobufSetting) String(sv *Values) string {
 	return json
 }
 
+// DefaultString returns the default value for the setting as a string.
+func (s *ProtobufSetting) DefaultString() string {
+	json, err := s.MarshalToJSON(s.defaultValue)
+	if err != nil {
+		panic(errors.Wrapf(err, "marshaling %s: %+v", proto.MessageName(s.defaultValue), s.defaultValue))
+	}
+	return json
+}
+
 // Encoded returns the encoded value of the current value of the setting.
 func (s *ProtobufSetting) Encoded(sv *Values) string {
 	p := s.Get(sv)
@@ -88,11 +97,6 @@ func (s *ProtobufSetting) Default() protoutil.Message {
 	return s.defaultValue
 }
 
-// DefaultString returns the default value for the setting as a string.
-func (s *ProtobufSetting) DefaultString() (string, error) {
-	return s.DecodeToString(s.EncodedDefault())
-}
-
 // Get retrieves the protobuf value in the setting.
 func (s *ProtobufSetting) Get(sv *Values) protoutil.Message {
 	loaded := sv.getGeneric(s.slot)
@@ -117,6 +121,25 @@ func (s *ProtobufSetting) Override(ctx context.Context, sv *Values, p protoutil.
 	sv.setDefaultOverride(s.slot, p)
 }
 
+func (s *ProtobufSetting) decodeAndSet(ctx context.Context, sv *Values, encoded string) error {
+	p, err := s.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	return s.set(ctx, sv, p)
+}
+
+func (s *ProtobufSetting) decodeAndSetDefaultOverride(
+	ctx context.Context, sv *Values, encoded string,
+) error {
+	p, err := s.DecodeValue(encoded)
+	if err != nil {
+		return err
+	}
+	sv.setDefaultOverride(s.slot, p)
+	return nil
+}
+
 func (s *ProtobufSetting) set(ctx context.Context, sv *Values, p protoutil.Message) error {
 	if err := s.Validate(sv, p); err != nil {
 		return err