reports: range not covered by spanconfig reported as under-replicated #91239
Assignees
Comments
tbg
added
C-bug
irfansharif
added a commit
to irfansharif/cockroach
that referenced
this issue
Nov 25, 2022
For this setting type: - the protoutil.Message is held in memory, - the byte representation is stored in system.settings, and - the json representation is used when accepting input and rendering state (through SHOW CLUSTER SETTING <setting-name>, the raw form is visible when looking directly at system.settings) We also use this setting type to support power a spanconfig.store.fallback_config_override, which overrides the fallback config used for ranges with no explicit span configs set. Previously we used a hardcoded value -- this makes it a bit more configurable. This is a partial and backportable workaround (read: hack) for cockroachdb#91238 and \cockroachdb#91239. In an internal escalation we observed "orphaned" ranges from dropped tables that were not being being referenced by span configs (by virtue of them originating from now-dropped tables/configs). Typically ranges of this sort are short-lived, they're emptied out through GC and merged into LHS neighbors. But if the neighboring ranges are large enough, or load just high enough, the merge does not occur. For such orphaned ranges we were using a hardcoded "fallback config", with a replication factor of three. This made for confusing semantics where if RANGE DEFAULT was configured to have a replication factor of five, our replication reports indicated there were under-replicated ranges. This is partly because replication reports today are powered by zone configs (thus looking at RANGE DEFAULT) -- this will change shortly as part of \cockroachdb#89987 where we'll instead consider span config data. In any case, we were warning users of under-replicated ranges but within KV we were not taking any action to upreplicate them -- KV was respecting the hard-coded fallback config. The issues above describe that we should apply each tenant's RANGE DEFAULT config to all such orphaned ranges, which is probably the right fix. This was alluded to in an earlier TODO but is still left for future work. // TODO(irfansharif): We're using a static[1] fallback span config // here, we could instead have this directly 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. // // [1]: Modulo the private spanconfig.store.fallback_config_override, which // applies globally. So this PR instead takes a shortcut -- it makes the static config configurable through a cluster setting. We can now do the following which alters what fallback config is applied to orphaned ranges, and in our example above, force such ranges to also have a replication factor of five. SET CLUSTER SETTING spanconfig.store.fallback_config_override = ' { "gcPolicy": {"ttlSeconds": 3600}, "numReplicas": 5, "rangeMaxBytes": "536870912", "rangeMinBytes": "134217728" }'; Release note: None
craig bot
pushed a commit
that referenced
this issue
Nov 28, 2022
92466: settings,spanconfig: introduce a protobuf setting type r=irfansharif a=irfansharif For this setting type: - the `protoutil.Message` is held in memory, - the byte representation is stored in `system.settings`, and - the json representation is used when accepting input and rendering state (through `SHOW CLUSTER SETTING <setting-name>`, the raw form is visible when looking directly at `system.settings`) We also use this setting type to support power a `spanconfig.store.fallback_config_override`, which overrides the fallback config used for ranges with no explicit span configs set. Previously we used a hardcoded value -- this makes it a bit more configurable. This is a partial and backportable workaround (read: hack) for #91238 and \#91239. In an internal escalation we observed "orphaned" ranges from dropped tables that were not being being referenced by span configs (by virtue of them originating from now-dropped tables/configs). Typically ranges of this sort are short-lived, they're emptied out through GC and merged into LHS neighbors. But if the neighboring ranges are large enough, or load just high enough, the merge does not occur. For such orphaned ranges we were using a hardcoded "fallback config", with a replication factor of three. This made for confusing semantics where if `RANGE DEFAULT` was configured to have a replication factor of five, our replication reports indicated there were under-replicated ranges. This is partly because replication reports today are powered by zone configs (thus looking at `RANGE DEFAULT`) -- this will change shortly as part of \#89987 where we'll instead consider span config data. In any case, we were warning users of under-replicated ranges but within KV we were not taking any action to upreplicate them -- KV was respecting the hard-coded fallback config. The issues above describe that we should apply each tenant's `RANGE DEFAULT` config to all such orphaned ranges, which is probably the right fix. This was alluded to in an earlier TODO but is still left for future work. ```go // TODO(irfansharif): We're using a static[1] fallback span config // here, we could instead have this directly 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. // // [1]: Modulo the private spanconfig.store.fallback_config_override, which // applies globally. ``` So this PR instead takes a shortcut -- it makes the static config configurable through a cluster setting. We can now do the following which alters what fallback config is applied to orphaned ranges, and in our example above, force such ranges to also have a replication factor of five. ```sql SET CLUSTER SETTING spanconfig.store.fallback_config_override = ' { "gcPolicy": {"ttlSeconds": 3600}, "numReplicas": 5, "rangeMaxBytes": "536870912", "rangeMinBytes": "134217728" }'; ``` Release note: None 92585: roachtest: reduce frequency of index-overload test r=irfansharif a=irfansharif Release note: None 92586: jobs: refresh job details before removing protected timestamps r=fqazi a=fqazi Fixes: #92493 Previously, we added the protected timestamps manager into the jobs frame work, which made it easier to automatically add and remove protected timestamps for jobs. Unfortunately, the Unprotect API when invoked from a clean up function never properly refreshed the job. Which could lead to a race condition trying to remove protected timestamps for schema changes. To address this, the Unprotect function will take advantage of the job update function to confirm that a refreshed copy does have protected timestamps set. Release note: None 92593: logictest: avoid a lint failure r=andreimatei a=knz Fixes #92592 The go compiler treats calls to `bazel.BuiltWithBazel()` as a compile-time constant. Therefore, ```go if !bazel.BuiltWithBazel() { skip.XXX() } ``` is considered to always terminate execution (because `skip` does its job by raising a panic), and any code coming after that is treated as dead/unreachable. 92599: multitenant: merge serverpb.RegionsServer into serverpb.TenantStatusServer r=knz a=ecwall Fixes #92598 Merge these 2 interfaces in anticipation for future work where more tenant functionality will be added to TenantStatusServer instead of creating an interface per function. Release note: None Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com> Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com> Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: Evan Wall <wall@cockroachlabs.com>
|
The code we're going to be using to generate replication reports in 23.1+ onwards is newly written to be multi-tenant aware, and more coupled to span configs: #89987. I mention it because of the following snippet: cockroach/pkg/spanconfig/spanconfigreporter/reporter.go Lines 130 to 133 in 05cbd97 What it's doing is considering the "span config" state as the authoritative state for what configuration to apply for a given range. So there's a well-defined config for "any range not covered by span configs" -- it's a hardcoded1 config (modulo 2 for 23.1, and 34 22.2 and 22.1 respectively). So these ranges will just not appear as under-replicated unless KV actually sees the need to upreplicate them, which was not true in the internal support case linked above. #91238 (also linked above) tracks the piece of work to make sure that these "unaddressed" ranges actually use the tenant's Footnotes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See https://github.com/cockroachlabs/support/issues/1858.
A range not referenced by the zone configs (result of merging a number of ranges belonging to tables that have been dropped) was using 3x replication (spanconfig default fallback) as opposed to 5x which is what the default zone config specified.
The replication reports reported this range as under-replicated, which is "not correct" (KV considers 3x the correct replication factor since that's what the spanconfig reflects). See also #91238, which inquires whether the 3x default is appropriate here.
cc @irfansharif
Jira issue: CRDB-21169
The text was updated successfully, but these errors were encountered: