Andrew Wong has posted comments on this change. Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.cc File src/kudu/consensus/consensus_meta_manager.cc: PS8, Line 109: RETURN_NOT_OK_PREPEND(ConsensusMetadata::DeleteOnDiskData(fs_manager_, tablet_id), : Substitute("Unable to delete consensus metadata for tablet $0", tablet_id)); Doesn't this fail if the cmeta is in-memory only? Is that something we should be worried about? http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager.h File src/kudu/consensus/consensus_meta_manager.h: PS7, Line 71: ConsensusMetadataCreateMode create_mode = : ConsensusMetadataCreateMode::FLUSH_ON_CREATE, > That's something that is caught by the cache check, so the first call will Hrm I'm specifically asking about LoadOrCreate, not Create--the first call Creates and doesn't flush, the second loads from cache. And in the second call, the create_mode doesn't matter. Not an actionable comment, just checking my understanding :) http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.h File src/kudu/consensus/consensus_meta_manager.h: PS8, Line 62: the cache nit: hrmm, since "the cache" might be confused with something Kudu-specific (e.g. block cache, or other caches if we ever implement them), consider "in-memory"? I don't feel too strongly about this, but thought I'd point it out. PS8, Line 67: otherwise create i nit: Maybe also note the insignificance of the parameters if a Load is expected? http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: PS7, Line 1088: guarantees that the replica has never voted in a leader election. > It's critical. If it's not true, we lose our Raft safety guarantees and spl Makes sense; so for the "normal" use case, this assumption is not so much necessary as it is just how things are, and if the assumption doesn't hold, we might end up in some bizarro Raft scenario. -- To view, visit http://gerrit.cloudera.org:8080/7988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b Gerrit-PatchSet: 8 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: Andrew Wong <aw...@cloudera.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
