----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62230/#review186298 -----------------------------------------------------------
Fix it, then Ship it! src/slave/gc.cpp Lines 84 (patched) <https://reviews.apache.org/r/62230/#comment262788> We already have ``` ~Executor() { terminate(process); wait(process); } ``` Does the `stop()` here make a difference? src/slave/gc.cpp Line 229 (original), 230 (patched) <https://reviews.apache.org/r/62230/#comment262785> There's a `#include <process/async.hpp>` added for `async` that we can remove now. src/slave/gc_process.hpp Lines 110 (patched) <https://reviews.apache.org/r/62230/#comment262786> Can we add a bit more expanation since the name `executor` is pretty generic? e.g., ``` // For executing path removals in a separate thread. ``` - Jiang Yan Xu On Sept. 12, 2017, 1:52 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62230/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2017, 1:52 p.m.) > > > Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-7964 > https://issues.apache.org/jira/browse/MESOS-7964 > > > Repository: mesos > > > Description > ------- > > This patch dispatches `rmdirs` to a single executor instead of multiple > `AsyncExecutor`s such that heavy-duty pruning events won't occupy all > worker threads and block other actors. > > > Diffs > ----- > > src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 > src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 > > > Diff: https://reviews.apache.org/r/62230/diff/3/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
