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

add descriptions for two system variables #9896

Merged
merged 12 commits into from Aug 12, 2022
Merged

add descriptions for two system variables #9896

merged 12 commits into from Aug 12, 2022

Conversation

cfzjywxk
Copy link
Contributor

@cfzjywxk cfzjywxk commented Aug 10, 2022

First-time contributors' checklist

What is changed, added or deleted? (Required)

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

These two variables could be applied to all the release versions below.

  • master (the latest development version)
  • v6.2 (TiDB 6.2 versions)
  • v6.1 (TiDB 6.1 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)

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 Aug 10, 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 added the first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. label Aug 10, 2022
@CLAassistant
Copy link

CLAassistant commented Aug 10, 2022

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot added missing-translation-status This PR does not have translation status info. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 10, 2022
@ti-chi-bot ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 10, 2022
@cfzjywxk cfzjywxk changed the title add descriptions for the system variable tidb_guarantee_linearizability add descriptions for two system variables Aug 10, 2022
- Persists to cluster: Yes
- Type: Boolean
- Default value: `OFF`
- This variable controls whether the calculated commit ts would be used for async commit. By default the two phase committer would request a new ts from the pd server and use it as the commit ts, and linearizability is guaranteed for all the concurrent transactions. This ts fetching cost could be saved if this variable is set to 'ON' which means the calculated commit is used, the cost is that causal consistency could not be guaranteed but not linearizability, more details could be referenced in this [document](https://en.pingcap.com/blog/async-commit-the-accelerator-for-transaction-commit-in-tidb-5-0/). If the usage scenario requires only causal consistency, this vairable could be set to 'ON' to improve performance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the description is not very accurate. When tidb_guarantee_linearizability is ON, the new ts from tso may not be the commit ts. It can still be smaller than the max read ts.

So, the more accurate way is to say whether a new ts from the pd server is used to calculate the final commit ts.

- Scope: SESSION
- Persists to cluster: No
- Type: String
- This variable is used to get the last transaction info within the current session. The information includes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we put it clearly that it's a read-only system variable?

Copy link
Contributor

@zyguan zyguan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sticnarf sticnarf left a comment

Choose a reason for hiding this comment

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

LGTM

@cfzjywxk
Copy link
Contributor Author

@songrijie PTAL

@TomShawn TomShawn self-assigned this Aug 10, 2022
@TomShawn TomShawn added add-missing-docs Add missing system variables to documentation area/transaction Indicates that the Issue or PR belongs to the area of transaction. translation/doing This PR's assignee is translating this PR. and removed missing-translation-status This PR does not have translation status info. labels Aug 10, 2022
- Persists to cluster: Yes
- Type: Boolean
- Default value: `OFF`
- This variable controls the way commit ts is calculated for async commit. By default the two phase committer would request a new ts from the pd server and use it to calculate the final commit ts, and linearizability is then guaranteed for all the concurrent transactions. This ts fetching from the pd server could be saved if this variable is set to 'ON', the cost is that only causal consistency could be guaranteed but not linearizability, more details could be referenced in this [document](https://en.pingcap.com/blog/async-commit-the-accelerator-for-transaction-commit-in-tidb-5-0/). If the usage scenario requires only causal consistency, this vairable could be set to 'ON' to improve performance.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we set this variable to ON, is the new TS fetched from PD used as the final commit TS?

Suggested change
- This variable controls the way commit ts is calculated for async commit. By default the two phase committer would request a new ts from the pd server and use it to calculate the final commit ts, and linearizability is then guaranteed for all the concurrent transactions. This ts fetching from the pd server could be saved if this variable is set to 'ON', the cost is that only causal consistency could be guaranteed but not linearizability, more details could be referenced in this [document](https://en.pingcap.com/blog/async-commit-the-accelerator-for-transaction-commit-in-tidb-5-0/). If the usage scenario requires only causal consistency, this vairable could be set to 'ON' to improve performance.
- This variable controls the way commit TS is calculated for async commit. By default (with the `OFF` value), the two-phase commit requests a new TS from the PD server and uses the TS to calculate the final commit TS. In this situation, linearizability is guaranteed for all the concurrent transactions.
- If you set this variable to `ON`, the TS fetched from the PD server is saved, with the cost that only causal consistency is guaranteed but not linearizability. For more details, see the blog post [Async Commit, the Accelerator for Transaction Commit in TiDB 5.0](https://en.pingcap.com/blog/async-commit-the-accelerator-for-transaction-commit-in-tidb-5-0/).
+ For scenarios that require only causal consistency, you can set this variable to `ON` to improve performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new TS fetched from pd would be used to calculate the final commit TS, it may or may not be the final commit TS.

- Scope: SESSION
- Persists to cluster: No
- Type: String
- This variable is used to get the last transaction info within the current session, it's a read-only variable. The transaction information includes:
Copy link
Contributor

@TomShawn TomShawn Aug 11, 2022

Choose a reason for hiding this comment

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

So users can do nothing with it, right?

Suggested change
- This variable is used to get the last transaction info within the current session, it's a read-only variable. The transaction information includes:
- This variable is used to get the last transaction information within the current session. it is a read-only variable. The transaction information includes:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

system-variables.md Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 12, 2022
@TomShawn
Copy link
Contributor

/hold

@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2022
@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Aug 12, 2022
@TomShawn TomShawn removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 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: dea3fbc

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 12, 2022
@ti-chi-bot ti-chi-bot merged commit 449e1c7 into master Aug 12, 2022
@TomShawn TomShawn deleted the cfzjywxk-patch-2 branch August 12, 2022 09:25
@songrijie
Copy link
Contributor

LGTM

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #9956.

@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 Aug 15, 2022
@TomShawn TomShawn added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Oct 14, 2022
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #10837.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-missing-docs Add missing system variables to documentation area/transaction Indicates that the Issue or PR belongs to the area of transaction. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. size/S Denotes a PR that changes 10-29 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants