Adar Dembo has posted comments on this change.

Change subject: locks: add new read-write mutex

Patch Set 4:

(1 comment)

> (1 comment)
 > LGTM with some concern regarding error detection if something goes
 > wrong.
 > So, we are about to catch some those non-run-time issues like
 > trying to acquire the lock held by the same thread, etc. in debug
 > mode, which is OK.
 > Do we expect to catch some run-time issues like lack of shared
 > memory to accommodate a new lock, etc.?  I.e. we are not
 > propagating errors from pthread_rwlock_xxx() functions in release
 > mode at all (if I'm not missing something).   Does it make sense to
 > throw some sort of exception for run-time errors (like
 > std::runtime_error)?

It probably does (I'm not 100% sure, would need to give it more thought), but 
I'd rather keep that out of scope of this patch. The behavior of this new lock 
is consistent with that of our other pthread-based locks.

If we're going to address the issue of runtime checking of pthread return 
values, I think it makes sense to do it across the board in a separate patch.
File src/kudu/util/

Line 92:   ; // NOLINT(whitespace/semicolon)
> As Dan already mentioned: is it possible to keep the semicolon under the if
Yes, but I actually messed up when I published this version of the diff. I 
meant to pull the DCHECK_EQ() out of the NDEBUG block. See the next revision.

If it's pulled out, it's no longer possible to put the semicolon on the end of 
the line, at least not without duplicating the DCHECK_EQ().

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to