Alexey Serbin has posted comments on this change. Change subject: [catalog manager] fixed deadlock on catalog shutdown ......................................................................
Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/6134/4//COMMIT_MSG Commit Message: PS4, Line 7: [catalog manager] fixed deadlock on catalog shutdown > well, you could always pstack the daemon and see where the threads were stu Those I do have -- I grabbed the stacks of all the threads using gdb while was trying to understand what was going on. If you want, I can post those, but I'm not sure it makes much sense in this context. http://gerrit.cloudera.org:8080/#/c/6134/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS4, Line 1022: // table (e.g. write newly > yes, adar got it right. sorry about the confusion oh, I see. I should have guessed :) And it seems I did, but I forgot to update this comment. PS4, Line 1024: because > oh, you're right. My bad here. We so refer to "consensus implementation" or Thank you for the clarification! It seems my use of 'consensus' was too ambiguous anyway. So, here here it should be 'consensus implementation', if I understand correctly :) PS4, Line 1030: eaderCb tas > this discussion came up a few times. Initially when I implemented it I used ok, I see. I'll replace with just 'quorum' -- does it make sense? Because the write operation cannot be completed unless it's acknowledged by the majority of the peers (i.e. quorum). However, replacing with 'majority of the peers' seems a better choice to me. http://gerrit.cloudera.org:8080/#/c/6134/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS6, Line 976: std::lock_guard<simple_spinlock> l(state_lock_); : DCHECK_EQ(kRunning, state_); : if (state_ == kRunning) { : return sys_catalog_->tablet_peer()->consensus()->role(); : } : return RaftPeerPB::UNKNOWN_ROLE; > this shouldn't likely DCHECK since it might be called on init/shutdown. Good catch! I'll update the code as suggested. -- 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: 6 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
