Alexey Serbin has posted comments on this change. Change subject: [c++ client] implemented session operations stats ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/4974/2/src/kudu/client/client.h File src/kudu/client/client.h: Line 1503: /// @return Total count of write operations in the session. > What do you think of reusing kudu::client::ResourceMetrics for this purpose Thank you for the reference. It seems the ResourceMetrics class is heavier and all its operations require synchronization. I'm not sure that's the best choice for the counter which is put into the hot paths with the KuduSession::Apply() calls. I think it's better to have something more lightweight for these simple counters. http://gerrit.cloudera.org:8080/#/c/4974/2/src/kudu/client/error_collector.cc File src/kudu/client/error_collector.cc: Line 21: #include <type_traits> > What is this for? This is where std::move() comes from. http://gerrit.cloudera.org:8080/#/c/4974/2/src/kudu/client/write_op_stats.h File src/kudu/client/write_op_stats.h: Line 20: #include <atomic> > I think the jury is still out regarding std::atomic vs. our built-in atomic All right, will use the in-house atomics then. -- To view, visit http://gerrit.cloudera.org:8080/4974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb4191105158f49d811763fdc8e82831b4e2e0be Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
