multitenant: merge serverpb.RegionsServer into serverpb.TenantStatusServer #92598
Assignees
Labels
A-multitenancy
Related to multi-tenancy
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Comments
ecwall
added
C-enhancement
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While doing work on #92293, a request came up to merge
RegionsServerandNodeLocalityServerintoTenantStatusServer: #92293 (review)Jira issue: CRDB-21878
Epic CRDB-16746
The text was updated successfully, but these errors were encountered: