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 <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
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

Reply via email to