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 32: (4 comments) Thanks. http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool-test.cc File src/kudu/util/threadpool-test.cc: http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool-test.cc@195 PS29, Line 195: SleepFor(MonoDelta::FromMilliseconds(500)); > If all tasks in token finished, it's not needed to sleep, right? Yes. The case's purpose is to test when token is shutdown and the scheduler is running, scheduler run the task which would be execited by the token no coredump or fail. The sleep 1000ms can adjust to 500ms. http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool-test.cc@267 PS29, Line 267: > nit: remove the blank line. Done http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool.cc@159 PS29, Line 159: DCHECK(!s.IsServiceUnavailable( > nit: you can DCHECK(s.IsServiceUnavailable()) to reduce if..else.. and inde ok. DONE http://gerrit.cloudera.org:8080/#/c/18674/29/src/kudu/util/threadpool.cc@164 PS29, Line 164: > Better toi describe how it happened. Done -- 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: 32 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: Sat, 16 Jul 2022 10:17:37 +0000 Gerrit-HasComments: Yes
