Convert several config options to Global sysvars

[morgo]

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 #33803	Merged
require-secure-transport	require_secure_transport	Same as MySQL variable.	@Alkaagr81 #34261	Merged
oom-action	tidb_mem_oom_action	Try and use a consistent prefix of "mem"	@ramanich1 #34644	Merged
prepared-plan-cache.enabled	tidb_enable_prepared_plan_cache		#34790	Merged
prepared-plan-cache.capacity	tidb_prepared_plan_cache_size	Usually "size" is used instead of capacity.	#34790	Merged
mem-quota-query	tidb_mem_quota_query	Option already exists, but is session only. Needs to change to GLOBAL	@espresso98 #34141	Merged
committer-concurrency	tidb_committer_concurrency		@espresso98 #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 #34305	Merged
run-auto-analyze	tidb_enable_auto_analyze	Added Apr 29 (Discussed with PM)	@Alkaagr81 #34643	Merged

Second Round (~v6.2)

Config Name	Suggested Sysvar Name	Notes	Assignee	Status
txn-total-size-limit	tidb_txn_total_size_limit		@Alkaagr81 #34448	Needs Feedback
txn-entry-size-limit	tidb_txn_entry_size_limit		@Alkaagr81 #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 #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, since sysvars have different conventions to configuration options.

[CbcWestwolf]

/assign

[morgo]

@CbcWestwolf I've added a column so we can assign sub-issues. There might be a lot of tests that break, so it might be easiest to fix 1-2 per PR :-)

[bb7133]

Hi @morgo @ramanich1 @espresso98 @Alkaagr81 , I'm thinking about 'combing all upgradeTo...() functions into one' for all the PRs that will be released in TiDB v6.1', say, upgradeToVer90() is introduced in #33803 and upgradeToVer91() will be introduced in #34644. Potentially we can could have one upgradeToVer90() and do the migration work for all system variables mentioned in this issue.

Pros of this idea is we could reduce the number of upgradeToVer function and the number of metadata versions. Do you think it's a good idea to do it?

[bb7133]

Also, we need to take 'SEM' into consideration. It's better to get the PMs involved for the decision. /cc @SunRunAway @easonn7

[morgo]

Hi @morgo @ramanich1 @espresso98 @Alkaagr81 , I'm thinking about 'combing all upgradeTo...() functions into one' for all the PRs that will be released in TiDB v6.1', say, upgradeToVer90() is introduced in #33803 and upgradeToVer91() will be introduced in #34644. Potentially we can could have one upgradeToVer90() and do the migration work for all system variables mentioned in this issue.

I have no problems with this change. I don't think we should add upgrade for require-secure-transport is all, since it can create a lock out scenario. The other PRs have upgrade in them as proposed, or part of this PR: #34711

[dveeden]

@CbcWestwolf maybe we should update the description for v6.6 etc?

[CbcWestwolf]

@dveeden OK, I'll do it.