> 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?
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. - Benjamin ----------------------------------------------------------- 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 > >
