Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16332 )
Change subject: KUDU-1587 part 1: load meter for ThreadPool ...................................................................... Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/16332/11/src/kudu/util/threadpool-test.cc File src/kudu/util/threadpool-test.cc: http://gerrit.cloudera.org:8080/#/c/16332/11/src/kudu/util/threadpool-test.cc@569 PS11, Line 569: SleepFor(MonoDelta::FromMilliseconds(10)); > What's the rationale behind the different low-value sleeps? I guess this is a remnant from a prior version when the tasks were sleeping for a millisecond. Removed. http://gerrit.cloudera.org:8080/#/c/16332/11/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/16332/11/src/kudu/util/threadpool.cc@605 PS11, Line 605: // The uninitialized value of MonoTime for the next task to be run means : // it's an empty queue, so no task to be run. > Would this have already been called by whatever dequeued the last task? I.e This might be true if we had a guarantee that DispatchThreads() always finds a item in the queue. I wasn't sure about that, so I added this piece. I'll take a closer look at this. http://gerrit.cloudera.org:8080/#/c/16332/11/src/kudu/util/threadpool.cc@862 PS11, Line 862: queue_head_submit_time_.store(queue_head_submit_time); : has_spare_threads_.store(has_spare_threads); : UpdateOverloadedStateUnlocked(); : VLOG(4) << Substitute("queue info snapshot: { $0, $1, $2 }", : has_spare_threads, : queue_head_submit_time_.load().ToString(), : overloaded_since_.load().ToString()); > nit: calling UpdateQueueInfoUnlocked() here instead would clarify the relat Good point! Done. -- To view, visit http://gerrit.cloudera.org:8080/16332 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I640716dc32f193e68361ca623ee7b9271e661d8b Gerrit-Change-Number: 16332 Gerrit-PatchSet: 11 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 26 Aug 2020 20:30:40 +0000 Gerrit-HasComments: Yes
