Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9246 )

Change subject: KUDU-2274. consensus: Remove ConsensusMetadata thread safety
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9246/1/src/kudu/consensus/consensus_meta.h
File src/kudu/consensus/consensus_meta.h:

PS1:
Is this just a WIP patch?

There are multiple methods in this file called xxx_unlocked() and 
XxxUnlocked().  By my understanding, if it's not a WIP patch, it would be nice 
to get rid of names asserting on thread-safe behavior of this class, removing 
variations of the unlocked methods.

If I'm not mistaken, it's possible to embed methods protected by scoped fake 
lock.


http://gerrit.cloudera.org:8080/#/c/9246/1/src/kudu/consensus/consensus_meta.h@236
PS1, Line 236: TODO(mpercy) Doc this
Indeed: it would be nice to explain the reasoning behind having this fake mutex.


http://gerrit.cloudera.org:8080/#/c/9246/1/src/kudu/consensus/consensus_meta.h@237
PS1, Line 237: lock_
nit: maybe, rename it to fake_lock_?



--
To view, visit http://gerrit.cloudera.org:8080/9246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8d086c3fba52826ebe0d3a44842d53ecb6a9265
Gerrit-Change-Number: 9246
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Feb 2018 20:42:45 +0000
Gerrit-HasComments: Yes

Reply via email to