Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18451 )
Change subject: [client] KUDU-3351: Add ResourceMetricsPB into WriteResponsePB ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/18451/7/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/18451/7/src/kudu/client/client-test.cc@2939 PS7, Line 2939: const KuduSession* s > Done OK, thanks: const pointer will do here as well -- it's a just a test after all. http://gerrit.cloudera.org:8080/#/c/18451/7/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: http://gerrit.cloudera.org:8080/#/c/18451/7/src/kudu/integration-tests/exactly_once_writes-itest.cc@178 PS7, Line 178: esponse.has_resource_metrics()) { > You are right. The rest of responses actually match. Thanks! http://gerrit.cloudera.org:8080/#/c/18451/9/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: http://gerrit.cloudera.org:8080/#/c/18451/9/src/kudu/integration-tests/exactly_once_writes-itest.cc@183 PS9, Line 183: response.release_resource_metrics(); I didn't notice that in previous review cycle, but instead here should be response.clear_resource_metrics(); Otherwise, it's a memory leak -- release returns the underlying structure along with transferring the ownership of the allocated structure to the caller. -- To view, visit http://gerrit.cloudera.org:8080/18451 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9adefd64b0058c66274a00e1b12334653fcab2b3 Gerrit-Change-Number: 18451 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Fri, 29 Apr 2022 21:00:51 +0000 Gerrit-HasComments: Yes
