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.

Sign up for GitHub

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

Jump to bottom

system-variables: update from generated source #6838

Merged
merged 7 commits into from
Dec 17, 2021
Merged

system-variables: update from generated source #6838

merged 7 commits into from
Dec 17, 2021

Conversation

morgo
Copy link
Contributor

@morgo morgo commented Nov 17, 2021 •
edited
Loading

What is changed, added or deleted? (Required)

This updates system variables, and is generated against the tidb source code. It is similar to previous PRs such as #6665

The changes are very minor and mainly improve consistency:

  • version_compile_os and version_compile_machine were alphabetically in the wrong order.
  • wait_timeout has changed default
  • timestamp has changed default (it now works correctly)

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)
  • 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

@morgo morgo requested a review from dveeden November 17, 2021 17:48
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 17, 2021 •
edited
Loading

[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.

@ti-chi-bot ti-chi-bot added missing-translation-status This PR does not have translation status info. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 17, 2021
@morgo
Copy link
Contributor Author

morgo commented Nov 17, 2021

There are a lot of broken external links which seem unrelated to this PR, and are causing the CI check to fail.

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 17, 2021
@dveeden
Copy link
Contributor

dveeden commented Nov 17, 2021

There are a lot of broken external links which seem unrelated to this PR, and are causing the CI check to fail.

#6837

TomShawn
TomShawn reviewed Nov 18, 2021
View reviewed changes
system-variables.md

### wait_timeout

- Scope: SESSION | GLOBAL
- Default value: `0`
- Default value: `28800`
Copy link
Contributor

Choose a reason for hiding this comment

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

We always treat the default value change with extra care. From which version this default value is changed? If it is changed from v5.3.0, shall we let the QA team @seiya-annie know and add it to v5.3.0 release notes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took a look at the PR, and it's not cherry picked: pingcap/tidb#29569
So it is master only right now.

It might be worth docs having a script to detect defaults changes. I am not specifically changing the default here - it has already been changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Translated into Chinese in pingcap/docs-cn#8076

@TomShawn
Copy link
Contributor

There are a lot of broken external links which seem unrelated to this PR, and are causing the CI check to fail.

You can ignore the external link checker. It only serves as a reference and does not block merge. It might be inaccurate.

@TomShawn TomShawn added translation/doing This PR's assignee is translating this PR. and removed missing-translation-status This PR does not have translation status info. labels Nov 18, 2021
@TomShawn TomShawn self-assigned this Nov 18, 2021
@TomShawn TomShawn added type/enhancement The issue or PR belongs to an enhancement. area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. labels Nov 18, 2021
@morgo
Copy link
Contributor Author

morgo commented Dec 10, 2021

I've unchecked 5.3 although it applies except the default for wait_timeout changing.

Can we merge this soon? There are already a few more updates, and I would like to create a new PR to follow on, so we can track system variables changes as they are made.

@ti-chi-bot 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 Dec 10, 2021
@TomShawn TomShawn added the for-future-release This PR only applies to master for now and might cherry-pick to a future release. label Dec 10, 2021
TomShawn
TomShawn reviewed Dec 10, 2021
View reviewed changes
system-variables.md
@@ -845,14 +846,14 @@ Query OK, 0 rows affected (0.09 sec)
- Default value: `ON`
- This variable is used to dynamically control whether the telemetry collection in TiDB is enabled. By setting the value to `OFF`, the telemetry collection is disabled. If the [`enable-telemetry`](/tidb-configuration-file.md#enable-telemetry-new-in-v402) TiDB configuration item is set to `false` on all TiDB instances, the telemetry collection is always disabled and this system variable will not take effect. See [Telemetry](/telemetry.md) for details.

### tidb_enable_tso_follower_proxy <span class="version-mark">New in v5.3</span>
### tidb_enable_tso_follower_proxy <span class="version-mark">New in v5.3.0</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes a few broken anchors in release-5.3.0.md. Could you please fix the broken anchors?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :-)

@ti-chi-bot ti-chi-bot removed the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 16, 2021
@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 16, 2021
@TomShawn
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 00ee361

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 17, 2021
@ti-chi-bot ti-chi-bot merged commit 4b437f5 into pingcap:master Dec 17, 2021
@morgo morgo deleted the fix-sv-from-generated-source branch December 17, 2021 02:21
@morgo morgo mentioned this pull request Dec 17, 2021
Merged
12 tasks
@TomShawn TomShawn added the type/compatibility-or-feature-change This PR involves compatibility changes or feature behavior changes. label Dec 28, 2021
@ran-huang ran-huang mentioned this pull request Dec 29, 2021
Merged
14 tasks
@TomShawn TomShawn mentioned this pull request Apr 2, 2022
Merged
15 tasks
@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 Apr 2, 2022
This was referenced Apr 7, 2022
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomShawn TomShawn TomShawn approved these changes

@dveeden dveeden dveeden approved these changes

Assignees

@TomShawn TomShawn

Labels
area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. for-future-release This PR only applies to master for now and might cherry-pick to a future release. 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/LGT2 Indicates that a PR has LGTM 2. 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. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@morgo @ti-chi-bot @dveeden @TomShawn