Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18867 )
Change subject: [threadpool] Fix an extreme coredump bug when shutdown thread pool with SchedulerThread ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc File src/kudu/util/threadpool-test.cc: http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc@189 PS5, Line 189: if (static_cast<int>(current_time) % 3 == 0) { what's the purpose pf mode 3? http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc@206 PS5, Line 206: SleepFor(MonoDelta::FromMilliseconds(5)); why sleep here? is there any other way to make sure all works done? http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc@232 PS5, Line 232: ASSERT_TRUE nit: you can use ASSERT_OK dirrectly, then you can get Status detailes if assert failed. http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool.h File src/kudu/util/threadpool.h: http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool.h@618 PS5, Line 618: ‘scheduler_’. nit: 'scheduler_'. And it'd better to move it closer above to the place where 'scheduler_' is defined. http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool.cc@418 PS5, Line 418: MutexLock unique_lock(scheduler_lock_); scheduler_ can be obtained by scheduler(), is it possible to prevent it to be deleted outside of class ThreadPool? -- 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: 5 Gerrit-Owner: Yuqi Du <[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, 27 Aug 2022 09:28:50 +0000 Gerrit-HasComments: Yes
