Todd Lipcon has posted comments on this change. Change subject: Create ConsensusMetadataManager ......................................................................
Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/7191/16/src/kudu/consensus/consensus_meta_manager.h File src/kudu/consensus/consensus_meta_manager.h: Line 67: Status Delete(const std::string& tablet_id); still not quite sure what the "expected" concurrency is here in the class. Specifically, what happens with the following: thread A: local_ref = Load("foo") thread B: Delete("foo") thread B: local_ref2 = Create("foo") thread A: local_ref->Flush() now we have two different refs pointing to the same instance. Do we need to invalidate the existing 'local_ref' in Delete so that we don't get into the situation where we have multiple ConsensusMetadata instances alive for the same tablet? Put another way, I agree that this class is thread-safe from the perspective of crashes, etc, but I am not sure what the expected _usage_ of it is, and how/whether it is protecting from different unexpected interleavings -- To view, visit http://gerrit.cloudera.org:8080/7191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia30c05dd0feec2b7530205f4d17dfc079a1c3451 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
