Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14533 )
Change subject: KUDU-2962 Fix kudu::itest::FindTabletFollowers() test utility function ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc File src/kudu/integration-tests/cluster_itest_util.cc: http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc@668 PS1, Line 668: const auto& > const auto* Done http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc@694 PS1, Line 694: DCHECK(!cstate.leader_uuid().empty()); > In some intermittent states Raft consensus might have no leader. I think i There is already a check above L685 whether the tablet server returned leader uuid. Is it possible that has_leader_uuid() is true but the actual the leader_uuid() is empty? http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc@719 PS1, Line 719: DCHECK > It would be great if this method worked the same way for both debug and rel Done http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/tools/kudu-admin-test.cc@190 PS1, Line 190: TServerDetails * > nit: in Kudu source we tend to stick the asterisk to the type Done http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/tools/kudu-admin-test.cc@191 PS1, Line 191: MonoDelta::FromSeconds(30) > Maybe, introduce a constant and use it here and at line 193 as well? Done http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/tools/kudu-admin-test.cc@192 PS1, Line 192: TServerDetails * > ditto Done http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/tools/kudu-admin-test.cc@193 PS1, Line 193: tablet_servers_ > Does it make sense to add a 'negative' test case when the set of supplied t Added a separate negative test case. -- To view, visit http://gerrit.cloudera.org:8080/14533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94146873b09cb95c54175d16305b713365cfeaa8 Gerrit-Change-Number: 14533 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Oct 2019 18:39:42 +0000 Gerrit-HasComments: Yes
