From 0a6897de788b0fe2a91266497b3819f2850d4c7c Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Thu, 20 Jan 2022 11:47:25 -0500 Subject: [PATCH] kvserver: avoid clobbering replica conf Fixes #75109. There are two ways to read the configuration applied over a given replica: (a) the copy embedded in each replica struct (b) spanconfig.StoreReader, hanging off the store struct The interface in (b) is implemented by both the span configs infrastructure and the legacy system config span it's designed to replace; it's typically used by KV queues (see #69172). When switching between subsystems in KV (controlled by spanconfig.store.enabled), for we transparently switch the source for (b). We also use then use the right source to refresh (a) for every replica. Incremental updates thereafter mutate (a) directly. As you'd expect, we need to take special care that only one subsystem is writing to (a) at a given point in time, a guarantee that was not upheld before this commit. This bug manifested after we enabled span configs by default (see #73876), and likely affected many tests. To avoid the system config span from clobbering (a) when span configs are enabled, we just need to check what spanconfig.store.enabled holds at the right spot. To repro: # Reliably fails with 1-2m on my GCE worker before this patch, # doesn't after. dev test pkg/kv/kvserver \ -f TestBackpressureNotAppliedWhenReducingRangeSize \ -v --show-logs --timeout 15m --stress Release note: None --- pkg/kv/kvserver/store.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 190ea155c6f4..8e84576a13e8 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -1950,9 +1950,9 @@ func (s *Store) Start(ctx context.Context, stopper *stop.Stopper) error { s.onSpanConfigUpdate(ctx, update) }) - // When toggling between the system config span and the span configs - // infrastructure, we want to re-apply configs on all replicas from - // whatever the new source is. + // When toggling between the system config span and the span + // configs infrastructure, we want to re-apply configs on all + // replicas from whatever the new source is. spanconfigstore.EnabledSetting.SetOnChange(&s.ClusterSettings().SV, func(ctx context.Context) { enabled := spanconfigstore.EnabledSetting.Get(&s.ClusterSettings().SV) if enabled { @@ -2281,6 +2281,10 @@ func (s *Store) removeReplicaWithRangefeed(rangeID roachpb.RangeID) { // systemGossipUpdate is a callback for gossip updates to // the system config which affect range split boundaries. func (s *Store) systemGossipUpdate(sysCfg *config.SystemConfig) { + if !s.cfg.SpanConfigsDisabled && spanconfigstore.EnabledSetting.Get(&s.ClusterSettings().SV) { + return // nothing to do + } + ctx := s.AnnotateCtx(context.Background()) s.computeInitialMetrics.Do(func() { // Metrics depend in part on the system config. Compute them as soon as we