Skip to content

*: refactor transaction documents#2198

Merged
anotherrachel merged 8 commits into
pingcap:masterfrom
dcalvin:update-optimistic-locking-doc
Feb 20, 2020
Merged

*: refactor transaction documents#2198
anotherrachel merged 8 commits into
pingcap:masterfrom
dcalvin:update-optimistic-locking-doc

Conversation

@dcalvin
Copy link
Copy Markdown

@dcalvin dcalvin commented Jan 19, 2020 •
edited by anotherrachel
Loading

What is changed, added or deleted?

Based on our reviews and offline discussions, we decided to refactor and update transaction-related documents, including:

  • Split and merge the current Optimistic Locking best practice doc into existing Transaction Documents (because it is not a best practice doc, rather, it is a compilation of existing content on transactions.)
  • The Transaction Overview doc include content applicable to both Optimistic Transaction and Pessimistic Transaction
  • The previous Transaction Model is now changed to Optimistic Transaction

@anotherrachel FYI.

What is the related PR or file link(s)?

  • Reference link(s):
  • This PR is to align with:
  • N/A (not applicable)

Which TiDB version(s) does your changes apply to?

  • All active versions: dev, v3.0, v2.1, v3.1
  • dev (the latest development version)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)
  • v3.1 (TiDB 3.1 versions)
  • v4.0 (TiDB 4.0 versions)
  • All active and inactive versions
  • N/A (not applicable)

Note: If your changes apply to multiple TiDB versions, make sure you update the documents in the corresponding version folders such as "dev", "v3.0", "v2.1" and "v3.1" in this PR.

  • Updated one version first. Will update other versions after I get two LGTMs.

MyonKeminta
MyonKeminta reviewed Jan 20, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/overview.md Outdated
@anotherrachel anotherrachel self-assigned this Feb 3, 2020
@TomShawn TomShawn added the translation/doing This PR’s assignee is translating this PR. label Feb 4, 2020
@lilin90
Copy link
Copy Markdown
Member

lilin90 commented Feb 7, 2020

@dcalvin Please resolve lint failure.

@dcalvin
Copy link
Copy Markdown
Author

dcalvin commented Feb 10, 2020 •
edited
Loading

@dcalvin Please resolve lint failure.

@anotherrachel will follow this PR through.

@anotherrachel anotherrachel force-pushed the update-optimistic-locking-doc branch 2 times, most recently from 9e493de to 60d1902 Compare February 10, 2020 11:42
lilin90
lilin90 reviewed Feb 10, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/overview.md Outdated
Comment thread v3.1/reference/transactions/overview.md
Comment thread v3.1/reference/transactions/overview.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md
@lilin90 lilin90 changed the title Refactor transaction documents *: refactor transaction documents Feb 10, 2020
@anotherrachel anotherrachel mentioned this pull request Feb 10, 2020
Closed
youjiali1995
youjiali1995 reviewed Feb 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@youjiali1995 youjiali1995 left a comment

Choose a reason for hiding this comment

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

Thanks @dcalvin . 由于 TiDB 事务模型变化较大，后面我们研发同学也会进行一波整理。这个 PR 处理掉 comments 后我觉得是可以的。

Comment thread v3.1/reference/configuration/tidb-server/tidb-specific-variables.md Outdated
Comment thread v3.1/reference/transactions/overview.md Outdated
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
Comment thread v3.1/reference/mysql-compatibility.md Outdated
Comment thread v3.1/reference/sql/statements/select.md Outdated
anotherrachel
anotherrachel reviewed Feb 11, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
sticnarf
sticnarf reviewed Feb 11, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
sticnarf
sticnarf reviewed Feb 11, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
sticnarf
sticnarf reviewed Feb 11, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
sticnarf
sticnarf reviewed Feb 11, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md Outdated
lilin90
lilin90 reviewed Feb 13, 2020
View reviewed changes
Comment thread v3.1/reference/transactions/transaction-optimistic.md
youjiali1995
youjiali1995 approved these changes Feb 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@youjiali1995 youjiali1995 left a comment

Choose a reason for hiding this comment

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

LGTM

@anotherrachel
Copy link
Copy Markdown
Contributor

anotherrachel commented Feb 17, 2020 •
edited
Loading

@lilin90 PTAL again, thanks! If the content is ready, I'll update other versions next.

lilin90
lilin90 reviewed Feb 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@lilin90 lilin90 left a comment

Choose a reason for hiding this comment

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

Rest LGTM

Comment thread v3.1/reference/mysql-compatibility.md Outdated
Comment thread v3.1/reference/sql/statements/select.md Outdated
@anotherrachel anotherrachel added dev size/large Changes of a large size. and removed DNM labels Feb 18, 2020
@anotherrachel anotherrachel added status/LGT1 Indicates that a PR has LGTM 1. v2.1 labels Feb 18, 2020
@anotherrachel
Copy link
Copy Markdown
Contributor

anotherrachel commented Feb 18, 2020 •
edited
Loading

All versions updated. Some changes are not applicable for v2.1.
PTAL, thanks! @lilin90 @youjiali1995 @sticnarf

@anotherrachel anotherrachel added the type/refactor The issue belongs to a refactor work. label Feb 18, 2020
lilin90
lilin90 reviewed Feb 18, 2020
View reviewed changes
Comment thread v2.1/reference/transactions/overview.md
>
> 由于 TiDB 中的资源是分布式的，TiDB 中单线程 workload 可能不会很好地利用分布式资源，因此性能相比于单实例部署的 MySQL 较低。这与 TiDB 中的事务延迟较高的情況类似。

### 大事务
Copy link
Copy Markdown
Member

@lilin90 lilin90 Feb 18, 2020

Choose a reason for hiding this comment

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

@youjiali1995 确认一下，TiDB v2.1 有大事务吗，是否只在某个版本开始有呢？

Copy link
Copy Markdown
Contributor

@youjiali1995 youjiali1995 Feb 20, 2020

Choose a reason for hiding this comment

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

不好意思，刚看到。4.0 才开始有的

Copy link
Copy Markdown
Contributor

@anotherrachel anotherrachel Feb 20, 2020

Choose a reason for hiding this comment

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

This confuses me... The doc that introduces transaction model (now deleted in this PR) used to say "大事务“, even in v2.1. See here.
So "大事务“ cannot be used here in v2.1 v3.0 and v3.1? @youjiali1995

Copy link
Copy Markdown
Contributor

@sticnarf sticnarf Feb 20, 2020

Choose a reason for hiding this comment

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

@anotherrachel The documentation are actually telling the user that TiDB has poor support of big transactions so there are some limitations.

Copy link
Copy Markdown
Contributor

@sticnarf sticnarf Feb 20, 2020

Choose a reason for hiding this comment

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

The limitations are relaxed in 4.0 so we say big transactions are supported in 4.0

Copy link
Copy Markdown
Contributor

@anotherrachel anotherrachel Feb 20, 2020

Choose a reason for hiding this comment

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

@sticnarf Thank you for clarifying that! Then we can keep the current description about "大事务“ in v2.1/v3.0/v3.1?
In dev version, I have already updated the description. See here.

Copy link
Copy Markdown
Contributor

@sticnarf sticnarf Feb 20, 2020

Choose a reason for hiding this comment

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

SGTM

@TomShawn
Copy link
Copy Markdown
Contributor

TomShawn commented Feb 20, 2020

@anotherrachel Please update this PR according to #2255, thanks!

@anotherrachel anotherrachel force-pushed the update-optimistic-locking-doc branch from 549b048 to d55f8f4 Compare February 20, 2020 11:54
@anotherrachel
Copy link
Copy Markdown
Contributor

anotherrachel commented Feb 20, 2020

@lilin90 PTAL, thanks!

lilin90
lilin90 approved these changes Feb 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@lilin90 lilin90 left a comment

Choose a reason for hiding this comment

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

LGTM

@anotherrachel anotherrachel merged commit f310595 into pingcap:master Feb 20, 2020
@ran-huang ran-huang mentioned this pull request Mar 6, 2020
Merged
1 task
@anotherrachel anotherrachel mentioned this pull request Mar 24, 2020
Merged
4 tasks
@ilovesoup ilovesoup mentioned this pull request Apr 1, 2020
Merged
4 tasks
This was referenced Apr 3, 2020
Merged
Merged
Merged
@anotherrachel anotherrachel 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 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MyonKeminta MyonKeminta MyonKeminta left review comments

@lilin90 lilin90 lilin90 approved these changes

@AndreMouche AndreMouche Awaiting requested review from AndreMouche

+3 more reviewers

@youjiali1995 youjiali1995 youjiali1995 approved these changes

@sticnarf sticnarf sticnarf left review comments

@anotherrachel anotherrachel anotherrachel left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@anotherrachel anotherrachel

Labels

size/large Changes of a large size. status/LGT1 Indicates that a PR has LGTM 1. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. type/refactor The issue belongs to a refactor work.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dcalvin @lilin90 @anotherrachel @TomShawn @MyonKeminta @youjiali1995 @sticnarf