> On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote: > > src/slave/constants.hpp, line 53 > > <https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53> > > > > What is the 'base executor' versus the 'command executor'? > > Alexander Rukletsov wrote: > We have Executor (lives in src/exec/exec.cpp) and CommandExecutor aka > mesos-executor (lives in src/launcher/executor.cpp). I find "executor" too > vague and use "base executor" to stress out I mean the one that lives in > exec.cpp. Is there a convention about naming these folks? > > Benjamin Hindman wrote: > Ah, I see. Well, CommandExecutor is just an instance of an executor and > actually uses the code from exec.cpp just like all current executors do (that > use libmesos). So there aren't actually two executors (base and command), > just one, and they all use exec.cpp (if they use libmesos). Does that make > sense? > > Alexander Rukletsov wrote: > Sorry, I was inexact in my comment. Indeed, there is only one executor, > but two libprocess processes (where all most of the work is done). Here is > what we have: > Executor <-> ExecutorProcess (I call them both "base executor", though > "base executor process" is more correct) > CommandExecutor <-> CommandExecutorProcess > The OS process where CommandExecutor lives instantiates also the driver, > together it looks like this: > _______________________________________________ > MesosExecutorDriver > * ExecutorProcess > | > V > * CommandExecutor <-> CommandExecutorProcess > | > V > task > _______________________________________________ > > My aim was to explain that there is a "wrapper" around the > CommandExecutorProcess which has its own shutdown period. For simplicity I > called this "wrapper" (which is ExecutorProcess and a bit > MesosExecutorDriver) "base executor". However, it looks like my terminology > is not good enough and maybe even misleading. What terms would you suggest, > Ben? > > Benjamin Hindman wrote: > IMHO we should just use the terms ExecutorProcess, MesosExecutorDriver, > etc. If you want to alias them within that comment then I'd suggest defining > the alias (as you've done for me here) and then using that alias in the > comment. That being said, my hunch is that you'll get more milage just using > the class names. This will also make renaming/refactoring easier as code > searches will grab these comments too.
Ok, agreed. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/#review52750 ----------------------------------------------------------- On Sept. 9, 2014, 12:54 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25434/ > ----------------------------------------------------------- > > (Updated Sept. 9, 2014, 12:54 p.m.) > > > Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy St. > Clair. > > > Bugs: MESOS-1571 > https://issues.apache.org/jira/browse/MESOS-1571 > > > Repository: mesos-git > > > Description > ------- > > The configurable slave's executor_shutdown_grace_period flag is propagated to > Executor and CommandExecutor through an environment variable. Shutdown > timeout in Executor and signal escalation timeout in CommandExecutor are now > dependent on this flag. Each nested timeout is somewhat shorter than the > parent one. > > > Diffs > ----- > > src/exec/exec.cpp 36d1778 > src/launcher/executor.cpp 12ac14b > src/slave/constants.hpp 9030871 > src/slave/constants.cpp e1da5c0 > src/slave/containerizer/containerizer.hpp 8a66412 > src/slave/containerizer/containerizer.cpp 0254679 > src/slave/containerizer/docker.cpp 0febbac > src/slave/containerizer/external_containerizer.cpp efbc68f > src/slave/containerizer/mesos/containerizer.cpp 9d08329 > src/slave/flags.hpp 21e0021 > src/tests/containerizer.cpp a17e1e0 > > Diff: https://reviews.apache.org/r/25434/diff/ > > > Testing > ------- > > make check (OS X 10.9.4; Ubuntu 14.04 amd64) > > > Thanks, > > Alexander Rukletsov > >
