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 43: (2 comments) Thanks your time and crs. http://gerrit.cloudera.org:8080/#/c/18674/43/src/kudu/util/threadpool.h File src/kudu/util/threadpool.h: http://gerrit.cloudera.org:8080/#/c/18674/43/src/kudu/util/threadpool.h@168 PS43, Line 168: token->Schedule > What about handling the return status? The return status can deal by users according to their scenarios. Generally it return ok, only token is shutdowned, it return not ok(happenning shutdowning). http://gerrit.cloudera.org:8080/#/c/18674/43/src/kudu/util/threadpool.h@172 PS43, Line 172: // ThreadPoolToken* t = token.release(); // delete the token(s). : // delete t; : // ThreadPool* pool = scheduler_pool.release(); // release scheduler_pool. : // delete pool; > Why is this necessary when both 'token' and 'scheduler_pool' are unique_ptr The reason of scheduler_pool.Shutdown before token release, the scheduler_pool has a SchedulerThread, it has some tasks will execute in future, which need token is not null, so safe way to do this , is firstly shutdown the SchedulerThread. In fact, The threadpool's other threads can continue run more time. (Next, I want to make threadpool and schedule thread loosely coupled.) token release before threadpool is requirement by before. -- 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: 43 Gerrit-Owner: Yuqi Du <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Mon, 25 Jul 2022 04:06:55 +0000 Gerrit-HasComments: Yes
