Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15224 )
Change subject: [test] fix flake in TsTabletManagerITest::TestTableStats ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/15224/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc File src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc: http://gerrit.cloudera.org:8080/#/c/15224/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc@1100 PS4, Line 1100: !s.ok() && > Don't need. Done http://gerrit.cloudera.org:8080/#/c/15224/4/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/15224/4/src/kudu/integration-tests/ts_tablet_manager-itest.cc@824 PS4, Line 824: !s.ok() && s.IsIllegalState() > This is duplicative; the test on s.IsIllegalState() is sufficient. Done http://gerrit.cloudera.org:8080/#/c/15224/4/src/kudu/integration-tests/ts_tablet_manager-itest.cc@828 PS4, Line 828: continue; > Do we want to CheckStats here too? Or do so after waiting for kMaxElectionT If the idea of this scope is checking stats after each election, probably we could do it. However, the idea behind this patch was simply to skip the rare case when the behavior is not of the expected one. From that perspective, I don't think calling CheckStats() is necessary here. -- To view, visit http://gerrit.cloudera.org:8080/15224 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibbd6652cdcb30f8899767dfb932cc402cf739115 Gerrit-Change-Number: 15224 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 18 Feb 2020 03:52:00 +0000 Gerrit-HasComments: Yes
