Todd Lipcon has posted comments on this change. Change subject: maintenance_manager: schedule work immediately when threads are free ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/6815/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: PS1, Line 43: DEFINE_int32(maintenance_manager_num_threads, 1, : "Size of the maintenance manager thread pool. " : "For spinning disks, the number of threads should " : "not be above the number of devices."); > should we change this default now? I don't think so. If anything, this change should make it so that people need fewer MM threads to do the same amount of work, rather than more. PS1, Line 221: or it is time to run another op. > The "shutting down" part is fairly intuitive, but this part isn't. It might Done. Also renamed the var, maybe better this time. PS1, Line 231: if (!FLAGS_enable_maintenance_manager) { : KLOG_EVERY_N_SECS(INFO, 30) << "Maintenance manager is disabled. Doing nothing"; : return; : } > not your fault but maybe move this above the inner while loop (before LOC 2 Done PS1, Line 238: // If we found no work to do, then we should sleep before trying again to schedule. : // Otherwise, we can go right into trying to find the next op. : force_sleep_next_iter = (op == nullptr); > since we're already checking whether op is null below maybe it would be cle but then we also need to set it to false after the if(){} block, right? -- To view, visit http://gerrit.cloudera.org:8080/6815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I63c4b48f5f02f3a1d3a8964993e78037ce72b1da Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
