Yuqi Du 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 6:

(5 comments)

Thanks your crs.

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:   int delay_ms = 1 + (static_cast<int>(current_time) % 3);
> what's the purpose pf mode 3?
To reduce ScheduleThread's length, to avoid queue too large. Fix it another 
form.


http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc@206
PS5, Line 206:
> why sleep here? is there any other way to make sure all works done?
To SchedulerThread run 4-5 times, the purpose not do all the works.


http://gerrit.cloudera.org:8080/#/c/18867/5/src/kudu/util/threadpool-test.cc@232
PS5, Line 232:
> nit: you can use ASSERT_OK dirrectly, then you can get Status detailes if a
Done


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:  queue_time_t
> nit: 'scheduler_'.
Done


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
'scheduler()' is not significate, it has deleted.



--
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: 6
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: Mon, 29 Aug 2022 09:11:44 +0000
Gerrit-HasComments: Yes

Reply via email to