Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18447 )
Change subject: [threadpool] KUDU-3364 Add TimerThread for thread pool ...................................................................... Patch Set 20: (6 comments) Thanks for you crs. http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.h File src/kudu/util/threadpool.h: http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.h@172 PS19, Line 172: bool empty() const { > nit: delare it as a const function. Done http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.h@280 PS19, Line 280: // isn't set, returns 'false'. Otherwise, returns whether the queue is > If it is only used in tests, better to deaclare it as a private function. Done http://gerrit.cloudera.org:8080/#/c/18447/17/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/18447/17/src/kudu/util/threadpool.cc@141 PS17, Line 141: while (!shutdown_.WaitFor(MonoDelta::FromMilliseconds(schedule_period_ms_))) { > So, is upper_bound OK? upper_bound is ok. Has fixed. http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.cc@127 PS19, Line 127: hedule > nit: can be omit. Done http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.cc@128 PS19, Line 128: > nit: 'scheduler' is OK, to keep the same style to other Threads. Done http://gerrit.cloudera.org:8080/#/c/18447/19/src/kudu/util/threadpool.cc@142 PS19, Line 142: MonoTime now = MonoTime::Now(); > nit: `using std::vector;` in the header of the file and use vector directly Done -- To view, visit http://gerrit.cloudera.org:8080/18447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If51fce48ae6a45a0bf3314f8a102ee373c5c442e Gerrit-Change-Number: 18447 Gerrit-PatchSet: 20 Gerrit-Owner: Yuqi Du <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Khazar Mammadli <[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: Tue, 17 May 2022 03:38:43 +0000 Gerrit-HasComments: Yes
