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

remove direct placement #8230

Merged
merged 17 commits into from
Mar 18, 2022
Merged

remove direct placement #8230

merged 17 commits into from
Mar 18, 2022

Conversation

xhebox
Copy link
Contributor

@xhebox xhebox commented Jan 26, 2022

Signed-off-by: xhe xw897002528@gmail.com

First-time contributors' checklist

What is changed, added or deleted? (Required)

translating pingcap/docs#7400, pingcap/docs#7823

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 (in Chinese).

  • master (the latest development version)
  • 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)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

What is the related PR or file 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

Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox xhebox requested review from lcwangchao and TomShawn January 26, 2022 05:42
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 26, 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 missing-translation-status This PR does not have translation status info. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 26, 2022
@TomShawn TomShawn added the translation/from-docs This PR is translated from a PR in pingcap/docs. label Jan 27, 2022
@ti-chi-bot ti-chi-bot removed the missing-translation-status This PR does not have translation status info. label Jan 27, 2022
@TomShawn TomShawn added area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. for-future-release This PR only applies to master for now and might cherry-pick to a future release. labels Jan 27, 2022
@ran-huang ran-huang added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2022
@TomShawn TomShawn added v6.0 This PR/issue applies to TiDB v6.0. and removed for-future-release This PR only applies to master for now and might cherry-pick to a future release. labels Mar 4, 2022
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
Copy link
Contributor Author

xhebox commented Mar 7, 2022

Updated with the latest commit of the upstream PR

Copy link
Contributor

@TomShawn TomShawn left a comment

Choose a reason for hiding this comment

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

Questions to be clarified:

  1. Placement options - 放置选项:Should this term be globally modified to "placement rules" or "placement policies"?
  2. Placement rules - 放置规则:What's the difference between "placement rules" and "placement policies"?
  3. Placement policies - 放置策略:The literal translation for "policy" is "策略" instead of "规则". If "placement policies" are translated in the same way as "placement rules", that can be confusing.

Could you please take a look? Thanks! @xhebox @morgo

placement-rules-in-sql.md Outdated Show resolved Hide resolved
placement-rules-in-sql.md Outdated Show resolved Hide resolved
placement-rules-in-sql.md Outdated Show resolved Hide resolved
placement-rules-in-sql.md Outdated Show resolved Hide resolved
sql-statements/sql-statement-alter-placement-policy.md Outdated Show resolved Hide resolved
sql-statements/sql-statement-show-placement-for.md Outdated Show resolved Hide resolved
sql-statements/sql-statement-show-placement.md Outdated Show resolved Hide resolved
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
@xhebox
Copy link
Contributor Author

xhebox commented Mar 9, 2022

@TomShawn

The feature is called placement rules in SQL. But there are only placement policy objects in SQL. These policies themselves are placement rules(which only exists on PD), in SQL context. So policy is almost synonymous as rules now. And most users are using rules/policies at the same time.

So, it is, policies are objects composed of options. You should not modify options. If we really want to unify something, it is policy and rule.

EDIT: my current proposal, if we want to change:

  1. placement options -> placement policy options (放置选项 -> 放置策略选项)
  2. placement rule -> placement policy (放置规则 -> 放置策略)
  3. Add some descriptions on placement rules(PD) and placement policies(TiDB/SQL)

EDIT2:
My intention of changing placement policy -> placement rules (放置策略 -> 放置规则) in the Chinese docs is that most people knowing placement rules(PD) 放置规则, so they just say create placement rules (放置规则) even if they are showing CREATE PLACEMENT POLICY statements.

xhebox and others added 7 commits March 9, 2022 14:19
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
Copy link
Contributor Author

xhebox commented Mar 10, 2022

@TomShawn Now it becomes a translation of pingcap/docs#7400 and pingcap/docs#7823

placement-rules-in-sql.md Outdated Show resolved Hide resolved

ALTER DATABASE test FOLLOWERS=2; -- 再次更改默认的放置选项,此更改不影响已有的表
CREATE TABLE t4 (a INT); -- 创建表 t4,默认的放置策略 p3 生效
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CREATE TABLE t4 (a INT); -- 创建表 t4,默认的放置策略 p3 生效。
CREATE TABLE t4 (a INT); -- 创建表 t4,默认的放置规则 p3 生效。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

故意的, p3是策略, 所以这个不改规则.

Copy link
Contributor

Choose a reason for hiding this comment

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

这里英文中是 rules

Copy link
Contributor Author

Choose a reason for hiding this comment

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

那我改一下英文

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creates a table t4 with the default policy p3. 不对啊就是policy

placement-rules-in-sql.md Outdated Show resolved Hide resolved
placement-rules-in-sql.md Outdated Show resolved Hide resolved
* TiDB 生态工具,包括 Backup & Restore (BR)、TiCDC、TiDB Lightning 和 TiDB Data Migration (DM),不支持放置规则。
* 临时表不支持放置选项,直接放置和放置策略均不支持
* 临时表不支持放置规则
Copy link
Contributor

Choose a reason for hiding this comment

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

这里英文写的是“placement options”,不知需要把这里的“规则”改回为“选项”,还是改英文的“option”呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我觉得不改也行, 这两个都是一种意思. 中文更抽象, 英文更具体.

placement-rules-in-sql.md Outdated Show resolved Hide resolved
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
xhebox and others added 3 commits March 11, 2022 12:47
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
placement-rules-in-sql.md Outdated Show resolved Hide resolved
Copy link
Contributor

@TomShawn TomShawn 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

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 11, 2022
xhebox and others added 2 commits March 11, 2022 13:37
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
placement-rules-in-sql.md Show resolved Hide resolved
placement-rules-in-sql.md Show resolved Hide resolved
@@ -173,7 +213,7 @@ PARTITION BY RANGE( YEAR(purchased) ) (

* Dumpling 不支持导出放置策略,见 [issue #29371](https://github.com/pingcap/tidb/issues/29371)。
* TiDB 生态工具,包括 Backup & Restore (BR)、TiCDC、TiDB Lightning 和 TiDB Data Migration (DM),不支持放置规则。
Copy link
Contributor

Choose a reason for hiding this comment

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

目前生态工具的情况是,Lightning 目前不支持放置规则。CDC/Binlog 不会同步 placement 到下游数据库。BR 支持 placement (In review:https://github.com/pingcap/tidb/pull/33007)需要修改下当前文档的描述。

之后需要 @IANTHEREAL @leoppro @sunzhaoyang 看下?

@TomShawn TomShawn added v6.0 This PR/issue applies to TiDB v6.0. and removed v6.0 This PR/issue applies to TiDB v6.0. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 18, 2022
@TomShawn
Copy link
Contributor

/remove-status LGT1
/status LGT2

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 18, 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: 89e3c74

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 18, 2022
@ti-chi-bot ti-chi-bot merged commit c0ce272 into pingcap:master Mar 18, 2022
@xhebox xhebox deleted the placement_2 branch March 18, 2022 08:05
@xhebox xhebox mentioned this pull request Mar 18, 2022
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. translation/from-docs This PR is translated from a PR in pingcap/docs. v6.0 This PR/issue applies to TiDB v6.0.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants