Yuqi Du has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18674 )

Change subject: [threadpool] Fix scheduler thread a coredump bug
......................................................................


Patch Set 7:

(1 comment)

Thanks your crs.

http://gerrit.cloudera.org:8080/#/c/18674/4/src/kudu/util/threadpool.cc
File src/kudu/util/threadpool.cc:

http://gerrit.cloudera.org:8080/#/c/18674/4/src/kudu/util/threadpool.cc@154
PS4, Line 154:       shared_ptr<ThreadPoolToken> token = 
task.thread_pool_token_;
             :       while (true) {
             :         Status s = token->Submit(task.f);
             :         if (s.ok()) {
             :           break;
             :         }
             :         if (s.IsServiceUnavailable()) {
             :           if (!token->MaySubmitNewTasks()) {
             :             // threadpool token is Shutdown, skip the task.
             :             break;
             :           }
             :           // If capacity full, retry submit the task again, 
generally not reach the branch.
             :           VLOG(1) << Substitute("threadpool token Submit status: 
$0", s.ToString());
             :           SleepFor(MonoDelta::FromMilliseconds(1));
             :         } else {
             :
> This looks a bit fishy to me.  I'd expect that the outer loop with 'shutdow
ThreadPoolToken* token and SchedulerThread' shutdown_ is not the same thing. 
It's possible token is shutdown but SchedulerThread is running.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5bc5511a745f3dc12dfe1a6a1813ece41ccc2a8
Gerrit-Change-Number: 18674
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Fri, 01 Jul 2022 08:58:09 +0000
Gerrit-HasComments: Yes

Reply via email to