Mike Percy has posted comments on this change. Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup ......................................................................
Patch Set 3: (11 comments) http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta.cc File src/kudu/consensus/consensus_meta.cc: PS3, Line 339: // Create() should not clobber. > I think the placement of this comment might be a bit confusing. Should this removed this comment http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager-stress-test.cc File src/kudu/consensus/consensus_meta_manager-stress-test.cc: PS3, Line 18: #include <stdint.h> > nit: prefer using <cstdint> Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.cc File src/kudu/consensus/consensus_meta_manager.cc: PS3, Line 97: if (s.ok()) > Did you consider narrowing the permissible errors here? e.g. if we hit an I Thanks for the catch. That's what I meant to do. Fixed. http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.h File src/kudu/consensus/consensus_meta_manager.h: PS3, Line 59: ConsensusMetadataCreateMode create_mode > What if this parameter was set to ConsensusMetadataCreateMode::FLUSH_ON_CRE Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/raft_consensus_quorum-test.cc File src/kudu/consensus/raft_consensus_quorum-test.cc: PS3, Line 191: scoped_refptr<ConsensusMetadata> cmeta; > nit: drop this and leave the last parameter of the Create() call below by d Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/integration-tests/tombstoned_voting-itest.cc File src/kudu/integration-tests/tombstoned_voting-itest.cc: PS3, Line 18: #include <stdint.h> > nit: consider replacing with <cstdint> and adding that into the list of the Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/master/sys_catalog.cc File src/kudu/master/sys_catalog.cc: PS3, Line 260: scoped_refptr<ConsensusMetadata> cmeta; > nit: consider dropping this since it's not used in the scope and leaving th Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_bootstrap-test.cc File src/kudu/tablet/tablet_bootstrap-test.cc: PS3, Line 146: scoped_refptr<ConsensusMetadata> cmeta; > Nit: it seems this is not used, consider dropping it and leaving the last p Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_replica-test.cc File src/kudu/tablet/tablet_replica-test.cc: PS3, Line 141: scoped_refptr<ConsensusMetadata> cmeta; > Nit: consider dropping this since it's not used, leaving the last parameter Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS3, Line 601: scoped_refptr<ConsensusMetadata> cmeta; > Nit: cmeta is not used in this scope, right? Consider dropping it. Done http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_source_session-test.cc File src/kudu/tserver/tablet_copy_source_session-test.cc: PS3, Line 157: scoped_refptr<ConsensusMetadata> cmeta; > nit: consider dropping cmeta -- it's not used in this scope elsewhere but p Done -- 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: 3 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: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
