David Ribeiro Alves has posted comments on this change. Change subject: Create ConsensusMetadataManager ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7191/7/src/kudu/consensus/consensus_meta_manager.cc File src/kudu/consensus/consensus_meta_manager.cc: Line 44 > > Isn't interleaving on Create() already a disallowed operation? in that case, if a load races with a create, can it see partially written data? same with a delete. it seems like concurrent creates/deletes can't happen, but what if a load happens while data is being mutated on disk? PS7, Line 51: nit: can't you just lock in CancelOpInProgress? http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: PS2, Line 169: meta > I did consider it but I think if we want to do that it should be a separate maybe just on the public method then. http://gerrit.cloudera.org:8080/#/c/7191/2/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: PS2, Line 1067: const scoped_refptr<consensus::ConsensusMetadataService>& > Why? Passing it this way provides useful information, i.e. that this is ref yeah, but the caller might then think that this method increases the refcount, which it doesn't, and that it no longer need to keep a refcount itself. -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> 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