Andrew Wong 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? 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. could we move the UpdateQueueInfoUnlocked() call into the if block? 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 relationship between TaskDequeuedUnlocked() and UpdateQueueInfoUnlocked(). -- 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 18:56:26 +0000 Gerrit-HasComments: Yes
