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

Refactor storage configuration and strategies for choosing paths with multi-disks #1216

Merged
merged 12 commits into from
Nov 13, 2020

Conversation

JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Nov 10, 2020

Signed-off-by: JaySon-Huang [email protected]

What problem does this PR solve?

Issue Number: a part of #1128

Problem Summary:
Previously, we choose a path according to the used size inside one table. The global data distribution may be skew.
And if the capacities of different disks are not the same, the disk with a smaller capacity makes get full while other disks are available.

What is changed and how it works?

  • StableDiskDelegator/PSDiskDelegatorMulti/PSDiskDelegatorRaft now chooses a new path according to the global available size.
  • Refactor the configuration for better backward compatibility and more human-readable

Related changes

  • Need to cherry-pick to the release branch 4.0

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
    • Deploy a cluster with version v4.0.7, load TPC-H 1, upgrade TiFlash node, run TPC-H queries, all is fine. Then load another TPC-H 1, run TPC-H queries, all is fine.

Side effects

  • N/A

Release note

  • No release note

@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@JaySon-Huang JaySon-Huang changed the title [DNM] Refactor storage configuration and strategies for choosing paths with multi-disks Refactor storage configuration and strategies for choosing paths with multi-disks Nov 11, 2020
@JaySon-Huang JaySon-Huang self-assigned this Nov 11, 2020
@JaySon-Huang JaySon-Huang added the type/new-feature Issue or PR for new feature label Nov 11, 2020
@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

default_profile = "default"
users_config = "users.toml"

# old style storage paths
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
# old style storage paths
# Deprecated path setting style. Check [storage] section for new style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.


## Store capacity of each path, i.e. max data size allowed.
## If it is not set, or is set to 0s, disk capacity will be used.
# capacity = [ ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also make an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

# dir = [ "/data0/tiflash", "/data1/tiflash" ]

## Store capacity of each path, i.e. max data size allowed.
## If it is not set, or is set to 0s, disk capacity will be used.
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
## If it is not set, or is set to 0s, disk capacity will be used.
## If it is not set, or is set to 0s, the actual disk capacity is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

Copy link
Contributor

@flowbehappy flowbehappy left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 12, 2020
@JaySon-Huang
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 13, 2020
@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot ti-srebot merged commit 6dbabf3 into pingcap:master Nov 13, 2020
@JaySon-Huang JaySon-Huang deleted the refine_choose_path branch November 13, 2020 12:31
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Nov 15, 2020
JaySon-Huang added a commit that referenced this pull request Nov 15, 2020
* Support multi-disks for PageStorage (#1156)
* Support multi-disks for RegionPersister (#1199)
* Refactor storage configuration and strategies for choosing paths with multi-disks (#1216)

Signed-off-by: JaySon-Huang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/new-feature Issue or PR for new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants