From 84b90a3c42a24fac525bd39720d249c5085c57c6 Mon Sep 17 00:00:00 2001 From: Rahul Aggarwal Date: Mon, 14 Aug 2023 10:26:53 -0400 Subject: [PATCH 1/3] go.mod: bump Pebble to 77e81e806c8b 77e81e80 pebble: Update Tokenbucket package and use WaitCtx 18e6ad42 pebble: Export keyspan.Fragmenter f9f63ef2 crossversion: add more comments Release note: --- DEPS.bzl | 12 ++++++------ build/bazelutil/distdir_files.bzl | 4 ++-- go.mod | 4 ++-- go.sum | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 50f4a569dbfb..3f179d9138df 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -1595,10 +1595,10 @@ def go_deps(): patches = [ "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch", ], - sha256 = "e8b3aac1060cd50b6339104fb50df94fc0442b8424883668f8badb488e769091", - strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230809191252-58a05cd4082c", + sha256 = "fea9844c8d46dd2c0bde835e0b329a94b855a12c37cb857cca08db0f9e7e9152", + strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230811190520-77e81e806c8b", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230809191252-58a05cd4082c.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230811190520-77e81e806c8b.zip", ], ) go_repository( @@ -1645,10 +1645,10 @@ def go_deps(): name = "com_github_cockroachdb_tokenbucket", build_file_proto_mode = "disable_global", importpath = "github.com/cockroachdb/tokenbucket", - sha256 = "7711efac97ce89c704e84b30c6c73887040d8ba50ce5b286e7113f4af7012339", - strip_prefix = "github.com/cockroachdb/tokenbucket@v0.0.0-20230613231145-182959a1fad6", + sha256 = "150f3e8e5b515c0886cda0809f09b5d5173d7f2c30eb2f2c6045c2aeb2183aa3", + strip_prefix = "github.com/cockroachdb/tokenbucket@v0.0.0-20230807174530-cc333fc44b06", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tokenbucket/com_github_cockroachdb_tokenbucket-v0.0.0-20230613231145-182959a1fad6.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tokenbucket/com_github_cockroachdb_tokenbucket-v0.0.0-20230807174530-cc333fc44b06.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index f8c5eb265417..6241eebb1bb5 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -320,14 +320,14 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230809191252-58a05cd4082c.zip": "e8b3aac1060cd50b6339104fb50df94fc0442b8424883668f8badb488e769091", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230811190520-77e81e806c8b.zip": "fea9844c8d46dd2c0bde835e0b329a94b855a12c37cb857cca08db0f9e7e9152", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220803192808-1806698b1b7b.zip": "3fda531795c600daf25532a4f98be2a1335cd1e5e182c72789bca79f5f69fcc1", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tablewriter/com_github_cockroachdb_tablewriter-v0.0.5-0.20200105123400-bd15540e8847.zip": "79daf1c29ec50cdd8dd1ea33f8a814963646a45a2ebe22742d652579340ebde0", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/teamcity/com_github_cockroachdb_teamcity-v0.0.0-20180905144921-8ca25c33eb11.zip": "9df6b028c9fb5bff7bdad844bda504356945fd6d3cd583c50f68d8b8e85060f6", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tokenbucket/com_github_cockroachdb_tokenbucket-v0.0.0-20230613231145-182959a1fad6.zip": "7711efac97ce89c704e84b30c6c73887040d8ba50ce5b286e7113f4af7012339", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tokenbucket/com_github_cockroachdb_tokenbucket-v0.0.0-20230807174530-cc333fc44b06.zip": "150f3e8e5b515c0886cda0809f09b5d5173d7f2c30eb2f2c6045c2aeb2183aa3", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/tools/com_github_cockroachdb_tools-v0.0.0-20211112185054-642e51449b40.zip": "37a3737dd23768b4997b2f0341d625658f5862cdbf808f7fbf3a7f9fd25913a7", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/ttycolor/com_github_cockroachdb_ttycolor-v0.0.0-20210902133924-c7d7dcdde4e8.zip": "1260533510c89abd6d8af573a40f0246f6865d5091144dea509b2c48e7c61614", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/vitess/com_github_cockroachdb_vitess-v0.0.0-20210218160543-54524729cc82.zip": "71f14e67f9396930d978d85c47b853f5cc4ce340e53cf88bf7d731b8428b2f77", diff --git a/go.mod b/go.mod index afc1333fe14e..4f55a76fc666 100644 --- a/go.mod +++ b/go.mod @@ -116,11 +116,11 @@ require ( github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55 github.com/cockroachdb/gostdlib v1.19.0 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b - github.com/cockroachdb/pebble v0.0.0-20230809191252-58a05cd4082c + github.com/cockroachdb/pebble v0.0.0-20230811190520-77e81e806c8b github.com/cockroachdb/redact v1.1.5 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b - github.com/cockroachdb/tokenbucket v0.0.0-20230613231145-182959a1fad6 + github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 github.com/cockroachdb/tools v0.0.0-20211112185054-642e51449b40 github.com/cockroachdb/ttycolor v0.0.0-20210902133924-c7d7dcdde4e8 github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd diff --git a/go.sum b/go.sum index 75c3ac4d0080..8228d27d3d23 100644 --- a/go.sum +++ b/go.sum @@ -493,8 +493,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE= github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs= -github.com/cockroachdb/pebble v0.0.0-20230809191252-58a05cd4082c h1:+m0pDsTerDJ8TMs46GiEAVHq2Doy/r1cwEf/EcT3dxw= -github.com/cockroachdb/pebble v0.0.0-20230809191252-58a05cd4082c/go.mod h1:FN5O47SBEz5+kO9fG8UTR64g2WS1u5ZFCgTvxGjoSks= +github.com/cockroachdb/pebble v0.0.0-20230811190520-77e81e806c8b h1:IqusT4xdwUaQlyCLnBlMOg0LgQYh9fRn0MHyvtW+plw= +github.com/cockroachdb/pebble v0.0.0-20230811190520-77e81e806c8b/go.mod h1:EDjiaAXc0FXiRmxDzcu1wIEJ093ohHMUWxrI6iku0XA= github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30= github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg= @@ -506,8 +506,8 @@ github.com/cockroachdb/tablewriter v0.0.5-0.20200105123400-bd15540e8847 h1:c7yLg github.com/cockroachdb/tablewriter v0.0.5-0.20200105123400-bd15540e8847/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA= github.com/cockroachdb/teamcity v0.0.0-20180905144921-8ca25c33eb11 h1:UAqRo5xPCyTtZznAJ9iPVpDUFxGI0a6QWtQ8E+zwJRg= github.com/cockroachdb/teamcity v0.0.0-20180905144921-8ca25c33eb11/go.mod h1:3299Mt0Q7PkqGqbsxhvbrTpMqRyIcZ6OMw4IEmiO09g= -github.com/cockroachdb/tokenbucket v0.0.0-20230613231145-182959a1fad6 h1:DJK8W/iB+s/qkTtmXSrHA49lp5O3OsR7E6z4byOLy34= -github.com/cockroachdb/tokenbucket v0.0.0-20230613231145-182959a1fad6/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ= +github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 h1:zuQyyAKVxetITBuuhv3BI9cMrmStnpT18zmgmTxunpo= +github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ= github.com/cockroachdb/tools v0.0.0-20211112185054-642e51449b40 h1:qVTb3XEv+7VVjPetop8yGbN95HfCeqZx+VMveeSJPZw= github.com/cockroachdb/tools v0.0.0-20211112185054-642e51449b40/go.mod h1:cllxeV+TYc387/XzQRnDg6YThHoDzFewovWffzAm37Q= github.com/cockroachdb/ttycolor v0.0.0-20210902133924-c7d7dcdde4e8 h1:Hli+oX84dKq44sLVCcsGKqifm5Lg9J8VoJ2P3h9iPdI= From 682f3624c9d32f033dc6a20d01f449722d9fe73c Mon Sep 17 00:00:00 2001 From: Austen McClernon Date: Mon, 14 Aug 2023 21:34:13 +0000 Subject: [PATCH 2/3] kvserver: expect 1 server in test protected ts The `TestProtectedTimestamp` test previously used a test cluster with 3 nodes, after 0147fccb, the test only uses a single node. Update the helper function for finding a replica, to expect just 1 server. Epic: none Release note: None --- pkg/kv/kvserver/client_protectedts_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/kv/kvserver/client_protectedts_test.go b/pkg/kv/kvserver/client_protectedts_test.go index 2d30e2e1be71..def5f8116292 100644 --- a/pkg/kv/kvserver/client_protectedts_test.go +++ b/pkg/kv/kvserver/client_protectedts_test.go @@ -125,13 +125,9 @@ ORDER BY raw_start_key ASC LIMIT 1`) getStoreAndReplica := func() (*kvserver.Store, *kvserver.Replica) { startKey := getTableStartKey() - // Okay great now we have a key and can go find replicas and stores and what not. - r := tc.LookupRangeOrFatal(t, startKey) - l, _, err := tc.FindRangeLease(r, nil) - require.NoError(t, err) - - lhServer := tc.Server(int(l.Replica.NodeID) - 1) - return getFirstStoreReplica(t, lhServer, startKey) + // There's only one server, so there's no point searching for which server + // the leaseholder is on, it could only be on s0. + return getFirstStoreReplica(t, s0, startKey) } waitForRangeMaxBytes := func(maxBytes int64) { From 76512f81950ae046368ac9bbc84436d2b2885950 Mon Sep 17 00:00:00 2001 From: Austen McClernon Date: Mon, 14 Aug 2023 21:37:30 +0000 Subject: [PATCH 3/3] kvserver: disable enqueue into repl q on span cfg update Replicas were enqueued into the replicate queue, upon the store receiving a span config update which could affect the replica. The replicate queue shouldQueue is relatively more expensive than other queues. Introduce the cluster setting kv.enqueue_in_replicate_queue_on_span_config_update.enabled, which when set to true, enables queuing up replicas on span config updates; when set to false, disables queuing replicas on span config updates. By default, this settings is set to false. Resolves: #108724 Release note (ops change): Introduce the kv.enqueue_in_replicate_queue_on_span_config_update.enabled cluster setting. When set to true, stores in the cluster will enqueue replicas for replication changes, upon receiving config updates which could affect the replica. This setting is off by default. Enabling this setting speeds up how quickly config triggered replication changes begin, but adds additional CPU overhead. The overhead scales with the number of leaseholders. --- pkg/cmd/roachtest/tests/lease_preferences.go | 1 + pkg/kv/kvserver/client_protectedts_test.go | 3 +++ pkg/kv/kvserver/replicate_queue.go | 11 +++++++++++ pkg/kv/kvserver/store.go | 15 ++++++++++++--- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/roachtest/tests/lease_preferences.go b/pkg/cmd/roachtest/tests/lease_preferences.go index 9138dc96b7b9..3eaa23acf152 100644 --- a/pkg/cmd/roachtest/tests/lease_preferences.go +++ b/pkg/cmd/roachtest/tests/lease_preferences.go @@ -176,6 +176,7 @@ func runLeasePreferences( // https://github.com/cockroachdb/cockroach/issues/105274 settings := install.MakeClusterSettings() settings.ClusterSettings["server.span_stats.span_batch_limit"] = "4096" + settings.ClusterSettings["kv.enqueue_in_replicate_queue_on_span_config_update.enabled"] = "true" startNodes := func(nodes ...int) { for _, node := range nodes { diff --git a/pkg/kv/kvserver/client_protectedts_test.go b/pkg/kv/kvserver/client_protectedts_test.go index def5f8116292..92d245873b7a 100644 --- a/pkg/kv/kvserver/client_protectedts_test.go +++ b/pkg/kv/kvserver/client_protectedts_test.go @@ -72,6 +72,9 @@ func TestProtectedTimestamps(t *testing.T) { _, err = conn.Exec("SET CLUSTER SETTING kv.closed_timestamp.target_duration = '100ms'") // speeds up the test require.NoError(t, err) + _, err = conn.Exec("SET CLUSTER SETTING kv.enqueue_in_replicate_queue_on_span_config_update.enabled = true") // speeds up the test + require.NoError(t, err) + const tableRangeMaxBytes = 64 << 20 _, err = conn.Exec("ALTER TABLE foo CONFIGURE ZONE USING "+ "gc.ttlseconds = 1, range_max_bytes = $1, range_min_bytes = 1<<10;", tableRangeMaxBytes) diff --git a/pkg/kv/kvserver/replicate_queue.go b/pkg/kv/kvserver/replicate_queue.go index 1af47fa49744..9c9389d13ea6 100644 --- a/pkg/kv/kvserver/replicate_queue.go +++ b/pkg/kv/kvserver/replicate_queue.go @@ -102,6 +102,17 @@ var MinLeaseTransferInterval = settings.RegisterDurationSetting( settings.NonNegativeDuration, ) +// EnqueueInReplicateQueueOnSpanConfigUpdateEnabled controls whether replicas +// are enqueued into the replicate queue, following a span config update which +// affects the replica. +var EnqueueInReplicateQueueOnSpanConfigUpdateEnabled = settings.RegisterBoolSetting( + settings.SystemOnly, + "kv.enqueue_in_replicate_queue_on_span_config_update.enabled", + "controls whether replicas are enqueued into the replicate queue for "+ + "processing, when a span config update occurs, which affects the replica", + false, +) + var ( metaReplicateQueueAddReplicaCount = metric.Metadata{ Name: "queue.replicate.addreplica", diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 00aab57487dd..5239e1e0af05 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -2386,6 +2386,12 @@ func (s *Store) onSpanConfigUpdate(ctx context.Context, updated roachpb.Span) { now := s.cfg.Clock.NowAsClockTimestamp() + // The replicate queue has a relatively more expensive queue check + // (shouldQueue), because it scales with the number of stores, and + // performs more checks. + enqueueToReplicateQueueEnabled := EnqueueInReplicateQueueOnSpanConfigUpdateEnabled.Get( + &s.GetStoreConfig().Settings.SV) + s.mu.RLock() defer s.mu.RUnlock() if err := s.mu.replicasByKey.VisitKeyRange(ctx, sp.Key, sp.EndKey, AscendingKeyOrder, @@ -2444,9 +2450,12 @@ func (s *Store) onSpanConfigUpdate(ctx context.Context, updated roachpb.Span) { s.mergeQueue.Async(replCtx, "span config update", true /* wait */, func(ctx context.Context, h queueHelper) { h.MaybeAdd(ctx, repl, now) }) - s.replicateQueue.Async(replCtx, "span config update", true /* wait */, func(ctx context.Context, h queueHelper) { - h.MaybeAdd(ctx, repl, now) - }) + + if enqueueToReplicateQueueEnabled { + s.replicateQueue.Async(replCtx, "span config update", true /* wait */, func(ctx context.Context, h queueHelper) { + h.MaybeAdd(ctx, repl, now) + }) + } return nil // more }, ); err != nil {