docs for instance scoped system variables #7897

morgo · 2022-03-22T03:16:01Z

What is changed, added or deleted? (Required)

This adds the documentation changes described earlier in https://github.com/pingcap/tidb/blob/master/docs/design/2021-12-08-instance-scope.md#documentation-changes

In master (and 6.0) we have native support for instance scoped variables, which are now set with "SET GLOBAL" instead of "SET SESSION" (note: SET INSTANCE was never a thing.)

We don't describe instance scoped variables as "instance scoped" but rather: "Persists to cluster: No". This was intentional because INSTANCE isn't really a thing in MySQL. The property that is most understantable is if it persists to the other nodes in the cluster, and INSTANCE and GLOBAL scope are mutually exclusive.

Which TiDB version(s) do your changes apply to? (Required)

Tips for choosing the affected version(s):

By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.

For details, see tips for choosing the affected versions.

What is the related PR or file link(s)?

pingcap/tidb#32888

Do your changes match any of the following descriptions?

Delete files
Change aliases
Need modification after applied to another branch
Might cause conflicts after applied to another branch

ti-chi-bot · 2022-03-22T03:16:02Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

TomShawn
dveeden

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

morgo · 2022-03-22T03:18:24Z

system-variables.md

+- Persists to cluster: Yes
+- Default value: `ON`


This is an additional change that was picked up by the script. The default has changed in TiDB 6.0.

CbcWestwolf · 2022-03-22T06:18:51Z

LGTM

dveeden · 2022-03-22T09:52:05Z

Might be good to check after merging if any other PRs have updated this file and make sure any newly added config options also have the right information. Maybe also check for other open PRs for the same file.

morgo · 2022-03-22T14:42:02Z

Might be good to check after merging if any other PRs have updated this file and make sure any newly added config options also have the right information. Maybe also check for other open PRs for the same file.

Currently it's easier to catch it on the next auto-generated config PR. This is generated from #5720 - I would like to add it to CI at some point, but we still have 70 sysvars to document first :(

…e-variables-docs

morgo · 2022-04-14T03:31:16Z

system-variables.md

- Range: `[1, 1073741824]`
+- Range: `[1, 3600]`


This is from pingcap/tidb#33473 - it only affects master / TiDB 6.1.

morgo · 2022-04-14T03:32:56Z

system-variables.md

- Range: `[1, 2147483647]`
+- Range: `[0, 2147483647]`


This is from pingcap/tidb#30664 - it only affects master and not TIDB 6.0.

system-variables.md

morgo · 2022-04-14T03:36:44Z

system-variables.md

+- Persists to cluster: Yes
+- Default value: `OFF`


The default is wrong. It is OFF not on. See pingcap/tidb#31547 by @ekexium

(It is wrong in 6.0 and master)

morgo · 2022-04-14T03:37:21Z

system-variables.md

-### tidb_top_sql_max_meta_count (New in v6.0.0)
-
- Scope: GLOBAL
- Default value: `5000`
- Range: `[1, 10000]`
- This variable is used to control the maximum number of SQL statement types collected by [Top SQL](/dashboard/top-sql.md) per minute.
-
-### tidb_top_sql_max_time_series_count (New in v6.0.0)
-
- Scope: GLOBAL
- Default value: `100`
- Range: `[1, 5000]`
- This variable is used to control how many SQL statements that contribute the most to the load (that is, top N) can be recorded by [Top SQL](/dashboard/top-sql.md) per minute.
-
-> **Note:**
->
-> Currently, the Top SQL page in TiDB Dashboard only displays the top 5 types of SQL queries that contribute the most to the load, which is irrelevant with the configuration of `tidb_top_sql_max_time_series_count`.
-


These have just been moved. They were not in alphabetical order :(

morgo · 2022-04-14T03:38:21Z

system-variables.md

- Scope: INSTANCE
+- Scope: GLOBAL
+- Persists to cluster: No


The documentation for Instance scope variables changes to "GLOBAL + Persists = No". See: https://github.com/pingcap/tidb/blob/master/docs/design/2021-12-08-instance-scope.md

morgo · 2022-04-14T03:39:30Z

system-variables.md

+- Default value: `0`
+- Range: `[-2147483648, 0]`


This is a bug fix in pingcap/tidb#32763 - it is function neutral.

system-variables.md

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

TomShawn

LGTM

system-variables.md

TomShawn · 2022-04-26T03:31:49Z

@ran-huang Shall we merge this PR now? All contents are applicable to v6.1.

morgo · 2022-04-26T04:12:05Z

@ran-huang Shall we merge this PR now? All contents are applicable to v6.1.

If you don't merge it, followup PRs will be more complicated because more is picked up by the script.

ran-huang · 2022-04-26T05:57:04Z

@ran-huang Shall we merge this PR now? All contents are applicable to v6.1.

If you don't merge it, followup PRs will be more complicated because more is picked up by the script.

@morgo If this PR is reviewed and approved by the code owner(s), we can merge it.

I understand that the follow-up PRs will be more complicated, and we hope to merge every PR as soon as we can. I only have one concern: these changes are applicable to v6.1, but since v6.1 has not been released yet, there might be reverts or further changes to the code -- it happened a lot in the past few releases. If that happens, I'm afraid no one will remember to make changes to the docs accordingly. That's the major reason we insist that this PR be merged after code freeze.

If we merge this PR before code freeze, we'll need to get back to it and confirm there are no further changes. Can we ask for your help by then?

morgo · 2022-04-27T00:26:09Z

If we merge this PR before code freeze, we'll need to get back to it and confirm there are no further changes. Can we ask for your help by then?

Sure that's fine - these changes are generated by a script, so if they are reverted (which is rare) a subsequent script execution will update to the new values.

But I have to say that its rare we revert PRs - and a more likely outcome of this process is that it is leading to the system variables docs being out of date. All but 2 changes also apply to 6.0, but because the last PR to update the docs took too long to review ( #7737) we had to delay this to now for documenting (the changes stack on each other, and get complicated to review if we don't merge them).

bb7133

LGTM

bb7133 · 2022-04-27T15:18:15Z

But I have to say that its rare we revert PR

I agree with it.

morgo · 2022-04-27T15:23:56Z

/merge

ti-chi-bot · 2022-04-27T15:23:59Z

This pull request has been accepted and is ready to merge.

Commit hash: 010079c

yudongusa · 2022-04-28T22:00:26Z

LGTM

morgo requested review from dveeden and CbcWestwolf March 22, 2022 03:16

ti-chi-bot added missing-translation-status This PR does not have translation status info. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 22, 2022

docs for instance scoped system variables

3b8e8e4

morgo force-pushed the add-instance-scope-variables-docs branch from cf1f8ce to 3b8e8e4 Compare March 22, 2022 03:17

morgo commented Mar 22, 2022

View reviewed changes

TomShawn requested a review from bb7133 March 22, 2022 03:29

shichun-0415 assigned TomShawn Mar 22, 2022

shichun-0415 requested a review from TomShawn March 22, 2022 03:31

TomShawn requested a review from ran-huang March 22, 2022 04:02

TomShawn added the for-future-release This PR only applies to master for now and might cherry-pick to a future release. label Mar 22, 2022

CbcWestwolf mentioned this pull request Mar 22, 2022

docs-cn for instance scoped system variables pingcap/docs-cn#8725

Merged

16 tasks

TomShawn added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. and removed translation/doing This PR's assignee is translating this PR. labels Mar 22, 2022

TomShawn assigned CbcWestwolf and unassigned TomShawn Mar 22, 2022

dveeden approved these changes Mar 22, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 22, 2022

TomShawn added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2022

morgo added 2 commits April 13, 2022 21:27

Add new changes

d504b12

Merge remote-tracking branch 'upstream/master' into add-instance-scop…

bf4a34d

…e-variables-docs

morgo commented Apr 14, 2022

View reviewed changes

fix extra new line

89d61fd

morgo requested a review from seiya-annie April 14, 2022 03:42

morgo added 2 commits April 13, 2022 21:52

update links based on anchor changing

85913bf

fix anchors again

4b32677

morgo force-pushed the add-instance-scope-variables-docs branch from 3c13e2e to 4b32677 Compare April 14, 2022 04:06

morgo removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 14, 2022

TomShawn reviewed Apr 26, 2022

View reviewed changes

system-variables.md Outdated Show resolved Hide resolved

system-variables.md Outdated Show resolved Hide resolved

system-variables.md Outdated Show resolved Hide resolved

system-variables.md Show resolved Hide resolved

morgo and others added 3 commits April 25, 2022 21:05

Update system-variables.md

478f5cb

Update system-variables.md

bb93f4a

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

Update system-variables.md

718605c

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

TomShawn approved these changes Apr 26, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 26, 2022

TomShawn reviewed Apr 26, 2022

View reviewed changes

system-variables.md Show resolved Hide resolved

fix ci

010079c

bb7133 approved these changes Apr 27, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 27, 2022

ti-chi-bot merged commit bc70595 into pingcap:master Apr 27, 2022

TomShawn added requires-followup This PR requires a follow-up task after being merged. v6.1 This PR/issue applies to TiDB v6.1. and removed for-future-release This PR only applies to master for now and might cherry-pick to a future release. labels Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs for instance scoped system variables #7897

docs for instance scoped system variables #7897

morgo commented Mar 22, 2022 •

edited

Loading

ti-chi-bot commented Mar 22, 2022 •

edited

Loading

morgo Mar 22, 2022

CbcWestwolf commented Mar 22, 2022

dveeden commented Mar 22, 2022

morgo commented Mar 22, 2022

morgo Apr 14, 2022

morgo Apr 14, 2022

morgo Apr 14, 2022

morgo Apr 14, 2022

morgo Apr 14, 2022

morgo Apr 14, 2022

TomShawn left a comment

TomShawn commented Apr 26, 2022

morgo commented Apr 26, 2022

ran-huang commented Apr 26, 2022 •

edited

Loading

morgo commented Apr 27, 2022

bb7133 left a comment

bb7133 commented Apr 27, 2022

morgo commented Apr 27, 2022

ti-chi-bot commented Apr 27, 2022

yudongusa commented Apr 28, 2022

docs for instance scoped system variables #7897

docs for instance scoped system variables #7897

Conversation

morgo commented Mar 22, 2022 • edited Loading

What is changed, added or deleted? (Required)

Which TiDB version(s) do your changes apply to? (Required)

What is the related PR or file link(s)?

Do your changes match any of the following descriptions?

ti-chi-bot commented Mar 22, 2022 • edited Loading

morgo Mar 22, 2022

Choose a reason for hiding this comment

CbcWestwolf commented Mar 22, 2022

dveeden commented Mar 22, 2022

morgo commented Mar 22, 2022

morgo Apr 14, 2022

Choose a reason for hiding this comment

morgo Apr 14, 2022

Choose a reason for hiding this comment

morgo Apr 14, 2022

Choose a reason for hiding this comment

morgo Apr 14, 2022

Choose a reason for hiding this comment

morgo Apr 14, 2022

Choose a reason for hiding this comment

morgo Apr 14, 2022

Choose a reason for hiding this comment

TomShawn left a comment

Choose a reason for hiding this comment

TomShawn commented Apr 26, 2022

morgo commented Apr 26, 2022

ran-huang commented Apr 26, 2022 • edited Loading

morgo commented Apr 27, 2022

bb7133 left a comment

Choose a reason for hiding this comment

bb7133 commented Apr 27, 2022

morgo commented Apr 27, 2022

ti-chi-bot commented Apr 27, 2022

yudongusa commented Apr 28, 2022

morgo commented Mar 22, 2022 •

edited

Loading

ti-chi-bot commented Mar 22, 2022 •

edited

Loading

ran-huang commented Apr 26, 2022 •

edited

Loading