Dinesh Bhat has posted comments on this change.

Change subject: [c++ client] implemented session operations stats
......................................................................


Patch Set 2: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4974/2/src/kudu/client/write_op_stats.h
File src/kudu/client/write_op_stats.h:

PS2, Line 37: ++total_
The standard seem to suggest that ++ operation follows strict synchronization 
semantics about memory ordering. If we know that this is always getting 
incremented, we could use :
total_.fetch_add(1, std::memory_order_relaxed);


PS2, Line 37: IncTotal
Nit: suggest using a nomenclature which could reflect that this is an atomic 
operation vs a regular increment op.


PS2, Line 40: total_
I wonder if this is equivalent to total_.load(memory_order_relaxed) ?


PS2, Line 49: total_count_
Do you want to point out where is this variable declared ?


-- 
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

Reply via email to