Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19175 )
Change subject: KUDU-1698 Test that RPC and session timeout being separate entities ...................................................................... Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/19175/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19175/3//COMMIT_MSG@7 PS3, Line 7: being nit: are http://gerrit.cloudera.org:8080/#/c/19175/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19175/4//COMMIT_MSG@7 PS4, Line 7: being nit: are http://gerrit.cloudera.org:8080/#/c/19175/4//COMMIT_MSG@9 PS4, Line 9: nit: comma http://gerrit.cloudera.org:8080/#/c/19175/4//COMMIT_MSG@10 PS4, Line 10: nit: comma http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@912 PS4, Line 912: RPCTimeoutMs nit: rpcTimeoutMs. Also, maybe make it constexpr? http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@921 PS4, Line 921: succesful nit: successful http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@923 PS4, Line 923: MakeScopedCleanup There is a SCOPED_CLEANUP macro, it should be okay to use it here as we don't want to cancel this thread. http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@931 PS4, Line 931: cluster_->CreateClient(&builder, &client_); Maybe wrap it in an ASSERT_OK? http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@932 PS4, Line 932: auto ent = cluster_->mini_master()->master()->metric_entity(); Does this need to be created so early? Can we move it to just above where the metric is instantiated? http://gerrit.cloudera.org:8080/#/c/19175/4/src/kudu/client/client-test.cc@935 PS4, Line 935: unique_ptr<KuduInsert> insert; : insert = BuildTestInsert(table.get(), 2); : ASSERT_OK(session->Apply(insert.release())); can this be one line? -- To view, visit http://gerrit.cloudera.org:8080/19175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I177f5dcf04417edff8816690765a0c2b57e44036 Gerrit-Change-Number: 19175 Gerrit-PatchSet: 4 Gerrit-Owner: Ádám Bakai <aba...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan <zcho...@cloudera.com> Gerrit-Comment-Date: Thu, 10 Nov 2022 21:48:38 +0000 Gerrit-HasComments: Yes
