> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > src/slave/slave.cpp > > Lines 9221-9246 (patched) > > <https://reviews.apache.org/r/64978/diff/2/?file=1934306#file1934306line9226> > > > > Kill this. See above comments. > > Qian Zhang wrote: > The reason I did this change is, only doing detach in `removeExecutor` > and `recoverExecutor` can not handle a corner case: Say a default executor > launches a large number of tasks (say the number of tasks is greater than > `MAX_COMPLETED_TASKS_PER_EXECUTOR`), and then all the tasks finishes, so > there will be some old tasks automatically removed from > `Executor::completedTasks` (since it is `boost::circular_buffer` with size > `MAX_COMPLETED_TASKS_PER_EXECUTOR`). When `removeExecutor` is called > eventually, we will be missed to do the detach for those old tasks since we > can not find them anywhere, that is kind of leak.
Fixed the issue mentioned in my previous comment by doing detach for the first task before pushing a task into a full `Executor::completedTasks`. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195000 ----------------------------------------------------------- On Jan. 15, 2018, 2:21 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64978/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2018, 2:21 p.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Vinod > Kone. > > > Bugs: MESOS-8279 > https://issues.apache.org/jira/browse/MESOS-8279 > > > Repository: mesos > > > Description > ------- > > In MESOS-7225, we made a task can access any volumes specified in its > disk resources from its own sandbox by introducing a workaround to the > default executor, i.e., add a `SANDBOX_PATH` volume with type `PARENT` > to the corresponding nested container. It will be translated into a bind > mount in the nested container's mount namespace, thus not visible in the > host mount namespace, that means the task's volume directory can not be > visible in Mesos UI since it operates in the host mount namespace. > > In this patch, to make the task's volume directory visible in Mesos UI, > we attached the executor's volume directory to it, so when users browse > task's volume directory in Mesos UI, what they actually browse is the > executor's volume directory. Note when calling `Files::attach()`, the > third argument `authorized` is not specified, that is because it is > already specified when we do the attach for the executor's sandbox and > it is also applied to the executor's tasks. > > > Diffs > ----- > > src/slave/slave.hpp ef0eae21af811cc09f43cd1d4c4ccc0c33cbeb39 > src/slave/slave.cpp aeb0fdaeda78f26de2ec790735bfa88c5be72850 > > > Diff: https://reviews.apache.org/r/64978/diff/4/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Qian Zhang > >
