spanconfig: improve typing for Update

By using a type definition instead of a stand-alone type, we can reduce
the amount of code needed to convert from a SpanConfigEntry to an Update
(something we frequently do). It also helps make it less error prone.
While here, we introduce two helpful constructors for the two kinds of
updates we're typically interested in -- additions and deletions.

Release note: None

pkg/kv/kvserver/client_spanconfigs_test.go

@@ -69,7 +69,7 @@ func TestSpanConfigUpdateAppliedToReplica(t *testing.T) {
 	span := repl.Desc().RSpan().AsRawSpanWithNoLocals()
 	conf := roachpb.SpanConfig{NumReplicas: 5, NumVoters: 3}
 
-	deleted, added := spanConfigStore.Apply(ctx, false /* dryrun */, spanconfig.Update{Span: span, Config: conf})
+	deleted, added := spanConfigStore.Apply(ctx, false /* dryrun */, spanconfig.Addition(span, conf))
 	require.Empty(t, deleted)
 	require.Len(t, added, 1)
 	require.True(t, added[0].Span.Equal(span))

pkg/spanconfig/spanconfig.go

@@ -232,8 +232,8 @@ type StoreReader interface {
 	GetSpanConfigForKey(ctx context.Context, key roachpb.RKey) (roachpb.SpanConfig, error)
 }
 
-// DescriptorUpdate captures the ID and type of a descriptor or zone that the
-// SQLWatcher has observed updated.
+// DescriptorUpdate captures the ID and the type of descriptor or zone that been
+// updated. It's the unit of what the SQLWatcher emits.
 type DescriptorUpdate struct {
 	// ID of the descriptor/zone that has been updated.
 	ID descpb.ID
@@ -244,14 +244,26 @@ type DescriptorUpdate struct {
 }
 
 // Update captures a span and the corresponding config change. It's the unit of
-// what can be applied to a StoreWriter.
-type Update struct {
-	// Span captures the key span being updated.
-	Span roachpb.Span
+// what can be applied to a StoreWriter. The embedded span captures what's being
+// updated; the config captures what it's being updated to. An empty config
+// indicates a deletion.
+type Update roachpb.SpanConfigEntry
 
-	// Config captures the span config the key span was updated to. An empty
-	// config indicates the span config being deleted.
-	Config roachpb.SpanConfig
+// Deletion constructs an update that represents a deletion over the given span.
+func Deletion(span roachpb.Span) Update {
+	return Update{
+		Span:   span,
+		Config: roachpb.SpanConfig{}, // delete
+	}
+}
+
+// Addition constructs an update that represents adding the given config over
+// the given span.
+func Addition(span roachpb.Span, conf roachpb.SpanConfig) Update {
+	return Update{
+		Span:   span,
+		Config: conf,
+	}
 }
 
 // Deletion returns true if the update corresponds to a span config being

pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go

@@ -255,9 +255,11 @@ func (s *KVSubscriber) run(ctx context.Context) error {
 			log.Infof(ctx, "received span configuration update for %s (deleted=%t)", entry.Span, deleted)
 		}
 
-		update := spanconfig.Update{Span: entry.Span}
-		if !deleted {
-			update.Config = entry.Config
+		var update spanconfig.Update
+		if deleted {
+			update = spanconfig.Deletion(entry.Span)
+		} else {
+			update = spanconfig.Update(entry)
 		}
 
 		if err := buffer.Add(&bufferEvent{update, ev.Value.Timestamp}); err != nil {

pkg/spanconfig/spanconfigreconciler/reconciler.go

@@ -184,10 +184,7 @@ func (f *fullReconciler) reconcile(
 
 	updates := make([]spanconfig.Update, len(entries))
 	for i, entry := range entries {
-		updates[i] = spanconfig.Update{
-			Span:   entry.Span,
-			Config: entry.Config,
-		}
+		updates[i] = spanconfig.Update(entry)
 	}
 
 	toDelete, toUpsert := storeWithExistingSpanConfigs.Apply(ctx, false /* dryrun */, updates...)
@@ -212,11 +209,8 @@ func (f *fullReconciler) reconcile(
 	// get rid of them below.
 	var storeWithExtraneousSpanConfigs *spanconfigstore.Store
 	{
-		for _, update := range updates {
-			storeWithExistingSpanConfigs.Apply(ctx, false /* dryrun */, spanconfig.Update{
-				Span:   update.Span,
-				Config: roachpb.SpanConfig{}, // delete
-			})
+		for _, u := range updates {
+			storeWithExistingSpanConfigs.Apply(ctx, false /* dryrun */, spanconfig.Deletion(u.Span))
 		}
 		storeWithExtraneousSpanConfigs = storeWithExistingSpanConfigs
 	}
@@ -229,11 +223,8 @@ func (f *fullReconciler) reconcile(
 	// Update the store that's supposed to reflect the latest span config
 	// contents. As before, we could've fetched this state from KV directly, but
 	// doing it this way is cheaper.
-	for _, deletedSpan := range deletedSpans {
-		storeWithLatestSpanConfigs.Apply(ctx, false /* dryrun */, spanconfig.Update{
-			Span:   deletedSpan,
-			Config: roachpb.SpanConfig{}, // delete
-		})
+	for _, d := range deletedSpans {
+		storeWithLatestSpanConfigs.Apply(ctx, false /* dryrun */, spanconfig.Deletion(d))
 	}
 
 	return storeWithLatestSpanConfigs, reconciledUpUntil, nil
@@ -276,10 +267,7 @@ func (f *fullReconciler) fetchExistingSpanConfigs(
 		}
 
 		for _, entry := range entries {
-			store.Apply(ctx, false /* dryrun */, spanconfig.Update{
-				Span:   entry.Span,
-				Config: entry.Config,
-			})
+			store.Apply(ctx, false /* dryrun */, spanconfig.Update(entry))
 		}
 	}
 	return store, nil
@@ -358,21 +346,15 @@ func (r *incrementalReconciler) reconcile(
 			updates := make([]spanconfig.Update, 0, len(missingTableIDs)+len(entries))
 			for _, entry := range entries {
 				// Update span configs for SQL state that changed.
-				updates = append(updates, spanconfig.Update{
-					Span:   entry.Span,
-					Config: entry.Config,
-				})
+				updates = append(updates, spanconfig.Update(entry))
 			}
 			for _, missingID := range missingTableIDs {
 				// Delete span configs for missing tables.
 				tableSpan := roachpb.Span{
 					Key:    r.codec.TablePrefix(uint32(missingID)),
 					EndKey: r.codec.TablePrefix(uint32(missingID)).PrefixEnd(),
 				}
-				updates = append(updates, spanconfig.Update{
-					Span:   tableSpan,
-					Config: roachpb.SpanConfig{}, // delete
-				})
+				updates = append(updates, spanconfig.Deletion(tableSpan))
 			}
 
 			toDelete, toUpsert := r.storeWithKVContents.Apply(ctx, false /* dryrun */, updates...)

pkg/spanconfig/spanconfigstore/store.go

@@ -146,10 +146,7 @@ func (s *Store) Apply(
 func (s *Store) Copy(ctx context.Context) *Store {
 	clone := New(s.fallback)
 	_ = s.ForEachOverlapping(ctx, keys.EverythingSpan, func(entry roachpb.SpanConfigEntry) error {
-		clone.Apply(ctx, false /* dryrun */, spanconfig.Update{
-			Span:   entry.Span,
-			Config: entry.Config,
-		})
+		clone.Apply(ctx, false /* dryrun */, spanconfig.Update(entry))
 		return nil
 	})
 	return clone

pkg/spanconfig/spanconfigstore/store_test.go

@@ -181,9 +181,9 @@ func TestRandomized(t *testing.T) {
 		sp, conf, op := genRandomSpan(), getRandomConf(), getRandomOp()
 		switch op {
 		case "set":
-			return spanconfig.Update{Span: sp, Config: conf}
+			return spanconfig.Addition(sp, conf)
 		case "del":
-			return spanconfig.Update{Span: sp}
+			return spanconfig.Deletion(sp)
 		default:
 		}
 		t.Fatalf("unexpected op: %s", op)
@@ -318,18 +318,9 @@ func TestStoreClone(t *testing.T) {
 
 	everything := spanconfigtestutils.ParseSpan(t, "[a, z)")
 	updates := []spanconfig.Update{
-		{
-			Span:   spanconfigtestutils.ParseSpan(t, "[a, b)"),
-			Config: spanconfigtestutils.ParseConfig(t, "A"),
-		},
-		{
-			Span:   spanconfigtestutils.ParseSpan(t, "[c, d)"),
-			Config: spanconfigtestutils.ParseConfig(t, "C"),
-		},
-		{
-			Span:   spanconfigtestutils.ParseSpan(t, "[e, f)"),
-			Config: spanconfigtestutils.ParseConfig(t, "E"),
-		},
+		spanconfig.Addition(spanconfigtestutils.ParseSpan(t, "[a, b)"), spanconfigtestutils.ParseConfig(t, "A")),
+		spanconfig.Addition(spanconfigtestutils.ParseSpan(t, "[c, d)"), spanconfigtestutils.ParseConfig(t, "C")),
+		spanconfig.Addition(spanconfigtestutils.ParseSpan(t, "[e, f)"), spanconfigtestutils.ParseConfig(t, "E")),
 	}
 
 	original := New(roachpb.TestingDefaultSpanConfig())

pkg/spanconfig/spanconfigtestutils/recorder.go

@@ -68,13 +68,13 @@ func (r *KVAccessorRecorder) UpdateSpanConfigEntries(
 
 	for _, d := range toDelete {
 		r.mu.mutations = append(r.mu.mutations, mutation{
-			update:   spanconfig.Update{Span: d},
+			update:   spanconfig.Deletion(d),
 			batchIdx: r.mu.batchCount,
 		})
 	}
 	for _, u := range toUpsert {
 		r.mu.mutations = append(r.mu.mutations, mutation{
-			update:   spanconfig.Update{Span: u.Span, Config: u.Config},
+			update:   spanconfig.Update(u),
 			batchIdx: r.mu.batchCount,
 		})
 	}

pkg/spanconfig/spanconfigtestutils/utils.go

@@ -157,11 +157,11 @@ func ParseStoreApplyArguments(t *testing.T, input string) (updates []spanconfig.
 		switch {
 		case strings.HasPrefix(line, deletePrefix):
 			line = strings.TrimPrefix(line, line[:len(deletePrefix)])
-			updates = append(updates, spanconfig.Update{Span: ParseSpan(t, line)})
+			updates = append(updates, spanconfig.Deletion(ParseSpan(t, line)))
 		case strings.HasPrefix(line, setPrefix):
 			line = strings.TrimPrefix(line, line[:len(setPrefix)])
 			entry := ParseSpanConfigEntry(t, line)
-			updates = append(updates, spanconfig.Update{Span: entry.Span, Config: entry.Config})
+			updates = append(updates, spanconfig.Update(entry))
 		default:
 			t.Fatalf("malformed line %q, expected to find prefix %q or %q",
 				line, setPrefix, deletePrefix)