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

Consider make rule location_labels be empty #31052

Closed
Tracked by #18030
nolouch opened this issue Dec 27, 2021 · 3 comments · Fixed by #31189
Closed
Tracked by #18030

Consider make rule location_labels be empty #31052

nolouch opened this issue Dec 27, 2021 · 3 comments · Fixed by #31189
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@nolouch
Copy link
Member

nolouch commented Dec 27, 2021

Background

As issue tikv/pd#4467 shown, creating placement rule by DDL will set the location_labels be region,zone,rack,host in default. but the TiKV labels may not guarantee the relevant label hierarchy, which leads to un-expect scheduling.

 {
    "group_id": "TiDB_DDL_65",
    "id": "partition_rule_65_0",
    "index": 2,
    "start_key": "7480000000000000ff4100000000000000f8",
    "end_key": "7480000000000000ff4200000000000000f8",
    "role": "voter",
    "count": 3,
    "label_constraints": [
      {
        "key": "region",
        "op": "in",
        "values": [
          "us-west1"
        ]
      },
      {
        "key": "engine",
        "op": "notIn",
        "values": [
          "tiflash"
        ]
      }
    ],
    "location_labels": [
      "region",
      "zone",
      "rack",
      "host"
    ],
    "version": 1
  },

Proposal

make location_labels be empty, it's more like an undefined behavior, we may consider deleting the filed location_labels in the rule later, and make it is affected by a global setting location labels. and empty more friendly for the compatibility issue.

@nolouch nolouch added the type/enhancement The issue or PR belongs to an enhancement. label Dec 27, 2021
@nolouch
Copy link
Member Author

nolouch commented Dec 27, 2021

ptal @xhebox @morgo @disksing

@xhebox
Copy link
Contributor

xhebox commented Dec 27, 2021

I would like to see that, probably make isolationLevel global too. That makes much more sense to me than setting them in every single rule.

@xhebox xhebox self-assigned this Dec 27, 2021
@disksing
Copy link
Contributor

Good idea. I would like to add a situation. When location_labels: [] is set, the rule does not take location isolation into consideration.

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