Skip to content

[enhancement](decommission) speed up decommission process (#14028)#14006

Merged
morningman merged 7 commits into
apache:masterfrom
dutyu:enhance_decommission
Nov 16, 2022
Merged

[enhancement](decommission) speed up decommission process (#14028)#14006
morningman merged 7 commits into
apache:masterfrom
dutyu:enhance_decommission

Conversation

@dutyu

@dutyu dutyu commented Nov 7, 2022

Copy link
Copy Markdown
Contributor

Proposed changes

Issue Number: close #14028

Problem summary

Describe your changes.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@hello-stephen

hello-stephen commented Nov 7, 2022

Copy link
Copy Markdown
Contributor

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.24 seconds
load time: 440 seconds
storage size: 17180407904 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221111081051_clickbench_pr_44219.html

@dutyu dutyu changed the title [enhancement](decommission) speed up decommission process (#13579) [enhancement](decommission) speed up decommission process (#14028) Nov 7, 2022

List<Long> backendTabletIds = invertedIndex.getTabletIdsByBackendId(beId);
if (backendTabletIds.isEmpty() && Config.drop_backend_after_decommission) {
if ((backendTabletIds.isEmpty() || Env.getCurrentRecycleBin().allTabletsInRecycledStatus(backendTabletIds))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

allTabletsInRecycledStatus is expensive, maybe we should put Config.drop_backend_after_decommission as the first condition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Appreciate for your suggestion, i have changed the code here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

allTabletsInRecycledStatus is expensive, maybe we should put Config.drop_backend_after_decommission as the first condition.

Would be appreciate if you spend some time to review it again.

@dutyu dutyu force-pushed the enhance_decommission branch from a471d2a to a55f50a Compare November 9, 2022 07:10

@cambyzju cambyzju left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

hf200012
hf200012 previously approved these changes Nov 11, 2022

@hf200012 hf200012 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions Bot added approved Indicates a PR has been approved by one committer. reviewed labels Nov 11, 2022
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

idToRecycleTime = Maps.newHashMap();
}

public synchronized boolean allTabletsInRecycledStatus(List<Long> backendTabletIds) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The backendTabletIds may contains millions of ids, which may cause a long run.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, as we've just discussed, i will add a threshold to control this check and add some logs.

@github-actions github-actions Bot removed the approved Indicates a PR has been approved by one committer. label Nov 11, 2022
@dutyu dutyu requested review from cambyzju, hf200012 and morningman and removed request for cambyzju, hf200012 and morningman November 14, 2022 06:28

@cambyzju cambyzju left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 15, 2022
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@morningman morningman merged commit 943e014 into apache:master Nov 16, 2022
@dutyu dutyu deleted the enhance_decommission branch July 9, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. kind/behavior-changed reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] decommission should safety drop be immediately when BE's tablets all in recycled state

5 participants