Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kv: move split and merge cluster settings to be zone/span configs #70614

Open
Tracked by #81009
data-matt opened this issue Sep 23, 2021 · 10 comments
Open
Tracked by #81009

kv: move split and merge cluster settings to be zone/span configs #70614

data-matt opened this issue Sep 23, 2021 · 10 comments
Assignees
Labels
A-zone-configs 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

@data-matt
Copy link

data-matt commented Sep 23, 2021

Is your feature request related to a problem? Please describe.

For hot ranges, we suggest tuning cluster level settings:

  • kv.range_split.by_load_enabled
  • kv.range_split.load_qps_threshold
  • kv.range_merge.queue_enabled

However, these settings will have impact on all workloads across the cluster, whilst the hot ranges issue appears within individual databases/tables. From a blast radius perspective:

  • We don’t want change these settings and impact a whole cluster at once.
  • Different workloads (against different tables) might want these settings to be tuned differently.

This shows up in internal incidents as well: https://github.com/cockroachlabs/support/issues/1590.

Describe the solution you'd like

We want to be able to tweak these settings on a per-schema object level. We already have per-schema object configurations provided through zone configs; we could introduce new fields there. We could also audit other cluster settings that would benefit from the same treatment.

Jira issue: CRDB-10140
Epic: CRDB-26686

@data-matt data-matt added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Sep 23, 2021
@irfansharif irfansharif changed the title kv tuning per database/table sql,kv: allow cluster setting tuning on a per database/table level Sep 30, 2021
@irfansharif
Copy link
Contributor

Edited some of the issue text. This work becomes more relevant in the context of secondary tenants looking to fine-tune performance in the same way, except they don't have access to the kind of privileged cluster settings to let them do so (for the blast radius they can have). Given our recent work to support zone configs for secondary tenants (#67679), building on top of it to fold in these tuning knobs seems like the right way to go.

@irfansharif
Copy link
Contributor

It'd be nice to also be able to more selectively set closed timestamp target durations for individual rangefeed. For internal subsystems that make use of rangefeeds (spanconfig.KVSubscriber from #69614) is a recent one, we probably want a very tight target duration. This would reduce how long it takes for a zone config to be fully applied, something that may come in handy if we decide to poll for it to provide better guarantees (ala #71977). Yet another setting that can be applied on a per-schema level of granularity is whether or not rangefeeds are enabled for the given table (+cc @ajwerner).

@irfansharif
Copy link
Contributor

Going to move this to the SQL schema board so it has an owner, this feels worth doing (+cc @ajwerner).

@ajwerner
Copy link
Contributor

For future readers: this issue is not about the general concept of cluster settings and instead it notices that there are some cluster settings which control KV configuration and can be applied on a per-range basis. These could instead be propagated in a more targeted way to the key-value store via the span configuration subsystem.

@data-matt
Copy link
Author

Yes @ajwerner that would be great, only settings that make sense to be set at a lower level :)

@ajwerner
Copy link
Contributor

Maybe @mwang1026 or @devadvocado, y'all want to get your heads together and decide whether we want to do this? Relates also to supporting range feed/changefeed per table (there's an issue for that somewhere I can dig up).

@mwang1026 mwang1026 added the O-postmortem Originated from a Postmortem action item. label May 27, 2022
@exalate-issue-sync exalate-issue-sync bot removed the O-postmortem Originated from a Postmortem action item. label Mar 1, 2023
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-schema-deprecated Use T-sql-foundations instead label May 10, 2023
@exalate-issue-sync exalate-issue-sync bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 10, 2023
@dt
Copy link
Member

dt commented Jun 21, 2023

I think we should close this, or reframe it to be specifically "change some kv configurations to be zone/span configs instead of cluster settings". Cluster settings are just that, cluster settings, and are doing what they're supposed to do, but are being used in a place where a different mechanism (zone configs) would be a better fit.

@knz
Copy link
Contributor

knz commented Jun 26, 2023

Relevant Slack thread: https://cockroachlabs.slack.com/archives/C0KB9Q03D/p1687779261954189

It appears the span config infra is already able to enable rangefeeds on a per-table basis. So we can expose this via zone configs to end-users.

@knz
Copy link
Contributor

knz commented Jun 26, 2023

Idea from @dt:

Do we actually even need zone/span configs for rangefeed? can we not just derive the fact it is enabled/disabled by the fact a client is connected to it or not?

Further discussion in aforementioned slack thread.

@dt dt changed the title sql,kv: allow cluster setting tuning on a per database/table level kv: move split and merge cluster settings to be zone/span configs Jun 26, 2023
@dt
Copy link
Member

dt commented Jun 26, 2023

I've retitled the issue to be specific to the settings named in it since as I mentioned above, cluster settings themselves are working as intended, just being used for something that doesn't want to be cluster-wide in these cases.

@blathers-crl blathers-crl bot added the T-kv KV Team label Jun 26, 2023
@exalate-issue-sync exalate-issue-sync bot removed the T-kv KV Team label Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-zone-configs 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)
Projects
None yet
Development

No branches or pull requests

6 participants