setting: clean up slot indexes

The internal setting code passes around slot indexes as bare integers,
and there are confusingly two variants: one is 1-indexed, another is
0-indexed.

This change makes all slot indexes 0-indexed and adds a `slotIdx`
type.

Note: the 1-indexed choice was perhaps useful at the time to help find
code paths where the slot index is not initialized, but now the slot
index is set along with other mandatory fields by `init()`.

Release note: None

pkg/settings/bool.go

@@ -24,7 +24,7 @@ var _ internalSetting = &BoolSetting{}
 
 // Get retrieves the bool value in the setting.
 func (b *BoolSetting) Get(sv *Values) bool {
-	return sv.getInt64(b.slotIdx) != 0
+	return sv.getInt64(b.slot) != 0
 }
 
 func (b *BoolSetting) String(sv *Values) string {
@@ -65,20 +65,20 @@ func (b *BoolSetting) Override(ctx context.Context, sv *Values, v bool) {
 	if v {
 		vInt = 1
 	}
-	sv.setDefaultOverrideInt64(b.slotIdx, vInt)
+	sv.setDefaultOverrideInt64(b.slot, vInt)
 }
 
 func (b *BoolSetting) set(ctx context.Context, sv *Values, v bool) {
 	vInt := int64(0)
 	if v {
 		vInt = 1
 	}
-	sv.setInt64(ctx, b.slotIdx, vInt)
+	sv.setInt64(ctx, b.slot, vInt)
 }
 
 func (b *BoolSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(b.slotIdx)
+	ok, val, _ := sv.getDefaultOverride(b.slot)
 	if ok {
 		b.set(ctx, sv, val > 0)
 		return

pkg/settings/common.go

@@ -17,59 +17,60 @@ import (
 
 // common implements basic functionality used by all setting types.
 type common struct {
-	key         string
-	description string
-	class       Class
-	visibility  Visibility
-	// Each setting has a slotIdx which is used as a handle with Values.
-	slotIdx       int
+	class         Class
+	key           string
+	description   string
+	visibility    Visibility
+	slot          slotIdx
 	nonReportable bool
 	retired       bool
 }
 
+// slotIdx is an integer in the range [0, MaxSetting) which is uniquely
+// associated with a registered setting. Slot indexes are used as "handles" for
+// manipulating the setting values. They are generated sequentially, in the
+// order of registration.
+type slotIdx int32
+
 // init must be called to initialize the fields that don't have defaults.
-func (i *common) init(class Class, slotIdx int, key string, description string) {
-	i.class = class
-	if slotIdx < 1 {
-		panic(fmt.Sprintf("Invalid slot index %d", slotIdx))
+func (c *common) init(class Class, key string, description string, slot slotIdx) {
+	c.class = class
+	c.key = key
+	c.description = description
+	if slot < 0 {
+		panic(fmt.Sprintf("Invalid slot index %d", slot))
 	}
-	if slotIdx > MaxSettings {
+	if slot >= MaxSettings {
 		panic("too many settings; increase MaxSettings")
 	}
-	i.slotIdx = slotIdx
-	i.key = key
-	i.description = description
-}
-
-func (i *common) isRetired() bool {
-	return i.retired
+	c.slot = slot
 }
 
-func (i *common) getSlotIdx() int {
-	return i.slotIdx
+func (c common) Class() Class {
+	return c.class
 }
 
-func (i common) Key() string {
-	return i.key
+func (c common) Key() string {
+	return c.key
 }
 
-func (i common) Description() string {
-	return i.description
+func (c common) Description() string {
+	return c.description
 }
 
-func (i common) Class() Class {
-	return i.class
+func (c common) Visibility() Visibility {
+	return c.visibility
 }
 
-func (i common) Visibility() Visibility {
-	return i.visibility
+func (c common) isReportable() bool {
+	return !c.nonReportable
 }
 
-func (i common) isReportable() bool {
-	return !i.nonReportable
+func (c *common) isRetired() bool {
+	return c.retired
 }
 
-func (i *common) ErrorHint() (bool, string) {
+func (c *common) ErrorHint() (bool, string) {
 	return false, ""
 }
 
@@ -83,36 +84,36 @@ func (i *common) ErrorHint() (bool, string) {
 //
 // All string settings are also non-reportable by default and must be
 // opted in to reports manually with SetReportable(true).
-func (i *common) SetReportable(reportable bool) {
-	i.nonReportable = !reportable
+func (c *common) SetReportable(reportable bool) {
+	c.nonReportable = !reportable
 }
 
 // SetVisibility customizes the visibility of a setting.
-func (i *common) SetVisibility(v Visibility) {
-	i.visibility = v
+func (c *common) SetVisibility(v Visibility) {
+	c.visibility = v
 }
 
 // SetRetired marks the setting as obsolete. It also hides
 // it from the output of SHOW CLUSTER SETTINGS.
-func (i *common) SetRetired() {
-	i.description = "do not use - " + i.description
-	i.retired = true
+func (c *common) SetRetired() {
+	c.description = "do not use - " + c.description
+	c.retired = true
 }
 
 // SetOnChange installs a callback to be called when a setting's value changes.
 // `fn` should avoid doing long-running or blocking work as it is called on the
 // goroutine which handles all settings updates.
-func (i *common) SetOnChange(sv *Values, fn func(ctx context.Context)) {
-	sv.setOnChange(i.slotIdx, fn)
+func (c *common) SetOnChange(sv *Values, fn func(ctx context.Context)) {
+	sv.setOnChange(c.slot, fn)
 }
 
 type internalSetting interface {
 	NonMaskedSetting
 
-	init(class Class, slotIdx int, key string, desc string)
+	init(class Class, key, description string, slot slotIdx)
 	isRetired() bool
 	setToDefault(ctx context.Context, sv *Values)
-	getSlotIdx() int
+
 	// isReportable indicates whether the value of the setting can be
 	// included in user-facing reports such as that produced by SHOW ALL
 	// CLUSTER SETTINGS.
@@ -126,5 +127,5 @@ type internalSetting interface {
 // numericSetting is used for settings that can be set using an integer value.
 type numericSetting interface {
 	internalSetting
-	set(ctx context.Context, sv *Values, i int64) error
+	set(ctx context.Context, sv *Values, value int64) error
 }

pkg/settings/duration.go

@@ -43,7 +43,7 @@ func (d *DurationSettingWithExplicitUnit) ErrorHint() (bool, string) {
 
 // Get retrieves the duration value in the setting.
 func (d *DurationSetting) Get(sv *Values) time.Duration {
-	return time.Duration(sv.getInt64(d.slotIdx))
+	return time.Duration(sv.getInt64(d.slot))
 }
 
 func (d *DurationSetting) String(sv *Values) string {
@@ -88,21 +88,21 @@ func (d *DurationSetting) Validate(v time.Duration) error {
 //
 // For testing usage only.
 func (d *DurationSetting) Override(ctx context.Context, sv *Values, v time.Duration) {
-	sv.setInt64(ctx, d.slotIdx, int64(v))
-	sv.setDefaultOverrideInt64(d.slotIdx, int64(v))
+	sv.setInt64(ctx, d.slot, int64(v))
+	sv.setDefaultOverrideInt64(d.slot, int64(v))
 }
 
 func (d *DurationSetting) set(ctx context.Context, sv *Values, v time.Duration) error {
 	if err := d.Validate(v); err != nil {
 		return err
 	}
-	sv.setInt64(ctx, d.slotIdx, int64(v))
+	sv.setInt64(ctx, d.slot, int64(v))
 	return nil
 }
 
 func (d *DurationSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(d.slotIdx)
+	ok, val, _ := sv.getDefaultOverride(d.slot)
 	if ok {
 		// As per the semantics of override, these values don't go through
 		// validation.

pkg/settings/float.go

@@ -30,7 +30,7 @@ var _ internalSetting = &FloatSetting{}
 
 // Get retrieves the float value in the setting.
 func (f *FloatSetting) Get(sv *Values) float64 {
-	return math.Float64frombits(uint64(sv.getInt64(f.slotIdx)))
+	return math.Float64frombits(uint64(sv.getInt64(f.slot)))
 }
 
 func (f *FloatSetting) String(sv *Values) string {
@@ -68,7 +68,7 @@ func (f *FloatSetting) Override(ctx context.Context, sv *Values, v float64) {
 	if err := f.set(ctx, sv, v); err != nil {
 		panic(err)
 	}
-	sv.setDefaultOverrideInt64(f.slotIdx, int64(math.Float64bits(v)))
+	sv.setDefaultOverrideInt64(f.slot, int64(math.Float64bits(v)))
 }
 
 // Validate that a value conforms with the validation function.
@@ -85,13 +85,13 @@ func (f *FloatSetting) set(ctx context.Context, sv *Values, v float64) error {
 	if err := f.Validate(v); err != nil {
 		return err
 	}
-	sv.setInt64(ctx, f.slotIdx, int64(math.Float64bits(v)))
+	sv.setInt64(ctx, f.slot, int64(math.Float64bits(v)))
 	return nil
 }
 
 func (f *FloatSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(f.slotIdx)
+	ok, val, _ := sv.getDefaultOverride(f.slot)
 	if ok {
 		// As per the semantics of override, these values don't go through
 		// validation.

pkg/settings/int.go

@@ -29,7 +29,7 @@ var _ numericSetting = &IntSetting{}
 
 // Get retrieves the int value in the setting.
 func (i *IntSetting) Get(sv *Values) int64 {
-	return sv.container.getInt64(i.slotIdx)
+	return sv.container.getInt64(i.slot)
 }
 
 func (i *IntSetting) String(sv *Values) string {
@@ -74,21 +74,21 @@ func (i *IntSetting) Validate(v int64) error {
 //
 // For testing usage only.
 func (i *IntSetting) Override(ctx context.Context, sv *Values, v int64) {
-	sv.setInt64(ctx, i.slotIdx, v)
-	sv.setDefaultOverrideInt64(i.slotIdx, v)
+	sv.setInt64(ctx, i.slot, v)
+	sv.setDefaultOverrideInt64(i.slot, v)
 }
 
 func (i *IntSetting) set(ctx context.Context, sv *Values, v int64) error {
 	if err := i.Validate(v); err != nil {
 		return err
 	}
-	sv.setInt64(ctx, i.slotIdx, v)
+	sv.setInt64(ctx, i.slot, v)
 	return nil
 }
 
 func (i *IntSetting) setToDefault(ctx context.Context, sv *Values) {
 	// See if the default value was overridden.
-	ok, val, _ := sv.getDefaultOverride(i.slotIdx)
+	ok, val, _ := sv.getDefaultOverride(i.slot)
 	if ok {
 		// As per the semantics of override, these values don't go through
 		// validation.

pkg/settings/registry.go

@@ -142,9 +142,9 @@ func register(class Class, key, desc string, s internalSetting) {
 			))
 		}
 	}
+	slot := slotIdx(len(registry))
+	s.init(class, key, desc, slot)
 	registry[key] = s
-	slotIdx := len(registry)
-	s.init(class, slotIdx, key, desc)
 }
 
 // NumRegisteredSettings returns the number of registered settings.

pkg/settings/string.go

@@ -56,7 +56,7 @@ var _ = (*StringSetting).Default
 
 // Get retrieves the string value in the setting.
 func (s *StringSetting) Get(sv *Values) string {
-	loaded := sv.getGeneric(s.slotIdx)
+	loaded := sv.getGeneric(s.slot)
 	if loaded == nil {
 		return ""
 	}
@@ -84,7 +84,7 @@ func (s *StringSetting) set(ctx context.Context, sv *Values, v string) error {
 		return err
 	}
 	if s.Get(sv) != v {
-		sv.setGeneric(ctx, s.slotIdx, v)
+		sv.setGeneric(ctx, s.slot, v)
 	}
 	return nil
 }