Skip to content

Commit b606fd4

Browse files
rafissrafiss
committed
server,kvtenant: remove usages of gossip.KeyDeprecatedSystemConfig
This key was only needed in the 21.2/22.2 mixed version state. Removing it and the related machinery allows us to make kvtenant.connector stop implementing SystemConfigProvider. The key should not be reused until it's entirely deleted from the gossip network of existing clusters, so for now the string constant remains as a placeholder. Release note: None
1 parent 19290ae commit b606fd4

14 files changed

Lines changed: 22 additions & 228 deletions

File tree

‎pkg/cli/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ go_library(
121121
"//pkg/cloud/impl:cloudimpl",
122122
"//pkg/cloud/userfile",
123123
"//pkg/clusterversion",
124-
"//pkg/config",
125124
"//pkg/docs",
126125
"//pkg/geo/geos",
127126
"//pkg/gossip",

‎pkg/cli/cliflags/flags.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,13 +1277,6 @@ File containing the JSON output from a node's /_status/gossip/ endpoint.
12771277
If specified, takes priority over host/port flags.`,
12781278
}
12791279

1280-
PrintSystemConfig = FlagInfo{
1281-
Name: "print-system-config",
1282-
Description: `
1283-
If specified, print the system config contents. Beware that the output will be
1284-
long and not particularly human-readable.`,
1285-
}
1286-
12871280
DecodeAsTable = FlagInfo{
12881281
Name: "decode-as-table",
12891282
Description: `

‎pkg/cli/context.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,6 @@ var debugCtx struct {
450450
replicated bool
451451
inputFile string
452452
ballastSize storagepb.SizeSpec
453-
printSystemConfig bool
454453
maxResults int
455454
decodeAsTableDesc string
456455
verbose bool
@@ -469,7 +468,6 @@ func setDebugContextDefaults() {
469468
debugCtx.inputFile = ""
470469
debugCtx.ballastSize = storagepb.SizeSpec{Capacity: 1000000000}
471470
debugCtx.maxResults = 0
472-
debugCtx.printSystemConfig = false
473471
debugCtx.decodeAsTableDesc = ""
474472
debugCtx.verbose = false
475473
debugCtx.keyTypes = showAll

‎pkg/cli/debug.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
3030
"github.com/cockroachdb/cockroach/pkg/cli/syncbench"
3131
"github.com/cockroachdb/cockroach/pkg/cloud"
32-
"github.com/cockroachdb/cockroach/pkg/config"
3332
"github.com/cockroachdb/cockroach/pkg/gossip"
3433
"github.com/cockroachdb/cockroach/pkg/keys"
3534
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
@@ -1069,16 +1068,6 @@ func parseGossipValues(gossipInfo *gossip.InfoStatus) (string, error) {
10691068
return "", errors.Wrapf(err, "failed to parse value for key %q", key)
10701069
}
10711070
output = append(output, fmt.Sprintf("%q: %v", key, clusterID))
1072-
} else if key == gossip.KeyDeprecatedSystemConfig {
1073-
if debugCtx.printSystemConfig {
1074-
var config config.SystemConfigEntries
1075-
if err := protoutil.Unmarshal(bytes, &config); err != nil {
1076-
return "", errors.Wrapf(err, "failed to parse value for key %q", key)
1077-
}
1078-
output = append(output, fmt.Sprintf("%q: %+v", key, config))
1079-
} else {
1080-
output = append(output, fmt.Sprintf("%q: omitted", key))
1081-
}
10821071
} else if key == gossip.KeyFirstRangeDescriptor {
10831072
var desc roachpb.RangeDescriptor
10841073
if err := protoutil.Unmarshal(bytes, &desc); err != nil {

‎pkg/cli/flags.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -989,7 +989,6 @@ func init() {
989989
{
990990
f := debugGossipValuesCmd.Flags()
991991
cliflagcfg.StringFlag(f, &debugCtx.inputFile, cliflags.GossipInputFile)
992-
cliflagcfg.BoolFlag(f, &debugCtx.printSystemConfig, cliflags.PrintSystemConfig)
993992
}
994993
{
995994
f := debugBallastCmd.Flags()

‎pkg/gossip/infostore_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func TestInfoStoreMostDistant(t *testing.T) {
376376
scInfo := is.newInfo(nil, time.Second)
377377
scInfo.Hops = 100
378378
scInfo.NodeID = nodes[0]
379-
if err := is.addInfo(KeyDeprecatedSystemConfig, scInfo); err != nil {
379+
if err := is.addInfo(KeyDistSQLDrainingPrefix, scInfo); err != nil {
380380
t.Fatal(err)
381381
}
382382

‎pkg/gossip/keys.go‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,19 @@ const (
5858
// bi-level key addressing scheme. The value is a roachpb.RangeDescriptor.
5959
KeyFirstRangeDescriptor = "first-range"
6060

61-
// KeyDeprecatedSystemConfig is the gossip key for the system DB span.
62-
// The value if a config.SystemConfig which holds all key/value
63-
// pairs in the system DB span.
61+
// KeyDeprecatedSystemConfig was used in the 21.2<->22.1 mixed version state,
62+
// and it's no longer used anywhere. It was the gossip key for the system DB
63+
// span. Its value was a config.SystemConfig which held all key/value pairs
64+
// in the system DB span.
6465
//
65-
// This key is used in the 21.2<->22.1 mixed version state. It is not used
66-
// in 22.1. However, it was written without a TTL, so there no guarantee
67-
// that it will actually be removed from the gossip network.
66+
// This key was written without a TTL, so there is no guarantee that it will
67+
// actually be removed from the gossip network. We would need to write a
68+
// migration to remove the data. Until then, this placeholder key should
69+
// remain to make sure the same key doesn't get reused.
6870
//
69-
// TODO(ajwerner): Write a migration to remove the data, or release a
70-
// a version which drops the key entirely, and then, in a subsequent
71-
// release, delete this key.
71+
// TODO(rafi): Write a migration to remove the data, or release a a version
72+
// which drops the key entirely, and then, in a subsequent release, delete
73+
// this key.
7274
KeyDeprecatedSystemConfig = "system-db"
7375

7476
// KeyDistSQLDrainingPrefix is the key prefix for each node's DistSQL

‎pkg/kv/kvclient/kvtenant/BUILD.bazel‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ go_library(
1111
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvtenant",
1212
visibility = ["//visibility:public"],
1313
deps = [
14-
"//pkg/config",
1514
"//pkg/config/zonepb",
1615
"//pkg/gossip",
1716
"//pkg/keys",
@@ -66,7 +65,6 @@ go_test(
6665
embed = [":kvtenant"],
6766
deps = [
6867
"//pkg/base",
69-
"//pkg/config",
7068
"//pkg/gossip",
7169
"//pkg/keys",
7270
"//pkg/kv/kvclient/kvcoord",

‎pkg/kv/kvclient/kvtenant/connector.go‎

Lines changed: 3 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"sort"
1414
"time"
1515

16-
"github.com/cockroachdb/cockroach/pkg/config"
1716
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
1817
"github.com/cockroachdb/cockroach/pkg/gossip"
1918
"github.com/cockroachdb/cockroach/pkg/keys"
@@ -128,12 +127,6 @@ type Connector interface {
128127
// OverridesMonitor provides access to tenant cluster setting overrides.
129128
settingswatcher.OverridesMonitor
130129

131-
// SystemConfigProvider provides access to basic host-tenant controlled
132-
// information regarding tenant zone configs. This is critical for the
133-
// mixed version 21.2->22.1 state where the tenant has not yet configured
134-
// its own zones.
135-
config.SystemConfigProvider
136-
137130
// GetClusterInitGracePeriodTS will return the timestamp used to signal the
138131
// end of the grace period for clusters with a license. The timestamp is
139132
// represented as the number of seconds since the Unix epoch.
@@ -181,11 +174,9 @@ type connector struct {
181174

182175
mu struct {
183176
syncutil.RWMutex
184-
client *client
185-
nodeDescs map[roachpb.NodeID]*roachpb.NodeDescriptor
186-
storeDescs map[roachpb.StoreID]*roachpb.StoreDescriptor
187-
systemConfig *config.SystemConfig
188-
systemConfigChannels map[chan<- struct{}]struct{}
177+
client *client
178+
nodeDescs map[roachpb.NodeID]*roachpb.NodeDescriptor
179+
storeDescs map[roachpb.StoreID]*roachpb.StoreDescriptor
189180
}
190181

191182
settingsMu struct {
@@ -249,12 +240,6 @@ var _ kvclient.NodeDescStore = (*connector)(nil)
249240
// requested owned by the requesting tenant?).
250241
var _ rangecache.RangeDescriptorDB = (*connector)(nil)
251242

252-
// connector is capable of providing a filtered view of the SystemConfig
253-
// containing only information applicable to secondary tenants. This obviates
254-
// the need for SQL-only tenant servers to join the cluster-wide gossip
255-
// network.
256-
var _ config.SystemConfigProvider = (*connector)(nil)
257-
258243
// connector is capable of finding debug information about the current
259244
// tenant within the cluster. This is necessary for things such as
260245
// debug zip and range reports.
@@ -295,7 +280,6 @@ func NewConnector(cfg ConnectorConfig, addrs []string) Connector {
295280

296281
c.mu.nodeDescs = make(map[roachpb.NodeID]*roachpb.NodeDescriptor)
297282
c.mu.storeDescs = make(map[roachpb.StoreID]*roachpb.StoreDescriptor)
298-
c.mu.systemConfigChannels = make(map[chan<- struct{}]struct{})
299283
c.settingsMu.allTenantOverrides = make(map[settings.InternalKey]settings.EncodedValue)
300284
c.settingsMu.specificOverrides = make(map[settings.InternalKey]settings.EncodedValue)
301285
c.settingsMu.notifyCh = make(chan struct{})
@@ -454,8 +438,6 @@ var gossipSubsHandlers = map[string]func(*connector, context.Context, string, ro
454438
gossip.MakePrefixPattern(gossip.KeyNodeDescPrefix): (*connector).updateNodeAddress,
455439
// Subscribe to all *StoreDescriptor updates.
456440
gossip.MakePrefixPattern(gossip.KeyStoreDescPrefix): (*connector).updateStoreMap,
457-
// Subscribe to a filtered view of *SystemConfig updates.
458-
gossip.KeyDeprecatedSystemConfig: (*connector).updateSystemConfig,
459441
}
460442

461443
var gossipSubsPatterns = func() []string {
@@ -548,59 +530,6 @@ func (c *connector) GetStoreDescriptor(storeID roachpb.StoreID) (*roachpb.StoreD
548530
return desc, nil
549531
}
550532

551-
// updateSystemConfig handles updates to a filtered view of the "system-db"
552-
// gossip key, performing the corresponding update to the connector's cached
553-
// SystemConfig.
554-
func (c *connector) updateSystemConfig(ctx context.Context, key string, content roachpb.Value) {
555-
cfg := config.NewSystemConfig(c.defaultZoneCfg)
556-
if err := content.GetProto(&cfg.SystemConfigEntries); err != nil {
557-
log.Errorf(ctx, "could not unmarshal system config: %v", err)
558-
return
559-
}
560-
561-
c.mu.Lock()
562-
defer c.mu.Unlock()
563-
c.mu.systemConfig = cfg
564-
for c := range c.mu.systemConfigChannels {
565-
select {
566-
case c <- struct{}{}:
567-
default:
568-
}
569-
}
570-
}
571-
572-
// GetSystemConfig implements the config.SystemConfigProvider interface.
573-
func (c *connector) GetSystemConfig() *config.SystemConfig {
574-
// TODO(nvanbenschoten): we need to wait in `(*connector).Start()` until the
575-
// system config is populated. As is, there's a small chance that we return
576-
// nil, which SQL does not handle.
577-
c.mu.RLock()
578-
defer c.mu.RUnlock()
579-
return c.mu.systemConfig
580-
}
581-
582-
// RegisterSystemConfigChannel implements the config.SystemConfigProvider
583-
// interface.
584-
func (c *connector) RegisterSystemConfigChannel() (_ <-chan struct{}, unregister func()) {
585-
// Create channel that receives new system config notifications. The channel
586-
// has a size of 1 to prevent connector from having to block on it.
587-
ch := make(chan struct{}, 1)
588-
589-
c.mu.Lock()
590-
defer c.mu.Unlock()
591-
c.mu.systemConfigChannels[ch] = struct{}{}
592-
593-
// Notify the channel right away if we have a config.
594-
if c.mu.systemConfig != nil {
595-
ch <- struct{}{}
596-
}
597-
return ch, func() {
598-
c.mu.Lock()
599-
defer c.mu.Unlock()
600-
delete(c.mu.systemConfigChannels, ch)
601-
}
602-
}
603-
604533
// RangeLookup implements the kvcoord.RangeDescriptorDB interface.
605534
func (c *connector) RangeLookup(
606535
ctx context.Context, key roachpb.RKey, rc rangecache.RangeLookupConsistency, useReverseScan bool,
@@ -1111,16 +1040,6 @@ func AddressResolver(s kvclient.NodeDescStore) nodedialer.AddressResolver {
11111040
}
11121041
}
11131042

1114-
// GossipSubscriptionSystemConfigMask filters a system config down to just the
1115-
// keys that a tenant SQL servers needs access to. All system tenant objects are
1116-
// filtered out (e.g. system tenant descriptors and users).
1117-
var GossipSubscriptionSystemConfigMask = config.MakeSystemConfigMask(
1118-
// Tenant SQL servers need just enough of the zone hierarchy to understand
1119-
// which zone configurations apply to their keyspace.
1120-
config.MakeZoneKey(keys.SystemSQLCodec, keys.RootNamespaceID),
1121-
config.MakeZoneKey(keys.SystemSQLCodec, keys.TenantsRangesID),
1122-
)
1123-
11241043
// CombineKVAddresses combines the remote and loopback addresses into a single
11251044
// slice, while aldo de-duplicating the loopback address if it's present in the
11261045
// remote addresses.

‎pkg/kv/kvclient/kvtenant/connector_test.go‎

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"testing"
1414
"time"
1515

16-
"github.com/cockroachdb/cockroach/pkg/config"
1716
"github.com/cockroachdb/cockroach/pkg/gossip"
1817
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord"
1918
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
@@ -201,18 +200,6 @@ func gossipEventForStoreDesc(desc *roachpb.StoreDescriptor) *kvpb.GossipSubscrip
201200
}
202201
}
203202

204-
func gossipEventForSystemConfig(cfg *config.SystemConfigEntries) *kvpb.GossipSubscriptionEvent {
205-
val, err := protoutil.Marshal(cfg)
206-
if err != nil {
207-
panic(err)
208-
}
209-
return &kvpb.GossipSubscriptionEvent{
210-
Key: gossip.KeyDeprecatedSystemConfig,
211-
Content: roachpb.MakeValueFromBytesAndTimestamp(val, hlc.Timestamp{}),
212-
PatternMatched: gossip.KeyDeprecatedSystemConfig,
213-
}
214-
}
215-
216203
func waitForNodeDesc(t *testing.T, c *connector, nodeID roachpb.NodeID) {
217204
t.Helper()
218205
testutils.SucceedsSoon(t, func() error {
@@ -234,7 +221,7 @@ func newConnector(cfg ConnectorConfig, addrs []string) *connector {
234221
}
235222

236223
// TestConnectorGossipSubscription tests connector's roles as a
237-
// kvclient.NodeDescStore and as a config.SystemConfigProvider.
224+
// kvclient.NodeDescStore.
238225
func TestConnectorGossipSubscription(t *testing.T) {
239226
defer leaktest.AfterTest(t)()
240227
defer log.Scope(t).Close(t)
@@ -255,11 +242,10 @@ func TestConnectorGossipSubscription(t *testing.T) {
255242
gossipSubC := make(chan *kvpb.GossipSubscriptionEvent)
256243
defer close(gossipSubC)
257244
gossipSubFn := func(req *kvpb.GossipSubscriptionRequest, stream kvpb.Internal_GossipSubscriptionServer) error {
258-
assert.Len(t, req.Patterns, 4)
245+
assert.Len(t, req.Patterns, 3)
259246
assert.Equal(t, "cluster-id", req.Patterns[0])
260247
assert.Equal(t, "node:.*", req.Patterns[1])
261248
assert.Equal(t, "store:.*", req.Patterns[2])
262-
assert.Equal(t, "system-db", req.Patterns[3])
263249
for gossipSub := range gossipSubC {
264250
if err := stream.Send(gossipSub); err != nil {
265251
return err
@@ -354,42 +340,6 @@ func TestConnectorGossipSubscription(t *testing.T) {
354340
desc, err = c.GetNodeDescriptor(3)
355341
require.Equal(t, node3, desc)
356342
require.NoError(t, err)
357-
358-
// Test config.SystemConfigProvider impl. Should not have a SystemConfig yet.
359-
sysCfg := c.GetSystemConfig()
360-
require.Nil(t, sysCfg)
361-
sysCfgC, _ := c.RegisterSystemConfigChannel()
362-
require.Len(t, sysCfgC, 0)
363-
364-
// Return first SystemConfig response.
365-
sysCfgEntries := &config.SystemConfigEntries{Values: []roachpb.KeyValue{
366-
{Key: roachpb.Key("a")},
367-
{Key: roachpb.Key("b")},
368-
}}
369-
gossipSubC <- gossipEventForSystemConfig(sysCfgEntries)
370-
371-
// Test config.SystemConfigProvider impl. Wait for update first.
372-
<-sysCfgC
373-
sysCfg = c.GetSystemConfig()
374-
require.NotNil(t, sysCfg)
375-
require.Equal(t, sysCfgEntries.Values, sysCfg.Values)
376-
377-
// Return updated SystemConfig response.
378-
sysCfgEntriesUp := &config.SystemConfigEntries{Values: []roachpb.KeyValue{
379-
{Key: roachpb.Key("a")},
380-
{Key: roachpb.Key("c")},
381-
}}
382-
gossipSubC <- gossipEventForSystemConfig(sysCfgEntriesUp)
383-
384-
// Test config.SystemConfigProvider impl. Wait for update first.
385-
<-sysCfgC
386-
sysCfg = c.GetSystemConfig()
387-
require.NotNil(t, sysCfg)
388-
require.Equal(t, sysCfgEntriesUp.Values, sysCfg.Values)
389-
390-
// A newly registered SystemConfig channel will be immediately notified.
391-
sysCfgC2, _ := c.RegisterSystemConfigChannel()
392-
require.Len(t, sysCfgC2, 1)
393343
}
394344

395345
// TestConnectorGossipSubscription tests connector's role as a
@@ -495,11 +445,10 @@ func TestConnectorRetriesUnreachable(t *testing.T) {
495445
gossipEventForNodeDesc(node2),
496446
}
497447
gossipSubFn := func(req *kvpb.GossipSubscriptionRequest, stream kvpb.Internal_GossipSubscriptionServer) error {
498-
assert.Len(t, req.Patterns, 4)
448+
assert.Len(t, req.Patterns, 3)
499449
assert.Equal(t, "cluster-id", req.Patterns[0])
500450
assert.Equal(t, "node:.*", req.Patterns[1])
501451
assert.Equal(t, "store:.*", req.Patterns[2])
502-
assert.Equal(t, "system-db", req.Patterns[3])
503452
for _, event := range gossipSubEvents {
504453
if err := stream.Send(event); err != nil {
505454
return err

0 commit comments

Comments
 (0)