> On Oct. 10, 2016, 3:32 p.m., Jiang Yan Xu wrote: > > src/launcher/executor.cpp, lines 903-910 > > <https://reviews.apache.org/r/52556/diff/2/?file=1525587#file1525587line903> > > > > You didn't start this but I wonder why we can't consistently use the > > flags, which can be provided through environment variables, i.e., use > > `flags.load("MESOS_", &argc, &argv);` above. > > > > That question aside, I don't think we need to be concerned about > > backwards-compatibility here: this is the command executor bundled with the > > agent so they are upgraded together. i.e., we are sure that the agent is > > setting the new variable. > > Gastón Kleiman wrote: > Wouldn't then the tasks started between the moment in which the binary is > overwritten and the moment in which the Mesos Agent is restarted fail? > > Jiang Yan Xu wrote: > OK I guess it'll be helpful in this regard. My main point is that this is > not a deprecation cycle concern. > > Generally it's advisable to restart the agent right after upgrading it > but even though it's done this way with most Mesos toolings I have seen, I've > also seen tasks failing when launched mid-upgrade with libmesos and > executable mismatch. > > Therefore, fine with me to do this to aid N -> N+1 upgrade. > > If we do this, the cleaniest approach may be flag aliases. > > Alexander Rukletsov wrote: > Technically, people may overwrite the mesos-executor binary. > > Jiang Yan Xu wrote: > As in, a custom executor but called via the agent <-> CommandExecutor > "protocol"? Is this an argument that the removal of `--launch_dir` flag > should go through a deprecation cycle? Could you elaborate? > > Alexander Rukletsov wrote: > Yes, this is what I meant. > > Another reason is that we probably should maintain upgradability among > all 1.x version, i.e. N -> N + m upgrades, at least this is how I read the > versioning doc. With this in mind, we can't simply remove the flag. > > Gastón Kleiman wrote: > I'm dropping this issue, since it seems like we agree that keeping > backwards-compatibility is useful for N -> N+1 upgrades. > > If someone wants to shepherd me, I volunteer to make `executor.cpp` fully > move to use env variables instead of a mix of flags and env variables.
> Wouldn't then the tasks started between the moment in which the binary is > overwritten and the moment in which the Mesos Agent is restarted fail? I don't understand why env variable can address this issue? If the evn is optional, then, it won't fail (same as the flag is optional). If the env in not optional, will the task still fail? - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52556/#review151918 ----------------------------------------------------------- On Oct. 11, 2016, 6:52 p.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52556/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2016, 6:52 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and > Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > ------- > > This environment variable points to the directory containing the > binaries used by the Mesos Agent. It will eventually replace the > `--launcher-dir` executor flag. > > > Diffs > ----- > > docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 > src/launcher/default_executor.cpp 2454bd7df608254af72af01460782f5ab78a19c1 > src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 > src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 > > Diff: https://reviews.apache.org/r/52556/diff/ > > > Testing > ------- > > `make check` > > > Thanks, > > Gastón Kleiman > >
