Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master ...................................................................... Patch Set 7: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405 PS4, Line 405: : ASSERT_OK(mini_master_->Restart()); : ASSERT_OK(mini_master_->WaitForCatalogManagerInit()); : run > Guess Alexey meant to use something like: Whatever works best to join the thread in case of assertion triggers. Current version in PS7 looks good to me. If I understood correctly, Attila suggested to use different boolean flags for different conditions leading to the necessity to join the thread. That could work as well. As is, the current version looks good and simple enough to allow the thread to join if any of the assertions after line 404 in PS7 are triggered. http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc File src/kudu/master/master.cc: http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc@300 PS7, Line 300: Status s = catalog_manager_init_status_.Get(); : return s; nit: it could be as simple as return catalog_manager_init_status_.Get(); -- To view, visit http://gerrit.cloudera.org:8080/17989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c Gerrit-Change-Number: 17989 Gerrit-PatchSet: 7 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 10 Nov 2021 20:06:44 +0000 Gerrit-HasComments: Yes
