Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15224 )
Change subject: [test] fix flake in TsTabletManagerITest::TestTableStats ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/15224/3/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/3/src/kudu/integration-tests/ts_tablet_manager-itest.cc@820 PS3, Line 820: auto s = LeaderStepDown(tserver, replica->tablet_id(), > I wonder how pervasive this issue is amongst our tests. Seems like every AS Yep, it's a good call. I also thought about this and it seems in several tests it's an issue. In some places ASSERT_EVENTUALLY() would help, in others (line this one) something else should be done. I was not sure we want to address all such cases in this patch, but why not? :) I'll post an update re-visiting other places with LeaderStepDown() as well. http://gerrit.cloudera.org:8080/#/c/15224/3/src/kudu/integration-tests/ts_tablet_manager-itest.cc@826 PS3, Line 826: anymor > anymore Done http://gerrit.cloudera.org:8080/#/c/15224/3/src/kudu/integration-tests/ts_tablet_manager-itest.cc@829 PS3, Line 829: } > Do you want to ASSERT_OK(s) after this if statement? And do you still want Whoops, indeed. Good catch: it seems the line with SleepFor() was accidentally removed. -- 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: 3 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 15 Feb 2020 03:53:44 +0000 Gerrit-HasComments: Yes