convert 2PC in TiDB sequence diagram from PNG to Mermaid code (#22560)#22561
Conversation
ti-chi-bot
added
lgtm
needs-1-more-lgtm
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to enhance the documentation for TiDB's optimistic transactions by converting a critical sequence diagram from a static PNG image to a dynamic Mermaid code block. This change improves the diagram's maintainability, version control, and accessibility, making it easier for contributors to understand and update the visual representation of the two-phase commit process. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully converts the 2PC sequence diagram from a static PNG image to Mermaid code, which is a great improvement for maintainability and accessibility. I've suggested some refinements to the Mermaid diagram's text to improve clarity, correct minor typos, and align it more closely with the documentation style guide, making it easier for users to understand.
| ```mermaid | ||
| --- | ||
| title: 2PC in TiDB | ||
| --- | ||
| sequenceDiagram | ||
| participant client | ||
| participant TiDB | ||
| participant PD | ||
| participant TiKV | ||
|
|
||
| client->>TiDB: begin | ||
| TiDB->>PD: get ts as start_ts | ||
|
|
||
| loop excute SQL | ||
| alt do read | ||
| TiDB->>PD: get region from PD or cache | ||
| TiDB->>TiKV: get data from TiKV or cache with start_ts | ||
| TiDB-->>client: return read result | ||
| end | ||
| alt do write | ||
| TiDB-->>TiDB: write in cache | ||
| TiDB-->>client: return write result | ||
| end | ||
| end | ||
|
|
||
| client->>TiDB: commit | ||
|
|
||
| opt start 2PC | ||
| TiDB-->>TiDB: for all keys need to write,choose first one as primary | ||
| TiDB->>PD: locate each key | ||
| TiDB-->>TiDB: group keys by region to [](region,keys) | ||
|
|
||
| opt prewrite with start_ts | ||
| TiDB->>TiKV: prewrite(primary_key,start_ts) | ||
| loop prewrite to each region in [](region,keys) parallelly | ||
| TiDB->>TiKV: prewrite(keys,primary_key,start_ts) | ||
| end | ||
| end | ||
|
|
||
| opt commit | ||
| TiDB-->>PD: get ts as commit_ts | ||
| TiDB-->>TiKV: commit primary with commit_ts | ||
| loop send commit to each region in [](region,keys) parallelly | ||
| TiDB->>TiKV: commit(keys,commit_ts) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| TiDB-->>client: success | ||
| ``` |
There was a problem hiding this comment.
The Mermaid diagram is a great addition. To enhance clarity and align with the documentation style guide, I've made several improvements:
- Corrected a typo from
excutetoexecute. - Improved the descriptions to be more formal and descriptive (e.g.,
do readtoRead operation). - Used consistent capitalization for actions.
- Simplified complex descriptions to make the flow easier to follow (e.g.,
for all keys need to write,choose first one as primarytoSelect a primary key from all keys to be written). - Made the two phases of the commit process more explicit (
Phase 1: Prewrite,Phase 2: Commit). - Corrected arrow types for requests to PD and TiKV to better reflect synchronous operations.
These changes make the diagram more professional and easier for new users to understand, as per the style guide's principle of clarity.
---
title: 2PC in TiDB
---
sequenceDiagram
participant client
participant TiDB
participant PD
participant TiKV
client->>TiDB: BEGIN
TiDB->>PD: Get timestamp as start_ts
loop Execute SQL statements
alt Read operation
TiDB->>PD: Get region from PD (or cache)
TiDB->>TiKV: Get data from TiKV (or cache) with start_ts
TiDB-->>client: Return read result
end
alt Write operation
TiDB-->>TiDB: Write to TiDB memory buffer
TiDB-->>client: Return write result
end
end
client->>TiDB: COMMIT
opt 2PC (Two-Phase Commit)
TiDB-->>TiDB: Select a primary key from all keys to be written
TiDB->>PD: Locate region for each key
TiDB-->>TiDB: Group keys by region
opt Phase 1: Prewrite
TiDB->>TiKV: Prewrite primary key with start_ts
loop For each region
TiDB->>TiKV: Prewrite secondary keys in parallel
end
end
opt Phase 2: Commit
TiDB->>PD: Get timestamp as commit_ts
TiDB->>TiKV: Commit primary key with commit_ts
loop For each region
TiDB->>TiKV: Commit secondary keys in parallel
end
end
end
TiDB-->>client: SuccessReferences
- The documentation should be clear, simple, and easy to understand for users. The suggested changes improve the clarity and readability of the sequence diagram. (link)
- The documentation should have a logical flow and clear sentence structure. The suggested changes improve the descriptions of the steps in the diagram. (link)
- If something might confuse a new user, suggest a reword. The original diagram's text was somewhat cryptic and could be confusing. The suggested changes make it more accessible. (link)
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Oreoxmt The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
This is an automated cherry-pick of #22560
First-time contributors' checklist
What is changed, added or deleted? (Required)
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.
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?