You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/01/02 17:39:16 UTC

[kudu-CR] [util] use lighter locking scheme for ThreadMgr

Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12112 )

Change subject: [util] use lighter locking scheme for ThreadMgr
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12112/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12112/8//COMMIT_MSG@49
PS8, Line 49:   As an additional piece of joy, it turned out that it's impossible to
            :   handle worker's crash in a multiprocessing.Pool, and that's by design.
            :   When a worker process crashes while holding the call_queue.rlock,
            :   no other process will ever be able to read the call_queue anymore,
            :   breaking the Pool as it cannot communicate with its workers.
Nice find. Is this the same as https://stackoverflow.com/questions/24894682/python-multiprocessing-crash-in-subprocess? If so, you could elide some of the explanation and link to that.


http://gerrit.cloudera.org:8080/#/c/12112/8/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:

http://gerrit.cloudera.org:8080/#/c/12112/8/src/kudu/util/thread.cc@226
PS8, Line 226:   typedef map<string, ThreadCategory> ThreadCategoryMap;
Should this also be unordered_map? Out of scope for this change?


http://gerrit.cloudera.org:8080/#/c/12112/8/src/kudu/util/thread.cc@335
PS8, Line 335:     thread_categories_[category][pthread_id] =
             :         ThreadDescriptor(category, name, tid);
May be worth a comment saying why we don't EmplaceOrDie here (the bit about forking after creating a kudu::Thread and how pthread identifiers may be reused).



-- 
To view, visit http://gerrit.cloudera.org:8080/12112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d49c1c39392e01c45019844430a4fe3d116c277
Gerrit-Change-Number: 12112
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 02 Jan 2019 17:39:16 +0000
Gerrit-HasComments: Yes