-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
docs: Add design document about Lock View #24375
Conversation
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
/run-all-tests |
[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 writing |
|`TRX_ID` | `unsigned bigint` | The transaction ID (aka. start ts) | | ||
|`TRX_STARTED`|`time`| Human readable start time of the transaction | | ||
|`DIGEST`|`text`| The digest of the current executing SQL statement | | ||
|`SQLS` | `text` | A list of all executed SQL statements' digests | |
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.
Maybe ALL_SQLS
, ALL_DIGESTS
, or HISTORY_DIGESTS
looks better.
|`STATE`| `enum('Running', 'Lock waiting', 'Committing', 'RollingBack')`| The state of the transaction | | ||
| `WAITING_START_TIME` | `time` | The elapsed time since the start of the current lock waiting (if any) | | ||
| `SCOPE` | `enum('Global', 'Local')` | The scope of the transaction | | ||
| `ISOLATION_LEVEL` | `enum('RR', 'RC')` | | |
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.
Maybe it's better to use REPEATABLE-READ
and READ-COMMITTED
instead of the abbreviations, to keep the format consistent with elsewhere, e. g. the variable @@tx_isolation
.
SonarCloud Quality Gate failed. 0 Bugs No Coverage information |
Updated permission requirements of the tables. PTAL again, thx. @longfangsong @youjiali1995 @cfzjywxk |
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
* All these information can be collected on TiDB side. Since the amount of concurrent transactions won't be too large, and it doesn't need to be persisted, so it's ok to implement it as a memory table. For querying among the cluster, just register the table under `infoschema/cluster.go` and write the global table name with the local one. | ||
* As the simplest way of implementing, most information can be passed with a similar way like `ProcessInfo`, or even directly passed via the `ProcessInfo` struct. | ||
* Permission: | ||
* `PROCESS` privilege is needed to access the full content of this table. For users without `PROCESS` permission, only transactions started by the current user will be shown, and others will be filtered out, which is similar to the `processlist` table. |
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.
Does mysql has the similar access requirement for this information?
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.
I didn't find any complete explanation. Here are some pieces of information I've found.
- The
INNODB_TRX
table of MySQL 5.7 requiresPROCESS
privilege ( ref ) - The
INNODB_LOCKS
table of MySQL 5.7 requiresPROCESS
privilege ( ref ) - The
INNODB_LOCK_WAITS
table of MySQL 5.7 requiresPROCESS
privilege ( ref ) - The
SHOW ENGINE
statement of MySQL 5.7, which provides a tool to analyze deadlocks, requiresPROCESS
privilege. ( ref ) - The
DATA_LOCKS
andDATA_LOCK_WATIS
table of MySQL 8.0 has different permission requirement, which I think I didn't understand. ( ref )
/merge |
This pull request has been accepted and is ready to merge. Commit hash: d8c7aa6
|
@MyonKeminta: Your PR was out of date, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes. 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. |
/run-all-tests This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin). Silence the bot with the |
What problem does this PR solve?
Problem Summary: This PR adds design document of Lock View (#24199)
What is changed and how it works?
What's Changed: This PR adds design document of Lock View.
Most content of this PR comes directly from our internal document on google doc, with some minor modification and additional information.
Related changes
Check List
Tests
Side effects
Release note