Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17743 )
Change subject: KUDU-1954 [maintenance] Add an optional flush-only thread pool ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@68 PS3, Line 68: specifica > nit: "specifically" Done http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@71 PS3, Line 71: maintenance_manager_num_threads, whic > maintenance_manager_num_threads? Done http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@535 PS3, Line 535: return {low_io_most_logs_retained_bytes_op, : std::move(notes), : LAUNCH_POOL(low_io_most_logs_retained_bytes_op->type(), flush_pool, general_pool)}; > Why bother evaluating which pool to pick here, rather than pushing that bur In my opinion now FindBestOp not only to find best op, and also find the thread pool it will execute. And I think hide the details in FindBestOp is better to keep the main loop in RunSchedulerThread clearer. -- To view, visit http://gerrit.cloudera.org:8080/17743 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6dca9d87e7287cc04482a1c18b1e0eefc5f9bcb1 Gerrit-Change-Number: 17743 Gerrit-PatchSet: 4 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sat, 14 Aug 2021 08:20:47 +0000 Gerrit-HasComments: Yes
