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

system-variables: update from generated source #7737

Merged
merged 2 commits into from
Mar 22, 2022

Conversation

morgo
Copy link
Contributor

@morgo morgo commented Mar 2, 2022

What is changed, added or deleted? (Required)

This updates the system-variables docs, as generated by linking against the tidb source code. There are a lot of small edits, I will explain each in-line.

This only affects tidb-master safely. There might be some smaller updates which need to be cherry picked to 5.4.

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.

  • master (the latest development version)
  • v6.0 (TiDB 6.0 versions)
  • v5.4 (TiDB 5.4 versions)
  • v5.3 (TiDB 5.3 versions)
  • v5.2 (TiDB 5.2 versions)
  • v5.1 (TiDB 5.1 versions)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

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

  • This PR is translated from:
  • Other reference link(s):

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
Copy link
Member

ti-chi-bot commented Mar 2, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • TomShawn

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.

@ti-chi-bot ti-chi-bot requested a review from TomShawn March 2, 2022 23:39
@ti-chi-bot 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 2, 2022
Comment on lines -297 to -303
### placement_checks <span class="version-mark">New in v5.3.0</span>

- Scope: SESSION | GLOBAL
- Default value: `ON`
- This variable controls whether DDL statements validate [Placement Rules in SQL](/placement-rules-in-sql.md).
- It is intended to be used by logical dump/restore tools to ensure that tables can always be created even if placement rules are violated. This is similar to how mysqldump writes `SET FOREIGN_KEY_CHECKS=0;` to the start of every dump file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable has been removed in TiDB 5.4. A new variable tidb_placement_mode exists instead (I will document it separately).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change will be in 5.4.1? i checked in 5.4.0, 'placement_checks' still exist, and no 'tidb_placement_mode' in system variable list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the PR: pingcap/tidb#31093

It looks like it was created before 5.4 code freeze, but merged after. So my understanding was not correct - it is a 6.0 only change.

Comment on lines -523 to +516
- Scope: SESSION | GLOBAL
- Scope: GLOBAL
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change is intentional and only affects master. See: https://github.com/pingcap/tidb/blob/master/docs/design/2021-12-08-instance-scope.md

Comment on lines -662 to -671
### tidb_enable_alter_placement

> **Warning:**
>
> Currently, Placement Rules in SQL is an experimental feature. It is not recommended that you use it in production environments.

- Scope: GLOBAL
- Default value: `OFF`
- This variable enables or disables [Placement Rules in SQL](/placement-rules-in-sql.md).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an experimental flag, but the feature is being declared stable in master.

@@ -779,6 +758,10 @@ Constraint checking is always performed in place for pessimistic transactions (d
>
> - Since v5.4.0, for a newly deployed TiDB cluster, this variable is enabled by default.

- Scope: SESSION | GLOBAL
- Default value: `ON`
- This variable is used to control whether to enable the index merge feature.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just moves the comment above the feature specification, to be consistent. We generate it this way in every other case.

Comment on lines -847 to +830
- Scope: SESSION | GLOBAL
- Scope: GLOBAL
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change is intentional and only affects master. See: https://github.com/pingcap/tidb/blob/master/docs/design/2021-12-08-instance-scope.md

Comment on lines +874 to +883
### tidb_enable_top_sql <span class="version-mark">New in v5.4.0</span>

> **Warning:**
>
> Currently, Top SQL is an experimental feature. It is not recommended that you use it for production environments.

- Scope: GLOBAL
- Default value: `OFF`
- This variable is used to control whether to enable the [Top SQL](/dashboard/top-sql.md) feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was previously at the bottom of the page (non alphabetical order). Chronological order doesn't make sense to users, so the order is fixed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default value of tidb_enable_top_sql is "ON" in v6.0.0

Comment on lines -1457 to +1450
- Default value: `0`
- Default value: ""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According the the source, the default is "". But it might be a bug: pingcap/tidb#32763

In either case, it's correct right now - but it should be changed in future to be numeric. This will also help improve the docs as the type is shown.

Comment on lines +1562 to +1564
### tidb_stats_load_pseudo_timeout <span class="version-mark">New in v5.4.0</span>

> **WARNING:**
> **Warning:**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two variables were in the wrong alphabetical order. Warning is also in upper case (inconsistent).

Comment on lines -1593 to -1620
- Scope: SESSION | GLOBAL
- Scope: GLOBAL
- Default value: `24`
- Range: `[0, 255]`
- This variable is used to set the history capacity of [statement summary tables](/statement-summary-tables.md).

### tidb_stmt_summary_internal_query <span class="version-mark">New in v4.0</span>

- Scope: SESSION | GLOBAL
- Scope: GLOBAL
- Default value: `OFF`
- This variable is used to control whether to include the SQL information of TiDB in [statement summary tables](/statement-summary-tables.md).

### tidb_stmt_summary_max_sql_length <span class="version-mark">New in v4.0</span>

- Scope: SESSION | GLOBAL
- Scope: GLOBAL
- Default value: `4096`
- Range: `[0, 2147483647]`
- This variable is used to control the length of the SQL string in [statement summary tables](/statement-summary-tables.md).

### tidb_stmt_summary_max_stmt_count <span class="version-mark">New in v4.0</span>

- Scope: SESSION | GLOBAL
- Scope: GLOBAL
- Default value: `3000`
- Range: `[1, 32767]`
- This variable is used to set the maximum number of statements that [statement summary tables](/statement-summary-tables.md) store in memory.

### tidb_stmt_summary_refresh_interval <span class="version-mark">New in v4.0</span>

- Scope: SESSION | GLOBAL
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change is intentional and only affects master. See: https://github.com/pingcap/tidb/blob/master/docs/design/2021-12-08-instance-scope.md

Comment on lines -1626 to -1635
### `tidb_enable_top_sql` <span class="version-mark">New in v5.4.0</span>

- Scope: GLOBAL
- Default value: `OFF`
- This variable is used to control whether to enable the [Top SQL](/dashboard/top-sql.md) feature.

> **Warning:**
>
> Currently, Top SQL is an experimental feature. It is not recommended that you use it for production environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was moved higher up in the document (correct alphabetical order).

Comment on lines -430 to +423
- Default value: `lower, md5, reverse, upper, vitess_hash`
- Default value: `lower, md5, reverse, tidb_shard, upper, vitess_hash`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is according to the generated source. The default has changed.

@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 2, 2022
@shichun-0415 shichun-0415 added area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. translation/doing This PR's assignee is translating this PR. type/enhancement The issue or PR belongs to an enhancement. and removed missing-translation-status This PR does not have translation status info. labels Mar 3, 2022
@shichun-0415 shichun-0415 added type/compatibility-or-feature-change This PR involves compatibility changes or feature behavior changes. and removed type/enhancement The issue or PR belongs to an enhancement. labels Mar 3, 2022
@TomShawn TomShawn added v6.0 This PR/issue applies to TiDB v6.0. area/general Relates to TiDB overview, architecture, and other general descriptions. and removed area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. labels Mar 3, 2022
@morgo
Copy link
Contributor Author

morgo commented Mar 4, 2022

I think there will be other updates. It looks like a lot of new sysvars were added which are not documented yet:

 124                 variable.TiDBSuperReadOnly,
 125                 variable.TiDBTableCacheLease,
 126                 variable.TiDBTopSQLMaxMetaCount,
 127                 variable.TiDBTopSQLMaxTimeSeriesCount,
 128                 variable.TiDBTxnAssertionLevel,
 129                 variable.TiDBPlacementMode,
 130                 variable.TiDBReadConsistency,
 131                 variable.TiDBEnableMutationChecker,
 132                 variable.TiDBLastDDLInfo,
 133                 variable.TiDBMemQuotaBindCache,
 134                 variable.SysdateIsNow

So it would be good to merge this first if that's possible :-)

@TomShawn TomShawn requested a review from bb7133 March 7, 2022 02:42
Copy link
Contributor

@mjonss mjonss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and thank you for the helpful comments!

Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bb7133
Copy link
Member

bb7133 commented Mar 15, 2022

/merge

@ti-chi-bot
Copy link
Member

@bb7133: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@morgo
Copy link
Contributor Author

morgo commented Mar 22, 2022

/merge

@ti-chi-bot
Copy link
Member

@morgo: /merge in this pull request requires 2 approval(s).

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

- This variable controls how TiDB behaves when the waiting time of SQL optimization reaches the timeout to synchronously load complete column statistics. The default value `OFF` means that SQL execution fails after the timeout. If you set this variable to `ON`, the SQL optimization gets back to using pseudo statistics after the timeout.
- Scope: SESSION | GLOBAL
- Default value: `0`
- Range: `[0, 2147483647]`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the value range changed? from [0, 4294967295]` changed to [0, 2147483647]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not specifically changed, it was always wrong. See: pingcap/tidb#30026 where it was introduced.

math.MaxInt32 == 32 bit signed integer (max = 2147483647).

@morgo
Copy link
Contributor Author

morgo commented Mar 22, 2022

/merge

@ti-chi-bot
Copy link
Member

@morgo: /merge in this pull request requires 2 approval(s).

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

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

/remove-status LGT1
/status LGT3

@ti-chi-bot ti-chi-bot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 22, 2022
@TomShawn
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 9f240cb

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 22, 2022
@ti-chi-bot ti-chi-bot merged commit 8c84c9c into pingcap:master Mar 22, 2022
@morgo morgo deleted the auto-update-sysvars.md branch March 22, 2022 02:50
@TomShawn 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/general Relates to TiDB overview, architecture, and other general descriptions. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. type/compatibility-or-feature-change This PR involves compatibility changes or feature behavior changes. v6.0 This PR/issue applies to TiDB v6.0.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants