remove direct placement #7400
remove direct placement #7400
Conversation
|
[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. |
ti-chi-bot
added
missing-translation-status
shichun-0415
added
the
area/sql-infra
shichun-0415
added
the
translation/doing
ti-chi-bot
removed
the
missing-translation-status
TomShawn
added
for-future-release
|
In 8ea693d I renamed Currently, the Synopsis does not convey that certain options are required together (i.e. regions + primary_region). I don't have any easy ideas on how to resolve that. At least from an error message and examples perspective this should be intuitive. |
| @@ -23,11 +23,11 @@ PolicyName ::= | |||
| Identifier | |||
|
|
|||
| PlacementOptionList ::= | |||
| DirectPlacementOption | |||
There was a problem hiding this comment.
Can we do this:
PlacementOptionList ::=
PlacementSugarSyntaxOptionList
| PlacementNoSugarSyntaxOptionList
PlacementSugarSyntaxOptionList ::= "PRIMARY_REGION" EqOpt stringLit "REGIONS" EqOpt stringLit ...
User can easily learn how to write a right sql
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
|
@morgo If Placement Rules in SQL is going to be GA in v6.0, shall we delete the experimental warning at Line 10? |
Yes. We can do it in this PR or a different PR (my preference), but either way works. |
Co-authored-by: TomShawn <[email protected]>
Co-authored-by: TomShawn <[email protected]>
|
If removing the experimental warning is the only thing to do for the v6.0 documentation, I might prefer doing it in this PR. Anyway, either way works for me, too. |
TomShawn
removed
the
do-not-merge/hold
|
/merge |
|
@morgo: In response to this:
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. |
|
/remove-status LGT1 |
ti-chi-bot
added
status/LGT2
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 01062cf
|
ti-chi-bot
added
the
status/can-merge
TomShawn
added
v6.0
|
@morgo There are still another 11 "placement options" in placement-rules-in-sql.md. Do they need to be modified, or kept as they are? |
| @@ -13,6 +13,13 @@ summary: The usage of ALTER PLACEMENT POLICY in TiDB. | |||
|
|
|||
| `ALTER PLACEMENT POLICY` is used to modify existing placement policies that have previously been created. All the tables and partitions which use the placement policy will automatically be updated. | |||
|
|
|||
| `ALTER PLACEMENT POLICY` _replaces_ the previous policy with the new definition. It does not _merge_ the old policy with the new one. In the following example, `FOLLOWERS=4` is lost when the `ALTER PLACEMENT POLICY` is executed: | |||
There was a problem hiding this comment.
| `ALTER PLACEMENT POLICY` _replaces_ the previous policy with the new definition. It does not _merge_ the old policy with the new one. In the following example, `FOLLOWERS=4` is lost when the `ALTER PLACEMENT POLICY` is executed: | |
| `ALTER PLACEMENT POLICY` _replaces_ the previous policy with the new definition. It does not _merge_ the old policy with the new one. In the following example, `FOLLOWERS=4` is overwritten by the `ALTER PLACEMENT POLICY` statement: |
There was a problem hiding this comment.
"lost" is more clear here, because it was not specified (overwritten) in the ALTER statement.
| @@ -11,7 +11,13 @@ summary: The usage of SHOW PLACEMENT FOR in TiDB. | |||
| > | |||
| > If you understand the risks, you can enable this experiment feature by executing `SET GLOBAL tidb_enable_alter_placement = 1;`. | |||
|
|
|||
| `SHOW PLACEMENT FOR` summarizes all placement options from direct placement and placement policies, and presents them in the canonical form for a specific table, database schema, or partition. | |||
| `SHOW PLACEMENT FOR` summarizes all placement options, and presents them in the canonical form for a specific table, database schema, or partition. | |||
There was a problem hiding this comment.
placement options, policies, or rules? The [docs-cn PR] uses 'placement policy' but translated as 'placement rules'. I am a bit confused. See https://github.com/pingcap/docs-cn/pull/8230/files#diff-439cd260163b3aaafae1cb9ac228dc2343f5ee9c23a9211fdb2c6168ea5c08beR12.
@xhebox @morgo
There was a problem hiding this comment.
- A placement OPTION is something like "FOLLOWERS=4".
- A placement POLICY is a way of grouping options together into a named object which can be assigned to tables.
- Placement RULES previously included both direct placement options and placement policies, but we've since removed direct placement so this term has lost much of its meaning. So it is now usually possible to use RULES and POLICY interchangably, but I think we should keep the terms for now because it is similar to using the terms "RBAC" and "permissions" interchangebly. "Policy" is like the RBAC of Placement.
|
|
||
| The statement returns a result set in which the `Scheduling_State` field indicates the current progress that the Placement Driver (PD) has made in scheduling the placement: | ||
|
|
||
| * `PENDING`: The PD has not yet started scheduling the placement. This might indicate that that the placement rules are semantically correct, but can not currently be satisfied by the cluster. For example, if `FOLLOWERS=4` but there are only 3 TiKV stores which are candidates for followers. |
There was a problem hiding this comment.
| * `PENDING`: The PD has not yet started scheduling the placement. This might indicate that that the placement rules are semantically correct, but can not currently be satisfied by the cluster. For example, if `FOLLOWERS=4` but there are only 3 TiKV stores which are candidates for followers. | |
| * `PENDING`: The PD has not yet started scheduling the placement. This might indicate that the placement rules are semantically correct, but cannot currently be satisfied by the cluster. For example, if `FOLLOWERS=4` but there are only 3 TiKV stores that are candidates for followers. |
TomShawn
added
the
requires-followup
|
I have created #7814 for followup |
TomShawn
removed
the
requires-followup
What is changed, added or deleted? (Required)
This updates the docs for a removed feature in master (direct placement)
Closes pingcap/tidb#31486
Closes pingcap/tidb#31559
Closes pingcap/tidb#31487
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?