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

Change subject: [threadpool] Fix unsafe behaviour when SchedulerThread shutdown
......................................................................


Patch Set 17:

(4 comments)

Thanks your crs.

http://gerrit.cloudera.org:8080/#/c/18867/16//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18867/16//COMMIT_MSG@9
PS16, Line 9: When shutting down a thread pool with Schedule
> Done
Done


http://gerrit.cloudera.org:8080/#/c/18867/16//COMMIT_MSG@9
PS16, Line 9: When shutting down a thread pool with SchedulerThread, thread 
pool's variable
            : 'scheduler_' is deleted and set to nullptr, at the same time, if 
thread pool
            : token is not shutdown and a task is scheduled on the toke
> Could you explain what scenario we need to call token->Schedule() after shu
There are 3 participants:
1. user threads, which use token->Scheduler(...), set a delayed task after a 
period time.
2. threadpool' threads, which will run the delay tasks or normal tasks.
3. threadpool's scheduler  thread,  step 1's delayed tasks will firstly be 
pushed into scheduler thread's queue, and then every period take some tasks to 
run.

The  safe shutdown orders as  'class SchedulerThread {' comments:
1. thread pool shutdown: to shutdown scheduler thread. (The following codes 
will do threadpool's shutdown)
2. token shutdown and release
3. pool shutdown(In fact 1 has shutdown) and release

At step 1, it takes a small period time.
First scheduler_thread shutdown, then execute shutdown logic codes of thread 
pool before.

When scheduler_thread shutdown finished, but thread pool is not shutdown.  
thread pool token can be used by user threads and token->Scheduler(...) can be 
called, but at the time, scheduler_thread is nullptr, so core dump would happen 
at https://gerrit.cloudera.org/c/18867/17/src/kudu/util/threadpool.cc#b267.


http://gerrit.cloudera.org:8080/#/c/18867/16//COMMIT_MSG@11
PS16, Line 11: uled on the tok
> Done
Done


http://gerrit.cloudera.org:8080/#/c/18867/16//COMMIT_MSG@14
PS16, Line 14: fix
> Done
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I021422f7e51e1007c5bdbc877ab445f70ba12357
Gerrit-Change-Number: 18867
Gerrit-PatchSet: 17
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Tue, 01 Nov 2022 07:58:39 +0000
Gerrit-HasComments: Yes

Reply via email to