Skip to content

Refine placement rules in sql#6766

Merged
TomShawn merged 12 commits into
pingcap:masterfrom
TomShawn:place-rule-modify
Nov 15, 2021
Merged

Refine placement rules in sql#6766
TomShawn merged 12 commits into
pingcap:masterfrom
TomShawn:place-rule-modify

Conversation

@TomShawn
Copy link
Copy Markdown
Contributor

@TomShawn TomShawn commented Nov 9, 2021

What is changed, added or deleted? (Required)

Refine placement rules in sql

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.

  • master (the latest development version)
  • 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

@ti-chi-bot
Copy link
Copy Markdown
Member

ti-chi-bot commented Nov 9, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • morgo

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.

Details

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 9, 2021
@TomShawn TomShawn marked this pull request as draft November 9, 2021 03:29
@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2021
@TomShawn TomShawn added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. type/bugfix This PR fixes a bug. labels Nov 9, 2021
@ti-chi-bot ti-chi-bot removed the missing-translation-status This PR does not have translation status info. label Nov 9, 2021
@TomShawn TomShawn added missing-translation-status This PR does not have translation status info. area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. labels Nov 9, 2021
@TomShawn TomShawn self-assigned this Nov 9, 2021
@TomShawn TomShawn removed the missing-translation-status This PR does not have translation status info. label Nov 9, 2021
@TomShawn
Copy link
Copy Markdown
Contributor Author

TomShawn commented Nov 9, 2021

/verify

@TomShawn TomShawn mentioned this pull request Nov 9, 2021
Merged
12 tasks
@ti-chi-bot ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 9, 2021
@TomShawn
Copy link
Copy Markdown
Contributor Author

TomShawn commented Nov 9, 2021

/verify

@TomShawn TomShawn mentioned this pull request Nov 9, 2021
Merged
13 tasks
@TomShawn TomShawn requested a review from morgo November 9, 2021 08:27
morgo
morgo approved these changes Nov 9, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@morgo morgo left a comment

Choose a reason for hiding this comment

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

LGTM, any reason this is still a draft?

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 9, 2021
@TomShawn TomShawn marked this pull request as ready for review November 10, 2021 01:52
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 10, 2021
@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 10, 2021
TomShawn
TomShawn commented Nov 10, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
Comment on lines +12 to +21
Placement Rules in SQL is a feature that enables you to specify where data is store in a TiKV cluster using SQL interfaces. Using this feature, consecutive data is scheduled to a specific regions, data centers, racks, or hosts, with a specified replica count. This is useful for scenarios including optimizing a high availability strategy with lower cost, ensuring that local replicas of data are available for local stale reads, and adhering to compliance requirements.

The detailed user scenarios are as follows:

Placement Rules allow you to configure where data will be stored in a TiKV cluster. This is useful for scenarios including optimizing a high availability strategy, ensuring that local copies of data will be available for local stale reads, and adhering to compliance requirements.
- Place data across regions to improve the access performance in a specific region
- Merge multiple databases of different applications to reduce the cost on database maintenance
- Increase replica count for important data to improve the application availability and data reliability
- Store new data into SSDs and store old data into HHDs to lower the cost on data archiving and storage
- Schedule the leaders of hotspot data to high-performance TiKV instances
- Separate irrelevant data to different storages to improve availability
Copy link
Copy Markdown
Contributor Author

@TomShawn TomShawn Nov 10, 2021

Choose a reason for hiding this comment

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

@morgo PTAL thanks! This is provided by the PM in v5.3.0 release notes.

Copy link
Copy Markdown
Contributor

@morgo morgo Nov 10, 2021

Choose a reason for hiding this comment

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

LGTM. Only one we need to be careful about is "Place data across regions to improve the access performance in a specific region", since this is only true for access (aka stale read) scenario. For modification there are problems because the user will also need local transaction, which is a separate feature.

If PM is happy to leave this in, then I have no problems with it.

Copy link
Copy Markdown
Contributor Author

@TomShawn TomShawn Nov 11, 2021

Choose a reason for hiding this comment

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

@ThomasYYYY PTAL, thanks!

Copy link
Copy Markdown

@ThomasYYYY ThomasYYYY Nov 11, 2021

Choose a reason for hiding this comment

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

Currently Placement Rules only solve data locality challenges. We'd better only mention read performance instead of access performance.

Copy link
Copy Markdown

@ThomasYYYY ThomasYYYY Nov 11, 2021

Choose a reason for hiding this comment

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

"compliance" is also a big topic. Maybe we can use "data locality requirements" instead of "compliance requirements", until we figure out all the compliance requirements from real world users.
@morgo What do you think?

Copy link
Copy Markdown
Contributor

@morgo morgo Nov 11, 2021

Choose a reason for hiding this comment

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

Yeah, I think that is good. We mention compliance in known limitations below, so it is good not to claim it in the description for now.

Copy link
Copy Markdown
Contributor Author

@TomShawn TomShawn Nov 15, 2021

Choose a reason for hiding this comment

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

I have replaced "compliance requirements" with "data locality requirements". PTAL

morgo
morgo reviewed Nov 11, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
morgo
morgo reviewed Nov 11, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
en-jin19
en-jin19 reviewed Nov 15, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
morgo
morgo reviewed Nov 15, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
TomShawn and others added 3 commits November 15, 2021 10:50
@TomShawn @morgo
Apply suggestions from code review
6f11dba 
Co-authored-by: Morgan Tocker <tocker@gmail.com>
@TomShawn @en-jin19
Update placement-rules-in-sql.md
680177f 
Co-authored-by: Enwei <jinenwei@pingcap.com>
@TomShawn @morgo
Update placement-rules-in-sql.md
f6fe982 
Co-authored-by: Morgan Tocker <tocker@gmail.com>
TomShawn
TomShawn commented Nov 15, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
TomShawn
TomShawn commented Nov 15, 2021
View reviewed changes
Comment thread placement-rules-in-sql.md Outdated
@TomShawn
Copy link
Copy Markdown
Contributor Author

TomShawn commented Nov 15, 2021

/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 Nov 15, 2021
@TomShawn
Copy link
Copy Markdown
Contributor Author

TomShawn commented Nov 15, 2021

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

ti-chi-bot commented Nov 15, 2021

This pull request has been accepted and is ready to merge.

DetailsCommit hash: 1661f05

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 15, 2021
@TomShawn TomShawn merged commit e2c5ff6 into pingcap:master Nov 15, 2021
@TomShawn TomShawn deleted the place-rule-modify branch November 15, 2021 11:37
@TomShawn TomShawn added the v5.3 This PR/issue applies to TiDB v5.3. label Nov 15, 2021
@TomShawn TomShawn mentioned this pull request Nov 17, 2021
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Liuxiaozhen12 Liuxiaozhen12 Awaiting requested review from Liuxiaozhen12

@ThomasYYYY ThomasYYYY Awaiting requested review from ThomasYYYY

@en-jin19 en-jin19 Awaiting requested review from en-jin19

1 more reviewer

@morgo morgo morgo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TomShawn TomShawn

Labels

area/sql-infra Indicates that the Issue or PR belongs to the area of sql-infra and sql-metadata. size/M Denotes a PR that changes 30-99 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/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. type/bugfix This PR fixes a bug. v5.3 This PR/issue applies to TiDB v5.3.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TomShawn @ti-chi-bot @morgo @en-jin19 @ThomasYYYY