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

fix(shell): disk_replica printer bug #508

Merged
merged 2 commits into from
Apr 2, 2020

Conversation

foreverneverer
Copy link
Contributor

@foreverneverer foreverneverer commented Apr 1, 2020

What problem does this PR solve?

disk_replica command will crash due to the bug of multi_printer.add(std::move(disk_printer)):
https://github.com/XiaoMi/pegasus/blob/47cea6363e963eeeb166cbd1661c5636324bb8e4/src/shell/commands/disk_rebalance.cpp#L235-L251
it can only be executed after the disk_printer was created, in other word, It should be move out of for (const auto &disk_info : resp.disk_infos) loop and in for (const auto &err_resp : err_resps) loop which will create new disk_printer every time.

Actually, It should be same with disk_capacity :https://github.com/XiaoMi/pegasus/blob/47cea6363e963eeeb166cbd1661c5636324bb8e4/src/shell/commands/disk_rebalance.cpp#L150-L169

What is changed and how it works?

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@neverchanje neverchanje changed the title fix: disk_replica printer bug fix(shell): disk_replica printer bug Apr 1, 2020
@neverchanje neverchanje added the type/bug-fix This PR fixes a bug. label Apr 2, 2020
@acelyc111 acelyc111 merged commit 7ca79c3 into apache:master Apr 2, 2020
@neverchanje neverchanje mentioned this pull request Apr 10, 2020
neverchanje pushed a commit that referenced this pull request Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.12.3 type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants