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

Reply via email to