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

Ensure that table_options respect "Policy Validation" and "Skipping Policy Validation" #27974

Closed
morgo opened this issue Sep 13, 2021 · 5 comments · Fixed by #29016
Closed

Ensure that table_options respect "Policy Validation" and "Skipping Policy Validation" #27974

morgo opened this issue Sep 13, 2021 · 5 comments · Fixed by #29016
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@morgo
Copy link
Contributor

morgo commented Sep 13, 2021

Enhancement

Subtask of #18030

We need to ensure that table_options respect "Policy Validation" and "Skipping Policy Validation" sections of the design document.

This involves adding a new sysvar to skip validation for reloading, which might be used by logical dump tools to prevent restore errors.

@morgo morgo added the type/enhancement The issue or PR belongs to an enhancement. label Sep 13, 2021
@sylzd
Copy link
Contributor

sylzd commented Oct 9, 2021

/assign

@morgo
Copy link
Contributor Author

morgo commented Oct 11, 2021

Here is an example bug:

mysql> create placement policy myplacement followers=4;
Query OK, 0 rows affected (0.08 sec)

mysql> show placement;
+--------------------+-------------+
| Target             | Placement   |
+--------------------+-------------+
| POLICY myplacement | FOLLOWERS=4 |
+--------------------+-------------+
1 row in set (0.00 sec)

mysql> create table t1 (a int) placement policy=myplacement;
ERROR 1105 (HY000): failed to notify PD the placement rules: "[PD:placement:ErrBuildRuleList]build rule list failed, needs at least one leader or voter for range {7480000000000000FF3800000000000000F8, 7480000000000000FF3900000000000000F8}"

The expectation is that the error should be returned in the create placement policy statement, not the create table statement.

Edit: I think this placement policy should be legal though. It just says 4 followers, no constraints. It should be implied there is a leader.

@AilinKid
Copy link
Contributor

AilinKid commented Oct 11, 2021

Edit: I think this placement policy should be legal though. It just says 4 followers, no constraints. It should be implied there is a leader.

This means we need to infer those implicit syntax logic, too many cases for auto complete, I'm wary about.

@morgo
Copy link
Contributor Author

morgo commented Oct 11, 2021

We discussed how to handle validation in our weekly meeting today. The notes are summarized at: https://docs.google.com/document/d/1_A-bU2JoXZ0Fllhh0xJUdEtDXJQmz52toeNItW42fF0/edit?n=Placement_Rules_in_SQL_Weekly_Report#heading=h.ti4gpl7ary6h

The short answer is: we will try and do validation in the tidb-server for now. This may lead to some cases where there are errors on create table, but we can handle these as bugs and try to improve our validation. If this proves ineffective, we can add the create-a-range-in-pd behavior.

@sylzd
Copy link
Contributor

sylzd commented Oct 19, 2021

What does create-a-range-in-pd behavior mean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants