-
Notifications
You must be signed in to change notification settings - Fork 688
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 docs for sysvar cache #5563
Add docs for sysvar cache #5563
Conversation
/label for-future-release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
system-variables.md
Outdated
@@ -25,8 +25,9 @@ SET GLOBAL tidb_distsql_scan_concurrency = 10; | |||
|
|||
> **Note:** | |||
> | |||
> TiDB differs from MySQL in that `GLOBAL` scoped variables **persist** through TiDB server restarts. Changes are also propagated to other TiDB servers every 2 seconds [TiDB #14531](https://github.com/pingcap/tidb/issues/14531). | |||
> Additionally, TiDB presents several MySQL variables from MySQL 5.7 as both readable and settable. This is required for compatibility, since it is common for both applications and connectors to read MySQL variables. For example: JDBC connectors both read and set query cache settings, despite not relying on the behavior. | |||
> <!-- Since next_version version --> Executing `SET GLOBAL` applies immediately on the TiDB server where the statement was issued. All TiDB servers in the cluster are sent a notification to refresh their global variables, which will begin immediately as a background operation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other TiDB servers
, receive a notification
, maybe?
BTW, I think it should be noted that the notification is immediate, but the refresh/eviction is not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually not "all other servers" but "all servers" (the initiating server will also receive the message too).
I used the language "begin immediately as a background operation" to indicate that it has no guarantee on completion time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notifications may not always succeed to be delivered to all the tidb instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhebox @tiancaiamao I have updated the wording. PTAL again, thx!
@xhebox PTAL again. Thanks! |
/hold |
Hi @morgo, we are preparing the user docs for TiDB v5.1 and would like to know whether the doc changes of this PR apply to TiDB v5.1 or not. Would you please check and reply? Thanks! |
Yes, it is in 5.1. |
Got it. Thanks for the confirmation. I'll add the v5.1 label to identify this PR. |
@xhebox @tiancaiamao @zhangjinpeng1987 PTAL, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @TomShawn
Co-authored-by: TomShawn <[email protected]>
@TomShawn this should be ready now |
Co-authored-by: TomShawn <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/remove-status LGT1 |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 5738e3c
|
/remove-translation doing |
What is changed, added or deleted? (Required)
This is the docs for pingcap/tidb#24359 which has not merged yet. It more accurately reflects the future-state of sysvars.
There is another PR for GA releases which describes how it previously worked better: #5562
It needs a "followup" to insert the version the change is effective from.
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?