Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14578 )
Change subject: [master] KUDU-2963 Fix and enhance TestCreateWhenMajorityOfReplicasFailCreation ...................................................................... Patch Set 4: (3 comments) Whoops, it seems I forgot to publish my review yesterday when I took a look at it. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc File src/kudu/integration-tests/create-table-itest.cc: http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@86 PS3, Line 86: GetNumCreateTabletRPCs I think it's necessary to wrap every call to this function with NO_FATALS(), otherwise if GetInt64Metrics() fails, the failure would be found only later on, and non-valid results would be used. An alternative approach would be making this function return Status and keep ASSERT_OK() at call sites. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@144 PS3, Line 144: bool in_progress = true; > Nice. Indeed. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@145 PS3, Line 145: while (in_progress) { : LOG(INFO) << "Waiting for the master to successfully create the table..."; : SleepFor(MonoDelta::FromMilliseconds(100)); : ASSERT_OK(client_->IsCreateTableInProgress(kTableName, &in_progress)); : } nit: this might be replaced with ASSERT_EVENTUALLY: ASSERT_EVENTUALLY([&]{ bool in_progress = true; ASSERT_OK(client_->IsCreateTableInProgress(kTableName, &in_progress)); ASSERT_FALSE(in_progress); }); -- To view, visit http://gerrit.cloudera.org:8080/14578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9943200dbf330481996e58e27744fe9fb49267f Gerrit-Change-Number: 14578 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 30 Oct 2019 21:13:34 +0000 Gerrit-HasComments: Yes
