pingcap / pingcap/tidb

Convert several config options to Global sysvars

Open
#33,769 7 comments 2 reactions 1 assignee Claimed by @CbcWestwolf View on GitHub
type/enhancement
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Enhancement

The TiDB cloud team has found that they need to modify the following config options, which are currently set in the TiDB configuration file. However, this is annoying since it requires all instances to be modified, and then restarted.

I checked with PM and we agreed that they are actually incorrectly config options and should be GLOBAL sysvars instead. We previously did not have such criteria for which should be a config option vs global sysvar, but we recently decided that the default should be GLOBAL sysvar. Options should only be config/instance scope if they refer to the local resources of the instance (log file locations, ports, etc.)

## First Round (v6.1)

| Config Name | Suggested Sysvar Name | Notes | Assignee | Status |
---- | -----------| ----- | ----- | -----
enable-batch-dml | tidb_enable_batch_dml | | @morgo https://github.com/pingcap/tidb/pull/33803 | Merged |
require-secure-transport | require_secure_transport | [Same as MySQL variable.](https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_require_secure_transport) | @Alkaagr81 https://github.com/pingcap/tidb/pull/34261 | Merged |
oom-action | tidb_mem_oom_action | Try and use a consistent prefix of "mem" | @ramanich1 https://github.com/pingcap/tidb/pull/34644 | Merged |
prepared-plan-cache.enabled | tidb_enable_prepared_plan_cache | | https://github.com/pingcap/tidb/pull/34790 | Merged |
prepared-plan-cache.capacity | tidb_prepared_plan_cache_size | Usually "size" is used instead of capacity. | https://github.com/pingcap/tidb/pull/34790 | Merged |
mem-quota-query | tidb_mem_quota_query | Option already exists, but is session only. Needs to change to GLOBAL | @espresso98 https://github.com/pingcap/tidb/pull/34141 | Merged |
committer-concurrency | tidb_committer_concurrency | | @espresso98 https://github.com/pingcap/tidb/pull/34368 | Merged |
query-log-max-len | tidb_query_log_max_len | Sysvar already exists. We can convert it to GLOBAL scope (confirmed by PM) | @espresso98 https://github.com/pingcap/tidb/pull/34305 | Merged |
| run-auto-analyze | tidb_enable_auto_analyze | Added Apr 29 (Discussed with PM) | @Alkaagr81 https://github.com/pingcap/tidb/pull/34643 | Merged |

## Second Round (~v6.2)

| Config Name | Suggested Sysvar Name | Notes | Assignee | Status |
---- | -----------| ----- | ----- | -----
txn-total-size-limit | tidb_txn_total_size_limit | | @Alkaagr81 https://github.com/pingcap/tidb/pull/34448 | Needs Feedback |
txn-entry-size-limit | tidb_txn_entry_size_limit | | @Alkaagr81 https://github.com/pingcap/tidb/pull/34448 | Needs Feedback |
index-usage-sync-lease | tidb_index_usage_sync_lease | | @ramanich1 | |
oom-use-tmp-storage | tidb_enable_tmp_storage_on_oom | Suggest prioritizing because of inconsistency between memory optons being split between sysvars/config |
performance.memory-usage-alarm-ratio | tidb_mem_usage_alarm_ratio | Suggest prioritizing because of inconsistency between memory optons being split between sysvars/config |

## Rejected for now

These require further discussion, or a different implementation. See also https://github.com/pingcap/tidb/issues/34960 which is for instance scoped

| Config Name | Notes |
---- | -----
alter-primary-key | Should be deprecated |
tikv-client.copr-cache.enable | Should be deprecated |

For the "Suggested Sysvar name", we should follow the [naming and usage proposal]( https://docs.google.com/document/d/1lGrfwdlv6ARcyAVrdXbQ5OYeFiVybBCcr172WCvAafI/edit?n=Sysvar_Usage_Proposal), since sysvars have different conventions to configuration options.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.