Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22016 )
Change subject: [tablet] use STL-based atomics in MemRowSet and MRSRow ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/22016/1/src/kudu/tablet/memrowset.cc File src/kudu/tablet/memrowset.cc: http://gerrit.cloudera.org:8080/#/c/22016/1/src/kudu/tablet/memrowset.cc@223 PS1, Line 223: live_row_count_.fetch_add(1, std::memory_order_relaxed); > [nit] Maybe add a comment that AnchorIfMinimum contains a lock, which ensur This is just a counter, so the only requirement in this particular place is atomicity (i.e. no updates are lost), and std::memory_order_relaxed is enough to guarantee so. Or your concern is about the behavior of the underlying unsigned type uint64_t and re-ordering of fetch_add() and fetch_sub() when the value of the underlying atomic counter is 0? The only question of synchronization and ordering happens when the counter value is read, i.e. in MemRowSet::CountLiveRows(). By design, this counter has metrics-like approximate semantics since the way how MemRowSet::CountLiveRows() is used cannot protect against TOCTOU, unless the memrowset is no longer active when it's being flushed, IIUC. You could check for more details at https://github.com/apache/kudu/commit/c1c8e25bbc2e783446f1dd257cd99a16a79ccb16 Does it make sense? If so, I can add the corresponding blurb at the declaration of the live_row_count_ field in memrowset.h unless you have some better ideas of what comment to add where. Please let me know. -- To view, visit http://gerrit.cloudera.org:8080/22016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b85fc07b4c81bc3b3cf736d7f14b424ceef81a4 Gerrit-Change-Number: 22016 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Tue, 05 Nov 2024 02:13:28 +0000 Gerrit-HasComments: Yes
