Alexey Serbin has posted comments on this change. Change subject: KUDU-871 (part 1). Refactor to make cmeta a first class object ......................................................................
Patch Set 1: (6 comments) some nits while skimming through http://gerrit.cloudera.org:8080/#/c/6957/1//COMMIT_MSG Commit Message: PS1, Line 35: Remove unused Consensus::State enum. nit: maybe, it makes sense to publish this change as separate tiny clean-up changelist? PS1, Line 37: * bug fix: RaftConsensus must hold lock before snoozing FD nit: if it's a bug fix, would it make sense to separate it and make sure all current tests pass before introducing such a major refactoring? Or it becomes a bug only when combined with the new refactored code? http://gerrit.cloudera.org:8080/#/c/6957/1/src/kudu/tablet/metadata.proto File src/kudu/tablet/metadata.proto: PS1, Line 77: TABLET_DATA_FRESH Given the description I would expect this is a broader counterpart of TABLET_DATA_COPYING. Is that true or there are some additional nits regarding voting, etc.? http://gerrit.cloudera.org:8080/#/c/6957/1/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: PS1, Line 244: $0Unable typo? it seems space is missing PS1, Line 593: *tablet_replica = tmp_replica; Would it make sense to allow the 'tablet_replica' parameter be nullptr? It seems in a couple of places in this file the result tablet replica output by this method is not used at this method's call sites. PS1, Line 1064: $0Unable typo? -- 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: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
