Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13207 )
Change subject: IMPALA-8460: Simplify cluster membership management ...................................................................... Patch Set 13: (2 comments) Thanks for the reviews! Addressed the remaining comments in PS14, will rebase the change next. http://gerrit.cloudera.org:8080/#/c/13207/13/be/src/scheduling/cluster-membership-mgr-test.cc File be/src/scheduling/cluster-membership-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/13207/13/be/src/scheduling/cluster-membership-mgr-test.cc@400 PS13, Line 400: num_expected_alive > use 'idx' here instead as this would fail if NUM_BACKENDS/2 is odd, eg for Oh, that was an annoying mistake. I fixed it properly and made sure it works correctly for N=10. http://gerrit.cloudera.org:8080/#/c/13207/11/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/13207/11/be/src/scheduling/cluster-membership-mgr.cc@256 PS11, Line 256: update.is_delta = true; > why do we set this to true explicitly? Technically because it's the right thing to do: This coordinator only provides a delta to the global state. The statestore doesn't require this to be set, but doing so allows us to use the update in tests without modifying it, e.g. to pass it to other ClusterMembershipMgr instances. I added a comment -- To view, visit http://gerrit.cloudera.org:8080/13207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3cf9a8bb060d0c6e9ec8868b7b21ce01f8740a3 Gerrit-Change-Number: 13207 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 29 May 2019 17:33:42 +0000 Gerrit-HasComments: Yes
