From 62f12374ed65c72330db7dff44c7d84e8aeec726 Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Wed, 15 Sep 2021 17:29:57 -0400 Subject: [PATCH] spanconfig: introduce spanconfig.StoreWriter (and its impl) In #69172 we introduced a spanconfig.StoreReader interface to abstract away the gossiped system config span. We motivated that PR by teasing a future implementation of the same interface, an in-memory data structure to maintain a mapping between between spans and configs (powered through a view over system.span_configurations introduced in \#69047). This PR introduces just that. Intended (future) usages: - #69614 introduces the KVWatcher interface, listening in on system.span_configurations. The updates generated by it will be used to populate per-store instantiations of this data structure, with an eye towards providing a "drop-in" replacement of the gossiped system config span (conveniently implementing the sibling spanconfig.StoreReader interface). - #69661 introduces the SQLWatcher interface, listening in on changes to system.{descriptor,zones} and generating denormalized span config updates for every descriptor/zone config change. These updates will need to be diffed against a spanconfig.StoreWriter populated with the existing contents of KVAccessor to generate the "targeted" diffs KVAccessor expects. Release note: None --- pkg/BUILD.bazel | 1 + pkg/config/zonepb/zone_test.go | 5 + pkg/kv/kvclient/kvcoord/split_test.go | 2 +- pkg/kv/kvserver/queue_concurrency_test.go | 2 +- pkg/kv/kvserver/split_queue_test.go | 2 +- pkg/kv/kvserver/store.go | 5 - pkg/roachpb/span_config.go | 20 ++ pkg/spanconfig/spanconfig.go | 108 +++++- pkg/spanconfig/spanconfigstore/BUILD.bazel | 35 ++ pkg/spanconfig/spanconfigstore/shadow.go | 91 +++++ pkg/spanconfig/spanconfigstore/store.go | 273 +++++++++++++++ pkg/spanconfig/spanconfigstore/store_test.go | 325 ++++++++++++++++++ pkg/spanconfig/spanconfigstore/testdata/basic | 95 +++++ .../spanconfigstore/testdata/internal | 42 +++ .../spanconfigstore/testdata/overlap | 88 +++++ 15 files changed, 1075 insertions(+), 19 deletions(-) create mode 100644 pkg/spanconfig/spanconfigstore/BUILD.bazel create mode 100644 pkg/spanconfig/spanconfigstore/shadow.go create mode 100644 pkg/spanconfig/spanconfigstore/store.go create mode 100644 pkg/spanconfig/spanconfigstore/store_test.go create mode 100644 pkg/spanconfig/spanconfigstore/testdata/basic create mode 100644 pkg/spanconfig/spanconfigstore/testdata/internal create mode 100644 pkg/spanconfig/spanconfigstore/testdata/overlap diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 303322b5098e..8a794d39038d 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -180,6 +180,7 @@ ALL_TESTS = [ "//pkg/settings:settings_test", "//pkg/spanconfig/spanconfigkvaccessor:spanconfigkvaccessor_test", "//pkg/spanconfig/spanconfigmanager:spanconfigmanager_test", + "//pkg/spanconfig/spanconfigstore:spanconfigstore_test", "//pkg/sql/catalog/catalogkeys:catalogkeys_test", "//pkg/sql/catalog/catalogkv:catalogkv_test", "//pkg/sql/catalog/catformat:catformat_test", diff --git a/pkg/config/zonepb/zone_test.go b/pkg/config/zonepb/zone_test.go index df9189d33709..2ac12278f5b2 100644 --- a/pkg/config/zonepb/zone_test.go +++ b/pkg/config/zonepb/zone_test.go @@ -1429,3 +1429,8 @@ func TestZoneConfigToSpanConfigConversion(t *testing.T) { require.Equal(t, tc.expectSpanConfig, spanConfig) } } + +func TestDefaultZoneAndSpanConfigs(t *testing.T) { + converted := DefaultZoneConfigRef().AsSpanConfig() + require.True(t, converted.Equal(roachpb.TestingDefaultSpanConfig())) +} diff --git a/pkg/kv/kvclient/kvcoord/split_test.go b/pkg/kv/kvclient/kvcoord/split_test.go index 9552061812f5..dca4e2759c7f 100644 --- a/pkg/kv/kvclient/kvcoord/split_test.go +++ b/pkg/kv/kvclient/kvcoord/split_test.go @@ -175,7 +175,7 @@ func TestRangeSplitsWithWritePressure(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) // Override default span config. - cfg := kvserver.TestingDefaultSpanConfig() + cfg := roachpb.TestingDefaultSpanConfig() cfg.RangeMaxBytes = 1 << 18 // Manually create the local test cluster so that the split queue diff --git a/pkg/kv/kvserver/queue_concurrency_test.go b/pkg/kv/kvserver/queue_concurrency_test.go index ac1ac6ce0cf6..637ab85e1828 100644 --- a/pkg/kv/kvserver/queue_concurrency_test.go +++ b/pkg/kv/kvserver/queue_concurrency_test.go @@ -71,7 +71,7 @@ func TestBaseQueueConcurrent(t *testing.T) { cfg: StoreConfig{ Clock: hlc.NewClock(hlc.UnixNano, time.Second), AmbientCtx: log.AmbientContext{Tracer: tracing.NewTracer()}, - DefaultSpanConfig: TestingDefaultSpanConfig(), + DefaultSpanConfig: roachpb.TestingDefaultSpanConfig(), }, } diff --git a/pkg/kv/kvserver/split_queue_test.go b/pkg/kv/kvserver/split_queue_test.go index 889bb76f084f..7b232da6482c 100644 --- a/pkg/kv/kvserver/split_queue_test.go +++ b/pkg/kv/kvserver/split_queue_test.go @@ -89,7 +89,7 @@ func TestSplitQueueShouldQueue(t *testing.T) { repl.mu.Lock() repl.mu.state.Stats = &enginepb.MVCCStats{KeyBytes: test.bytes} repl.mu.Unlock() - conf := TestingDefaultSpanConfig() + conf := roachpb.TestingDefaultSpanConfig() conf.RangeMaxBytes = test.maxBytes repl.SetSpanConfig(conf) diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index e3401bcb5bf4..7039d975eda5 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -2972,8 +2972,3 @@ func min(a, b int) int { } return b } - -// TestingDefaultSpanConfig exposes the default span config for testing purposes. -func TestingDefaultSpanConfig() roachpb.SpanConfig { - return zonepb.DefaultZoneConfigRef().AsSpanConfig() -} diff --git a/pkg/roachpb/span_config.go b/pkg/roachpb/span_config.go index 4ab884bc7fd0..546c137bb844 100644 --- a/pkg/roachpb/span_config.go +++ b/pkg/roachpb/span_config.go @@ -98,3 +98,23 @@ func (c ConstraintsConjunction) String() string { } return sb.String() } + +// TestingDefaultSpanConfig exports the default span config for testing purposes. +func TestingDefaultSpanConfig() SpanConfig { + return SpanConfig{ + RangeMinBytes: 128 << 20, // 128 MB + RangeMaxBytes: 512 << 20, // 512 MB + // Use 25 hours instead of the previous 24 to make users successful by + // default. Users desiring to take incremental backups every 24h may + // incorrectly assume that the previous default 24h was sufficient to do + // that. But the equation for incremental backups is: + // GC TTLSeconds >= (desired backup interval) (time to perform incremental backup) + // We think most new users' incremental backups will complete within an + // hour, and larger clusters will have more experienced operators and will + // understand how to change these settings if needed. + GCPolicy: GCPolicy{ + TTLSeconds: 25 * 60 * 60, + }, + NumReplicas: 3, + } +} diff --git a/pkg/spanconfig/spanconfig.go b/pkg/spanconfig/spanconfig.go index 58997d41d354..2917bdf889da 100644 --- a/pkg/spanconfig/spanconfig.go +++ b/pkg/spanconfig/spanconfig.go @@ -47,24 +47,110 @@ type ReconciliationDependencies interface { // through the KVAccessor. } -// Store is a data structure used to store span configs. +// Store is a data structure used to store spans and their corresponding +// configs. type Store interface { + StoreWriter StoreReader - - // TODO(irfansharif): We'll want to add a StoreWriter interface here once we - // implement a data structure to store span configs. We expect this data - // structure to be used in KV to eventually replace the use of the - // gossip-backed system config span. } -// Silence the unused linter. -var _ Store = nil +// StoreWriter is the write-only portion of the Store interface. +type StoreWriter interface { + // Apply applies the given update[1]. It also returns the existing spans that + // were deleted and entries that were newly added to make room for the + // update. The deleted list can double as a list of overlapping spans in the + // Store, provided the update is not a no-op[2]. + // + // Span configs are stored in non-overlapping fashion. When an update + // overlaps with existing configs, the existing configs are deleted. If the + // overlap is only partial, the non-overlapping components of the existing + // configs are re-added. If the update itself is adding an entry, that too + // is added. This is best illustrated with the following example: + // + // [--- X --) is a span with config X + // + // Store | [--- A ----)[------------- B -----------)[---------- C -----) + // Update | [------------------ D -------------) + // | + // Deleted | [------------- B -----------)[---------- C -----) + // Added | [------------------ D -------------)[--- C -----) + // Store* | [--- A ----)[------------------ D -------------)[--- C -----) + // + // TODO(irfansharif): We'll make use of the dryrun option in a future PR + // when wiring up the reconciliation job to use the KVAccessor. Since the + // KVAccessor is a "targeted" API (the spans being deleted/upserted + // have to already be present with the exact same bounds), we'll dryrun an + // update against a StoreWriter (pre-populated with the entries present in + // KV) to generate the targeted deletes and upserts we'd need to issue. + // After successfully installing them in KV, we can keep our StoreWriter + // up-to-date by actually applying the update. + // + // There's also the question of a "full reconciliation pass". We'll be + // generating updates reactively listening in on changes to + // system.{descriptor,zones} (see SQLWatcher). It's possible then for a + // suspended tenant's table history to be GC-ed away and for its SQLWatcher + // to never detect that a certain table/index/partition has been deleted. + // Left as is, this results in us never issuing a corresponding span config + // deletion request. We'd be leaving a bunch of delete-able span configs + // lying around, and a bunch of empty ranges as a result of those. A "full + // reconciliation pass" is our attempt to find all these extraneous entries + // in KV and to delete them. + // + // We can use a StoreWriter here too (one that's pre-populated with the + // contents of KVAccessor, as before). We'd iterate through all descriptors, + // find all overlapping spans, issue KVAccessor deletes for them, and upsert + // the descriptor's span config[3]. As for the StoreWriter itself, we'd + // simply delete the overlapping entries. After iterating through all the + // descriptors, we'd finally issue KVAccessor deletes for all span configs + // still remaining in the Store. + // + // TODO(irfansharif): The descriptions above presume holding the entire set + // of span configs in memory, but we could break away from that by adding + // pagination + retrieval limit to the GetSpanConfigEntriesFor API. We'd + // then paginate through chunks of the keyspace at a time, do a "full + // reconciliation pass" over just that chunk, and continue. + // + // [1]: Unless dryrun is true. We'll still generate the same {deleted,added} + // lists. + // [2]: We could alternatively expose a GetAllOverlapping() API to make + // things clearer. + // [3]: We could skip the delete + upsert dance if the descriptor's exact + // span config entry already exists in KV. Using Apply (dryrun=true) + // against a StoreWriter (populated using KVAccessor contents) using + // the descriptor's span config entry would return empty lists, + // indicating a no-op. + Apply(ctx context.Context, update Update, dryrun bool) ( + deleted []roachpb.Span, added []roachpb.SpanConfigEntry, + ) +} -// StoreReader is the read-only portion of the Store interface. It's an adaptor -// interface implemented by config.SystemConfig to let us later swap out the -// source with one backed by a view of `system.span_configurations`. +// StoreReader is the read-only portion of the Store interface. It doubles as an +// adaptor interface for config.SystemConfig. type StoreReader interface { NeedsSplit(ctx context.Context, start, end roachpb.RKey) bool ComputeSplitKey(ctx context.Context, start, end roachpb.RKey) roachpb.RKey GetSpanConfigForKey(ctx context.Context, key roachpb.RKey) (roachpb.SpanConfig, error) } + +// Update captures what span has seen a config change. It will be the unit of +// what a {SQL,KV}Watcher emits, and what can be applied to a StoreWriter. +type Update struct { + // Span captures the key span being updated. + Span roachpb.Span + + // Config captures the span config the key span was updated to. An empty + // config indicates the span config being deleted. + Config roachpb.SpanConfig +} + +// Deletion returns true if the update corresponds to a span config being +// deleted. +func (u Update) Deletion() bool { + return u.Config.IsEmpty() +} + +// Addition returns true if the update corresponds to a span config being +// added. +func (u Update) Addition() bool { + return !u.Deletion() +} diff --git a/pkg/spanconfig/spanconfigstore/BUILD.bazel b/pkg/spanconfig/spanconfigstore/BUILD.bazel new file mode 100644 index 000000000000..17547f542d2f --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/BUILD.bazel @@ -0,0 +1,35 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "spanconfigstore", + srcs = [ + "shadow.go", + "store.go", + ], + importpath = "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigstore", + visibility = ["//visibility:public"], + deps = [ + "//pkg/keys", + "//pkg/roachpb:with-mocks", + "//pkg/spanconfig", + "//pkg/util/interval", + "//pkg/util/log", + "//pkg/util/syncutil", + "@com_github_cockroachdb_errors//:errors", + ], +) + +go_test( + name = "spanconfigstore_test", + srcs = ["store_test.go"], + data = glob(["testdata/**"]), + embed = [":spanconfigstore"], + deps = [ + "//pkg/roachpb:with-mocks", + "//pkg/spanconfig", + "//pkg/util/leaktest", + "//pkg/util/randutil", + "@com_github_cockroachdb_datadriven//:datadriven", + "@com_github_stretchr_testify//require", + ], +) diff --git a/pkg/spanconfig/spanconfigstore/shadow.go b/pkg/spanconfig/spanconfigstore/shadow.go new file mode 100644 index 000000000000..c9625f19fdeb --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/shadow.go @@ -0,0 +1,91 @@ +// Copyright 2021 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package spanconfigstore + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/errors" +) + +// ShadowReader wraps around two spanconfig.StoreReaders and logs warnings (if +// expensive logging is enabled) when there are divergent results from the two. +type ShadowReader struct { + new, old spanconfig.StoreReader +} + +// NewShadowReader instantiates a new shadow reader. +func NewShadowReader(new, old spanconfig.StoreReader) *ShadowReader { + return &ShadowReader{ + new: new, + old: old, + } +} + +var _ = NewShadowReader // defeat the unused linter. + +var _ spanconfig.StoreReader = &ShadowReader{} + +// NeedsSplit is part of the spanconfig.StoreReader interface. +func (s *ShadowReader) NeedsSplit(ctx context.Context, start, end roachpb.RKey) bool { + newResult := s.new.NeedsSplit(ctx, start, end) + if log.ExpensiveLogEnabled(ctx, 1) { + oldResult := s.old.NeedsSplit(ctx, start, end) + if newResult != oldResult { + log.Warningf(ctx, "needs split: mismatched responses between old result (%t) and new (%t) for start=%s end=%s", + oldResult, newResult, start.String(), end.String()) + } + } + + return newResult +} + +// ComputeSplitKey is part of the spanconfig.StoreReader interface. +func (s *ShadowReader) ComputeSplitKey(ctx context.Context, start, end roachpb.RKey) roachpb.RKey { + newResult := s.new.ComputeSplitKey(ctx, start, end) + if log.ExpensiveLogEnabled(ctx, 1) { + oldResult := s.old.ComputeSplitKey(ctx, start, end) + if !newResult.Equal(oldResult) { + str := func(k roachpb.RKey) string { + if len(k) == 0 { + return "" + } + return k.String() + } + + log.Warningf(ctx, "compute split key: mismatched responses between old result (%s) and new (%s) for start=%s end=%s", + str(oldResult), str(newResult), str(start), str(end)) + } + } + return newResult +} + +// GetSpanConfigForKey is part of the spanconfig.StoreReader interface. +func (s *ShadowReader) GetSpanConfigForKey( + ctx context.Context, key roachpb.RKey, +) (roachpb.SpanConfig, error) { + newResult, errNew := s.new.GetSpanConfigForKey(ctx, key) + if log.ExpensiveLogEnabled(ctx, 1) { + oldResult, errOld := s.old.GetSpanConfigForKey(ctx, key) + if !newResult.Equal(oldResult) { + log.Warningf(ctx, "get span config for key: mismatched responses between old result (%s) and new(%s) for key=%s", + oldResult.String(), newResult.String(), key.String()) + } + if !errors.Is(errNew, errOld) { + log.Warningf(ctx, "get span config for key: mismatched errors between old result (%s) and new (%s) for key=%s", + errOld, errNew, key.String()) + } + } + return newResult, errNew +} diff --git a/pkg/spanconfig/spanconfigstore/store.go b/pkg/spanconfig/spanconfigstore/store.go new file mode 100644 index 000000000000..0f3d357fd7c3 --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/store.go @@ -0,0 +1,273 @@ +// Copyright 2021 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package spanconfigstore + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/util/interval" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/syncutil" +) + +// Store is an in-memory data structure to store and retrieve span configs. +// Internally it makes use of an interval tree to store non-overlapping span +// configs. +type Store struct { + mu struct { + syncutil.RWMutex + tree interval.Tree + idAlloc int64 + } + + // TODO(irfansharif): We're using a static fall back span config here, we + // could instead have this track the host tenant's RANGE DEFAULT config, or + // go a step further and use the tenant's own RANGE DEFAULT instead if the + // key is within the tenant's keyspace. We'd have to thread that through the + // KVAccessor interface by reserving special keys for these default configs. + + // fallback is the span config we'll fall back on in the absence of + // something more specific. + fallback roachpb.SpanConfig +} + +var _ spanconfig.Store = &Store{} + +// New instantiates a span config store with the given fallback. +func New(fallback roachpb.SpanConfig) *Store { + s := &Store{fallback: fallback} + s.mu.tree = interval.NewTree(interval.ExclusiveOverlapper) + return s +} + +// NeedsSplit is part of the spanconfig.StoreReader interface. +func (s *Store) NeedsSplit(ctx context.Context, start, end roachpb.RKey) bool { + return len(s.ComputeSplitKey(ctx, start, end)) > 0 +} + +// ComputeSplitKey is part of the spanconfig.StoreReader interface. +func (s *Store) ComputeSplitKey(ctx context.Context, start, end roachpb.RKey) roachpb.RKey { + sp := roachpb.Span{Key: start.AsRawKey(), EndKey: end.AsRawKey()} + + // We don't want to split within the system config span while we're still + // also using it to disseminate zone configs. + // + // TODO(irfansharif): Once we've fully phased out the system config span, we + // can get rid of this special handling. + if keys.SystemConfigSpan.Contains(sp) { + return nil + } + if keys.SystemConfigSpan.ContainsKey(sp.Key) { + return roachpb.RKey(keys.SystemConfigSpan.EndKey) + } + + s.mu.RLock() + defer s.mu.RUnlock() + + idx := 0 + var splitKey roachpb.RKey = nil + s.mu.tree.DoMatching(func(i interval.Interface) (done bool) { + if idx > 0 { + splitKey = roachpb.RKey(i.(*storeEntry).Span.Key) + return true // we found our split key, we're done + } + + idx++ + return false // more + }, sp.AsRange()) + + return splitKey +} + +// GetSpanConfigForKey is part of the spanconfig.StoreReader interface. +func (s *Store) GetSpanConfigForKey( + ctx context.Context, key roachpb.RKey, +) (roachpb.SpanConfig, error) { + sp := roachpb.Span{Key: key.AsRawKey(), EndKey: key.Next().AsRawKey()} + + s.mu.RLock() + defer s.mu.RUnlock() + + var conf roachpb.SpanConfig + found := false + s.mu.tree.DoMatching(func(i interval.Interface) (done bool) { + conf = i.(*storeEntry).Config + found = true + return true + }, sp.AsRange()) + + if !found { + if log.ExpensiveLogEnabled(ctx, 1) { + log.Warningf(ctx, "span config not found for %s", key.String()) + } + conf = s.fallback + } + return conf, nil +} + +// Apply is part of the spanconfig.StoreWriter interface. +func (s *Store) Apply( + ctx context.Context, update spanconfig.Update, dryrun bool, +) (deleted []roachpb.Span, added []roachpb.SpanConfigEntry) { + s.mu.Lock() + defer s.mu.Unlock() + + if !update.Span.Valid() || len(update.Span.EndKey) == 0 { + log.Fatalf(ctx, "invalid span") + } + + entriesToDelete, entriesToAdd := s.accumulateOpsForLocked(update) + + deleted = make([]roachpb.Span, len(entriesToDelete)) + for i := range entriesToDelete { + entry := &entriesToDelete[i] + if !dryrun { + if err := s.mu.tree.Delete(entry, false); err != nil { + log.Fatalf(ctx, "%v", err) + } + } + deleted[i] = entry.Span + } + + added = make([]roachpb.SpanConfigEntry, len(entriesToAdd)) + for i := range entriesToAdd { + entry := &entriesToAdd[i] + if !dryrun { + if err := s.mu.tree.Insert(entry, false); err != nil { + log.Fatalf(ctx, "%v", err) + } + } + added[i] = entry.SpanConfigEntry + } + + return deleted, added +} + +// accumulateOpsForLocked returns the list of store entries that would be +// deleted and added if the given update was to be applied. To apply a given +// update, we want to find all overlapping spans and clear out just the +// intersections. If the update is adding a new span config, we'll also want to +// add it store entry after. We do this by deleting all overlapping spans in +// their entirety and re-adding the non-overlapping portions, if any. +// Pseudo-code: +// +// for entry in store.overlapping(update.span): +// union, intersection = union(update.span, entry), intersection(update.span, entry) +// pre, post = span{union.start_key, intersection.start_key}, span{intersection.end_key, union.end_key} +// +// delete entry +// if entry.contains(update.span.start_key): +// add pre=entry.conf +// if entry.contains(update.span.end_key): +// add post=entry.conf +// +// if adding: +// add update.span=update.conf +// +func (s *Store) accumulateOpsForLocked(update spanconfig.Update) (toDelete, toAdd []storeEntry) { + for _, overlapping := range s.mu.tree.Get(update.Span.AsRange()) { + existing := overlapping.(*storeEntry) + var ( + union = existing.Span.Combine(update.Span) + inter = existing.Span.Intersect(update.Span) + + pre = roachpb.Span{Key: union.Key, EndKey: inter.Key} + post = roachpb.Span{Key: inter.EndKey, EndKey: union.EndKey} + ) + + // Delete the existing span in its entirety. Below we'll re-add the + // non-intersecting parts of the span. + toDelete = append(toDelete, *existing) + + if existing.Span.ContainsKey(update.Span.Key) { // existing entry contains the update span's start key + // ex: [-----------------) + // + // up: [-------) + // up: [-------------) + // up: [-------------- + // up: [-------) + // up: [-----------------) + // up: [------------------ + + // Re-add the non-intersecting span, if any. + if pre.Valid() { + toAdd = append(toAdd, s.makeEntryLocked(pre, existing.Config)) + } + } + + if existing.Span.ContainsKey(update.Span.EndKey) { // existing entry contains the update span's end key + // ex: [-----------------) + // + // up: -------------) + // up: [------------) + // up: [---------) + + // Re-add the non-intersecting span. + toAdd = append(toAdd, s.makeEntryLocked(post, existing.Config)) + } + } + + if update.Addition() { + if len(toDelete) == 1 && + toDelete[0].Span.Equal(update.Span) && + toDelete[0].Config.Equal(update.Config) { + // We're deleting exactly what we're going to add, this is a no-op. + return nil, nil + } + + // Add the update itself. + toAdd = append(toAdd, s.makeEntryLocked(update.Span, update.Config)) + + // TODO(irfansharif): If we're adding an entry, we could inspect the + // entries before and after and check whether either of them have the + // same config. If they do, we could coalesce them into a single span. + // Given that these boundaries determine where we split ranges, we'd be + // able to reduce the number of ranges drastically (think adjacent + // tables/indexes/partitions with the same config). This would be + // especially significant for secondary tenants, where we'd be able to + // avoid unconditionally splitting on table boundaries. We'd still want + // to split on tenant boundaries, so certain preconditions would need to + // hold. For performance reasons, we'd probably also want to offer + // a primitive to allow manually splitting on specific table boundaries. + } + + return toDelete, toAdd +} + +func (s *Store) makeEntryLocked(sp roachpb.Span, conf roachpb.SpanConfig) storeEntry { + s.mu.idAlloc++ + return storeEntry{ + SpanConfigEntry: roachpb.SpanConfigEntry{Span: sp, Config: conf}, + id: s.mu.idAlloc, + } +} + +// storeEntry is the type used to store and sort values in the span config +// store. +type storeEntry struct { + roachpb.SpanConfigEntry + id int64 +} + +var _ interval.Interface = &storeEntry{} + +// Range implements interval.Interface. +func (s *storeEntry) Range() interval.Range { + return s.Span.AsRange() +} + +// ID implements interval.Interface. +func (s *storeEntry) ID() uintptr { + return uintptr(s.id) +} diff --git a/pkg/spanconfig/spanconfigstore/store_test.go b/pkg/spanconfig/spanconfigstore/store_test.go new file mode 100644 index 000000000000..3341cf33b552 --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/store_test.go @@ -0,0 +1,325 @@ +// Copyright 2021 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package spanconfigstore + +import ( + "context" + "fmt" + "math/rand" + "regexp" + "strings" + "testing" + + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/randutil" + "github.com/cockroachdb/datadriven" + "github.com/stretchr/testify/require" +) + +// spanRe matches strings of the form "[start, end)", capturing both the "start" +// and "end" keys. +var spanRe = regexp.MustCompile(`^\[(\w+),\s??(\w+)\)$`) + +// configRe matches a single word. It's a shorthand for declaring a unique +// config. +var configRe = regexp.MustCompile(`^(\w+)$`) + +func TestSpanRe(t *testing.T) { + for _, tc := range []struct { + input string + expMatch bool + expStart, expEnd string + }{ + {"[a, b)", true, "a", "b"}, + {"[acd, bfg)", true, "acd", "bfg"}, // multi character keys allowed + {"[a,b)", true, "a", "b"}, // separating space is optional + {"[ a,b) ", false, "", ""}, // extraneous spaces disallowed + {"[a,b ) ", false, "", ""}, // extraneous spaces disallowed + {"[a,, b)", false, "", ""}, // only single comma allowed + {" [a, b)", false, "", ""}, // need to start with '[' + {"[a,b)x", false, "", ""}, // need to end with ')' + } { + require.Equalf(t, tc.expMatch, spanRe.MatchString(tc.input), "input = %s", tc.input) + if !tc.expMatch { + continue + } + + matches := spanRe.FindStringSubmatch(tc.input) + require.Len(t, matches, 3) + start, end := matches[1], matches[2] + require.Equal(t, tc.expStart, start) + require.Equal(t, tc.expEnd, end) + } +} + +// parseSpan is helper function that constructs a roachpb.Span from a string of +// the form "[start, end)". +func parseSpan(t *testing.T, sp string) roachpb.Span { + if !spanRe.MatchString(sp) { + t.Fatalf("expected %s to match span regex", sp) + } + + matches := spanRe.FindStringSubmatch(sp) + start, end := matches[1], matches[2] + return roachpb.Span{ + Key: roachpb.Key(start), + EndKey: roachpb.Key(end), + } +} + +// parseConfig is helper function that constructs a roachpb.SpanConfig that's +// "tagged" with the given string (i.e. a constraint with the given string a +// required key). +func parseConfig(t *testing.T, conf string) roachpb.SpanConfig { + if !configRe.MatchString(conf) { + t.Fatalf("expected %s to match config regex", conf) + } + return roachpb.SpanConfig{ + Constraints: []roachpb.ConstraintsConjunction{ + { + Constraints: []roachpb.Constraint{ + { + Key: conf, + }, + }, + }, + }, + } +} + +// printSpan is a helper function that transforms roachpb.Span into a string of +// the form "[start,end)". The span is assumed to have been constructed by the +// parseSpan helper above. +func printSpan(sp roachpb.Span) string { + return fmt.Sprintf("[%s,%s)", string(sp.Key), string(sp.EndKey)) +} + +// printSpanConfig is a helper function that transforms roachpb.SpanConfig into +// a readable string. The span config is assumed to have been constructed by the +// parseSpanConfig helper above. +func printSpanConfig(conf roachpb.SpanConfig) string { + return conf.Constraints[0].Constraints[0].Key // see parseConfig for what a "tagged" roachpb.SpanConfig translates to +} + +// printSpanConfigEntry is a helper function that transforms +// roachpb.SpanConfigEntry into a string of the form "[start, end):config". The +// span and config are expected to have been constructed using the +// parse{Span,Config} helpers above. +func printSpanConfigEntry(entry roachpb.SpanConfigEntry) string { + return fmt.Sprintf("%s:%s", printSpan(entry.Span), printSpanConfig(entry.Config)) +} + +// TestingGetAllOverlapping is a testing only helper to retrieve the set of +// overlapping entries in sorted order. +func (s *Store) TestingGetAllOverlapping( + _ context.Context, sp roachpb.Span, +) []roachpb.SpanConfigEntry { + s.mu.RLock() + defer s.mu.RUnlock() + + // Iterate over all overlapping ranges and return corresponding span config + // entries. + var res []roachpb.SpanConfigEntry + for _, overlapping := range s.mu.tree.Get(sp.AsRange()) { + res = append(res, overlapping.(*storeEntry).SpanConfigEntry) + } + return res +} + +func TestDatadriven(t *testing.T) { + defer leaktest.AfterTest(t)() + + ctx := context.Background() + datadriven.Walk(t, "testdata", func(t *testing.T, path string) { + store := New(parseConfig(t, "FALLBACK")) + datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { + var ( + spanStr, confStr, keyStr string + ) + switch d.Cmd { + case "set": + d.ScanArgs(t, "span", &spanStr) + d.ScanArgs(t, "conf", &confStr) + span, config := parseSpan(t, spanStr), parseConfig(t, confStr) + + dryrun := d.HasArg("dryrun") + deleted, added := store.Apply(ctx, spanconfig.Update{Span: span, Config: config}, dryrun) + + var b strings.Builder + for _, sp := range deleted { + b.WriteString(fmt.Sprintf("deleted %s\n", printSpan(sp))) + } + for _, ent := range added { + b.WriteString(fmt.Sprintf("added %s\n", printSpanConfigEntry(ent))) + } + return b.String() + + case "delete": + d.ScanArgs(t, "span", &spanStr) + span := parseSpan(t, spanStr) + + dryrun := d.HasArg("dryrun") + deleted, added := store.Apply(ctx, spanconfig.Update{Span: span}, dryrun) + + var b strings.Builder + for _, sp := range deleted { + b.WriteString(fmt.Sprintf("deleted %s\n", printSpan(sp))) + } + for _, ent := range added { + b.WriteString(fmt.Sprintf("added %s\n", printSpanConfigEntry(ent))) + } + return b.String() + + case "get": + d.ScanArgs(t, "key", &keyStr) + config, err := store.GetSpanConfigForKey(ctx, roachpb.RKey(keyStr)) + require.NoError(t, err) + return fmt.Sprintf("conf=%s", printSpanConfig(config)) + + case "needs-split": + d.ScanArgs(t, "span", &spanStr) + span := parseSpan(t, spanStr) + start, end := roachpb.RKey(span.Key), roachpb.RKey(span.EndKey) + result := store.NeedsSplit(ctx, start, end) + return fmt.Sprintf("%t", result) + + case "compute-split": + d.ScanArgs(t, "span", &spanStr) + span := parseSpan(t, spanStr) + start, end := roachpb.RKey(span.Key), roachpb.RKey(span.EndKey) + splitKey := store.ComputeSplitKey(ctx, start, end) + return fmt.Sprintf("key=%s", string(splitKey)) + + case "overlapping": + d.ScanArgs(t, "span", &spanStr) + span := parseSpan(t, spanStr) + entries := store.TestingGetAllOverlapping(ctx, span) + var results []string + for _, entry := range entries { + results = append(results, printSpanConfigEntry(entry)) + } + return strings.Join(results, "\n") + + default: + } + + t.Fatalf("unknown command: %s", d.Cmd) + return "" + }) + }) +} + +// TestRandomized randomly sets/deletes span configs for arbitrary keyspans +// within some alphabet. For a test span, it then asserts that the config we +// retrieve is what we expect to find from the store. It also ensures that all +// ranges are non-overlapping. +func TestRandomized(t *testing.T) { + defer leaktest.AfterTest(t)() + + randutil.SeedForTests() + ctx := context.Background() + alphabet := "abcdefghijklmnopqrstuvwxyz" + configs := "ABCDEF" + ops := []string{"set", "del"} + + genRandomSpan := func() roachpb.Span { + startIdx, endIdx := rand.Intn(len(alphabet)-1), 1+rand.Intn(len(alphabet)-1) + if startIdx == endIdx { + endIdx = (endIdx + 1) % len(alphabet) + } + if endIdx < startIdx { + startIdx, endIdx = endIdx, startIdx + } + spanStr := fmt.Sprintf("[%s, %s)", string(alphabet[startIdx]), string(alphabet[endIdx])) + sp := parseSpan(t, spanStr) + require.True(t, sp.Valid()) + return sp + } + + getRandomConf := func() roachpb.SpanConfig { + confStr := fmt.Sprintf("conf_%s", string(configs[rand.Intn(len(configs))])) + return parseConfig(t, confStr) + } + + getRandomOp := func() string { + return ops[rand.Intn(2)] + } + + testSpan := parseSpan(t, "[f,g)") // pin a single character span to test with + var expConfig roachpb.SpanConfig + var expFound bool + + const numOps = 5000 + store := New(roachpb.TestingDefaultSpanConfig()) + for i := 0; i < numOps; i++ { + sp, conf, op := genRandomSpan(), getRandomConf(), getRandomOp() + switch op { + case "set": + store.Apply(ctx, spanconfig.Update{Span: sp, Config: conf}, false) + if testSpan.Overlaps(sp) { + expConfig, expFound = conf, true + } + case "del": + store.Apply(ctx, spanconfig.Update{Span: sp}, false) + if testSpan.Overlaps(sp) { + expConfig, expFound = roachpb.SpanConfig{}, false + } + default: + t.Fatalf("unexpected op: %s", op) + } + } + + overlappingConfigs := store.TestingGetAllOverlapping(ctx, testSpan) + if !expFound { + require.Len(t, overlappingConfigs, 0) + } else { + // Check to see that the set of overlapping span configs is exactly what + // we expect. + require.Len(t, overlappingConfigs, 1) + gotSpan, gotConfig := overlappingConfigs[0].Span, overlappingConfigs[0].Config + + require.Truef(t, gotSpan.Contains(testSpan), + "improper result: expected retrieved span (%s) to contain test span (%s)", + printSpan(gotSpan), printSpan(testSpan)) + + require.Truef(t, expConfig.Equal(gotConfig), + "mismatched configs: expected %s, got %s", + printSpanConfig(expConfig), printSpanConfig(gotConfig)) + + // Ensure that the config accessed through the StoreReader interface is + // the same as above. + storeReaderConfig, err := store.GetSpanConfigForKey(ctx, roachpb.RKey(testSpan.Key)) + require.NoError(t, err) + require.True(t, gotConfig.Equal(storeReaderConfig)) + } + + var last roachpb.SpanConfigEntry + everythingSpan := parseSpan(t, fmt.Sprintf("[%s,%s)", + string(alphabet[0]), string(alphabet[len(alphabet)-1]))) + for i, cur := range store.TestingGetAllOverlapping(ctx, everythingSpan) { + if i == 0 { + last = cur + continue + } + + // Span configs are returned in strictly sorted order. + require.True(t, last.Span.Key.Compare(cur.Span.Key) < 0, + "expected to find spans in strictly sorted order, found %s then %s", + printSpan(last.Span), printSpan(cur.Span)) + + // Span configs must also be non-overlapping. + require.Falsef(t, last.Span.Overlaps(cur.Span), + "expected non-overlapping spans, found %s and %s", + printSpan(last.Span), printSpan(cur.Span)) + } +} diff --git a/pkg/spanconfig/spanconfigstore/testdata/basic b/pkg/spanconfig/spanconfigstore/testdata/basic new file mode 100644 index 000000000000..2f894d29d056 --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/testdata/basic @@ -0,0 +1,95 @@ +# Test basic get/set/delete operations where the spans retrieved are identical +# to the ones being added/deleted, and are non-overlapping with respect to one +# another. + +# Check that missing keys fallback to a static config. +get key=b +---- +conf=FALLBACK + + +# Test that dryruns don't actually mutate anything. +set span=[b,d) conf=A dryrun +---- +added [b,d):A + +get key=b +---- +conf=FALLBACK + + +# Add span configs for real. +set span=[b,d) conf=A +---- +added [b,d):A + +set span=[f,h) conf=B +---- +added [f,h):B + + +# Check that a no-op operation shows up as much. +set span=[f,h) conf=B +---- + + +# Check that a few keys are as we'd expect. +get key=b +---- +conf=A + +get key=c +---- +conf=A + +get key=f +---- +conf=B + +get key=g +---- +conf=B + +get key=h +---- +conf=FALLBACK + + +# Check that a delete dryrun does nothing. +delete span=[f,h) dryrun +---- +deleted [f,h) + +get key=f +---- +conf=B + + +# Delete a span for real. +delete span=[f,h) +---- +deleted [f,h) + +# Check that a no-op operation does nothing. +delete span=[f,g) +---- + +delete span=[f,h) +---- + +# Check that keys are as we'd expect (including the deleted one). +get key=b +---- +conf=A + +get key=c +---- +conf=A + +get key=f +---- +conf=FALLBACK + +get key=g +---- +conf=FALLBACK diff --git a/pkg/spanconfig/spanconfigstore/testdata/internal b/pkg/spanconfig/spanconfigstore/testdata/internal new file mode 100644 index 000000000000..5cffaf80713c --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/testdata/internal @@ -0,0 +1,42 @@ +# Test the store's internal view of overlapping span configs. + +overlapping span=[a,z) +---- + +set span=[b,d) conf=A +---- +added [b,d):A + +set span=[f,g) conf=B +---- +added [f,g):B + +overlapping span=[b,d) +---- +[b,d):A + +overlapping span=[b,g) +---- +[b,d):A +[f,g):B + +overlapping span=[b,j) +---- +[b,d):A +[f,g):B + +overlapping span=[a,j) +---- +[b,d):A +[f,g):B + +delete span=[f,g) +---- +deleted [f,g) + +overlapping span=[f,g) +---- + +overlapping span=[b,j) +---- +[b,d):A diff --git a/pkg/spanconfig/spanconfigstore/testdata/overlap b/pkg/spanconfig/spanconfigstore/testdata/overlap new file mode 100644 index 000000000000..486357ed7e7f --- /dev/null +++ b/pkg/spanconfig/spanconfigstore/testdata/overlap @@ -0,0 +1,88 @@ +# Test operations where the spans overlap with the existing ones. + +set span=[b,h) conf=A +---- +added [b,h):A + + +# Check that writing a span with a partial overlap first deletes the existing +# entry and adds three new ones. +set span=[d,f) conf=B +---- +deleted [b,h) +added [b,d):A +added [f,h):A +added [d,f):B + +overlapping span=[b,h) +---- +[b,d):A +[d,f):B +[f,h):A + + +# Check that writing a span that partially overlaps with multiple existing +# entries deletes all of them, and re-adds the right non-overlapping fragments +# with the right configs. +set span=[c,e) conf=C +---- +deleted [b,d) +deleted [d,f) +added [b,c):A +added [e,f):B +added [c,e):C + +overlapping span=[b,h) +---- +[b,c):A +[c,e):C +[e,f):B +[f,h):A + +# Check that when a span being written to entirely envelopes an existing entry, +# that entry is deleted in its entirety. +delete span=[d,g) +---- +deleted [c,e) +deleted [e,f) +deleted [f,h) +added [c,d):C +added [g,h):A + +overlapping span=[b,h) +---- +[b,c):A +[c,d):C +[g,h):A + +# Validate that the right split points (span start keys) are surfaced. +needs-split span=[b,h) +---- +true + +compute-split span=[b,h) +---- +key=c + +set span=[b,g) conf=A +---- +deleted [b,c) +deleted [c,d) +added [b,g):A + +overlapping span=[b,h) +---- +[b,g):A +[g,h):A + +needs-split span=[b,h) +---- +true + +compute-split span=[b,h) +---- +key=g + +needs-split span=[h,z) +---- +false