Alexey Serbin has posted comments on this change. Change subject: [catalog manager] fixed deadlock on catalog shutdown ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/6134/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 722: ConsensusStatePB cstate = consensus->ConsensusState(CONSENSUS_CONFIG_COMMITTED); > Need to null check here. Done PS2, Line 838: "Not loading sys catalog metadata"; > wrap this properly and add "<<" before the new line's text. This is not a part of this changelist anymore. PS2, Line 907: Status CatalogManager::InitCertAuthority() { > oh I see that you're refactoring. likely worth doing in a previous patch (i This is not a part of this changelist anymore. http://gerrit.cloudera.org:8080/#/c/6134/2/src/kudu/master/catalog_manager.h File src/kudu/master/catalog_manager.h: PS2, Line 571: InitCertAuthority > docs This is not a part of this changelist anymore. PS2, Line 571: Status InitCertAuthority(); : Status InitTokenSigner(); > did you meant this to be here? This is not a part of this changelist anymore. PS2, Line 633: // Check if it's time to generate a new Token Signing Key for TokenSigner. : // If so, generate one and persist it into the system table. After that, : // push it into the TokenSigner's key queue. : Status TryGenerateNewTskUnlocked(); > same, seems unrelated to this change This is not a part of this changelist anymore. PS2, Line 759: // Check if the specified status should be considered as a fatal error for : // a leader master. Errors happened during shutdown are not considered fatal. : void CheckIfFatalLeaderError(const Status& s) const; > this is very hairy. The way we treat error right now means that this would This is not a part of this changelist anymore. -- To view, visit http://gerrit.cloudera.org:8080/6134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I10ad66fe33d4696adf2a02a09e2790afa8869583 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[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
