Alexey Serbin has posted comments on this change. Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup ......................................................................
Patch Set 7: Code-Review+2 (3 comments) > (6 comments) > > > (7 comments) > > > > I have the following question: why do we need to introduce that > > additional state into ConsensusMetadata object? Why not to leave > > the OVERWRITE/NO_OVERWRITE parameter for its Flush() method? > > After discussing this with you offline, I agree that we can > simplify the implementation and avoid this additional state. I'll > back out this portion of the patch. Thank you for addressing my concerns! As for now, LGTM, just a couple of nits. Feel free to ignore, though. http://gerrit.cloudera.org:8080/#/c/7988/7//COMMIT_MSG Commit Message: Line 40: nit: do you want to mention new tests for CreateOrLoad()? http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager-test.cc File src/kudu/consensus/consensus_meta_manager-test.cc: PS7, Line 138: { nit: this additional scope might be removed, I think. http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: PS6, Line 127: using consensus::ConsensusMetadataCreateMode; > This one is needed :) oh, now I see -- that's for LoadOrCreate() call; it seems I didn't scroll down enough to see that :) -- 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: 7 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
