Todd Lipcon has posted comments on this change. Change subject: KUDU-871 (part 1). Refactor to make cmeta a first class object ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/6957/1//COMMIT_MSG Commit Message: PS1, Line 16: 1) Allow RaftConsensus to operate while not initialized or in a shutdown : state. Also pass ConsensusMetadata into the RaftConsensus : constructor. not 100% following this. I thought when discussing this, the idea was to do one or the other of these changes, not both? i.e., if you make it so you can instantiate RaftConsensus even for tombstoned tablet replicas, then why isn't it sufficient to still keep ConsensusMeta confined to being within the RaftConsensus class? Or alternatively, if you make TabletReplica aware of ConsensusMetadata, why do you need to make changes to the RaftConsensus lifecycle? PS1, Line 41: * Add additional lifecycle runtime assertions in TabletReplica and : RaftConsensus. Can you also clearly document the lifecycle for RaftConsensus? I'm not entirely following it from the patch. eg when you first create a RaftConsensus object, it seems to exist but have a lot of nullptr members. But then when you do Shutdown() on it, it doesn't re-nullify those members. Does that imply that a RaftConsensus instance that was tombstoned on a live server has different state than one that was tombstoned at startup? Does that have impact on thread lifecycle or memory consumption? Put another way, the above commit message tries to describe the changes, but I'm not sure what the new "state of the world" is, and the class docs here aren't clarified enough to make me feel confident that the new state of the world is correct. -- To view, visit http://gerrit.cloudera.org:8080/6957 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia27a091d27b3996d37009d5ec866e744f9608388 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
