Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20657 )
Change subject: IMPALA-12525: Fix flaky test test_statestored_manual_failover ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/20657/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20657/4//COMMIT_MSG@12 PS4, Line 12: During test, the network of active statestored could be disabled before nit: a blank line between paragraphs would be nice http://gerrit.cloudera.org:8080/#/c/20657/4//COMMIT_MSG@30 PS4, Line 30: ready to accept registraion requests. This caused impalad hit DCHECK. typo in registration http://gerrit.cloudera.org:8080/#/c/20657/4/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/20657/4/be/src/statestore/statestore-subscriber.cc@1108 PS4, Line 1108: bool StatestoreSubscriber::StatestoreStub::IsStatestoreActive() { This function now seems to be unused. http://gerrit.cloudera.org:8080/#/c/20657/4/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/20657/4/be/src/statestore/statestore.cc@2039 PS4, Line 2039: } else if (majority_failed) { This logic could be simplified a little. Since "state != FAILED" always continues, we don't need to use "else if", which would visually distinguish handling all the failure cases. We can also move all the "found_peer_ = false; connected_peer_metric_->SetValue(is_active_);" logic out of the conditionals (and avoid repeating it) since it happens at the end for all cases. http://gerrit.cloudera.org:8080/#/c/20657/4/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/20657/4/tests/custom_cluster/test_statestored_ha.py@668 PS4, Line 668: assert(statestoreds[0].service.get_metric_value("statestore.active-status")) Is the order of this guaranteed? -- To view, visit http://gerrit.cloudera.org:8080/20657 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If03bf09d22a2875d2c1eec8a4f62eeefc5d855dc Gerrit-Change-Number: 20657 Gerrit-PatchSet: 4 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 07 Nov 2023 22:27:19 +0000 Gerrit-HasComments: Yes
