Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15828 )
Change subject: [master-test] added GetTableSchema() microbenchmarks ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15828/1//COMMIT_MSG@13 PS1, Line 13: debug Are such wide disparities there in release mode as well? http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1110 PS1, Line 1110: if (resp.has_error()) { : LOG(WARNING) << SecureDebugString(resp); : continue; : } Do you think it's worth failing this test on error? That way we can more easily not use bogus results? http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1139 PS1, Line 1139: const bool supports_authz = GetParam(); : FLAGS_master_support_authz_tokens = supports_authz; : : for (auto idx = 0; idx < kNumTables; ++idx) { : auto table_name = Substitute("testtb_$0", idx); : EXPECT_OK(CreateTable(table_name, kTableSchema)); : } nit: consider putting these in some common SetUp()? http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1180 PS1, Line 1180: auto stopper_cleanup = MakeScopedCleanup([&]() { : for (auto& t : caller_threads) { : t.join(); : } : }); nit: just FYI, you can use this too if you'd like, especially if you don't need to cancel the cleanup -- may be slightly more ergonomic: SCOPED_CLEANUP({ /* do stuff */ }) http://gerrit.cloudera.org:8080/#/c/15828/1/src/kudu/master/master-test.cc@1191 PS1, Line 1191: calls nit: maybe "function calls"? an RPC is also a "call", so it wasn't quite clear to me at first what the difference was from the commit message. -- To view, visit http://gerrit.cloudera.org:8080/15828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1f3d4fe1df426d50bd465c8ca7bbfb6924bffe9 Gerrit-Change-Number: 15828 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 28 Apr 2020 01:13:44 +0000 Gerrit-HasComments: Yes
