Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19608 )
Change subject: [master] exclude tservers in MAINTENANCE_MODE when leader rebalancing ...................................................................... Patch Set 7: (11 comments) > Patch Set 5: > > (3 comments) Thanks for your advices. http://gerrit.cloudera.org:8080/#/c/19608/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19608/5//COMMIT_MSG@7 PS5, Line 7: in > nit: in Done http://gerrit.cloudera.org:8080/#/c/19608/5//COMMIT_MSG@9 PS5, Line 9: Cur > nit: Change to "Currently, " Done http://gerrit.cloudera.org:8080/#/c/19608/5//COMMIT_MSG@10 PS5, Line 10: Because a tserver in MAINTENANCE_MODE : is maintainning by > Could you explain these semantics in the commit message? E.g. why is it not I rewrite it. http://gerrit.cloudera.org:8080/#/c/19608/5//COMMIT_MSG@12 PS5, Line 12: st > nit:whose in in http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer-test.cc File src/kudu/master/auto_leader_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer-test.cc@239 PS5, Line 239: tserve > nit: tserver Done http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer-test.cc@251 PS5, Line 251: ASSERT_STR_CONTAINS(out, Substitute("$0,$1", mini_tserver->uuid(), "MAINTENANCE_MODE")); > Shall we check the leader replicas' location in case the leader replicas wi I am not sure that I have understood your means. It doesn't matter whether 'enter_maintenance' command cause some the leader replicas transferring. At this, we focus whether leaders is rebalanced. Because if all tservers are normal, kudu cluster will reach rebalanced and one of them is in MAINTENANCE_MODE kudu cluster would not reach rebalanced. If we are careful about 'enter_maintenance''s influence, we can add unit tests about the CLI command. http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer-test.cc@263 PS5, Line 263: // This cluster cannot reach a rebalanced state of leaders since 1 of the 3 tservers : // is in maintenance mode. > nit: Rephrase to "This cluster cannot reach a rebalanced state of leaders s Done http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer.cc File src/kudu/master/auto_leader_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer.cc@239 PS5, Line 239: uuid_leaders > +1 That's a problem. Leader rebalancer should guarantee that don't transfer leader to a tserver in MAINTENANCE_MODE. Tservers in MAINTENANCE_MODE hold some leader replicas, that's possible, and transfer them to other tservers is not the function of leader rebalancing, it's another control flow. Leader rebalancer don't do the tranferring. http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer.cc@250 PS5, Line 250: that ar > nit: change to "that are" Done http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer.cc@250 PS5, Line 250: transferrin > nit: transferring Done http://gerrit.cloudera.org:8080/#/c/19608/5/src/kudu/master/auto_leader_rebalancer.cc@392 PS5, Line 392: set<stri > nit: set Done -- To view, visit http://gerrit.cloudera.org:8080/19608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f85a675e69fd02a62e2625881dad2ca5e27acd9 Gerrit-Change-Number: 19608 Gerrit-PatchSet: 7 Gerrit-Owner: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Thu, 23 Mar 2023 08:36:15 +0000 Gerrit-HasComments: Yes
