Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review154279 --- Superseded by https://reviews.apache.org/r/53227/. - Gastón Kleiman On Oct. 27, 2016, 5:34 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 27, 2016, 5:34 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 > src/launcher/default_executor.hpp PRE-CREATION > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review154229 --- Ship it! Ship It! - haosdent huang On Oct. 27, 2016, 5:34 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 27, 2016, 5:34 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 > src/launcher/default_executor.hpp PRE-CREATION > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review154083 --- Patch looks great! Reviews applied: [52878, 53084, 53196, 53197, 52879, 52880] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On Oct. 27, 2016, 5:34 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 27, 2016, 5:34 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 > src/launcher/default_executor.hpp PRE-CREATION > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/ --- (Updated Oct. 27, 2016, 5:34 p.m.) Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, Jie Yu, and Jiang Yan Xu. Changes --- Rebase. Bugs: MESOS-6288 https://issues.apache.org/jira/browse/MESOS-6288 Repository: mesos Description --- Added "launcher_dir" to the default executor flags. Diffs (updated) - src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 src/launcher/default_executor.hpp PRE-CREATION src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 Diff: https://reviews.apache.org/r/52880/diff/ Testing --- `make check` Thanks, Gastón Kleiman
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review153726 --- Patch looks great! Reviews applied: [52878, 53084, 52879, 52880] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On Oct. 24, 2016, 9:08 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 24, 2016, 9:08 a.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 > src/launcher/default_executor.hpp PRE-CREATION > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/ --- (Updated Oct. 24, 2016, 9:08 a.m.) Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, Jie Yu, and Jiang Yan Xu. Changes --- Rebased. Bugs: MESOS-6288 https://issues.apache.org/jira/browse/MESOS-6288 Repository: mesos Description --- Added "launcher_dir" to the default executor flags. Diffs (updated) - src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 src/launcher/default_executor.hpp PRE-CREATION src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 Diff: https://reviews.apache.org/r/52880/diff/ Testing --- `make check` Thanks, Gastón Kleiman
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/ --- (Updated Oct. 21, 2016, 11:39 a.m.) Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, Jie Yu, and Jiang Yan Xu. Changes --- Rebased. Bugs: MESOS-6288 https://issues.apache.org/jira/browse/MESOS-6288 Repository: mesos Description --- Added "launcher_dir" to the default executor flags. Diffs (updated) - src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d src/launcher/default_executor.hpp PRE-CREATION src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 Diff: https://reviews.apache.org/r/52880/diff/ Testing --- `make check` Thanks, Gastón Kleiman
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
> On Oct. 14, 2016, 8:25 p.m., Jiang Yan Xu wrote: > > This is conversation carried over from /r/52556/ > > > > Now that we're in the business of (re)naming things for consistency, > > understandability and unambiguity, how about we just follow some widely > > used conventions: > > https://www.gnu.org/prep/standards/html_node/Directory-Variables.html > > > > The motivation in the renaming in /r/52556/ seems to be because the number > > of binaries in the directory has grown and not all related to the launcher > > anymore. `--libexec_dir` or `MESOS_LIBEXEC_DIR` feels pretty clear for what > > it is. > > > > BTW, when I first see the flag `--runtime_dir` on the agent, I thought it > > was for the "runtime binaries" as well. Would `--runstate_dir` or > > `MESOS_RUNSTATE_DIR` (from the linked doc) be better names? > > > > ^^ @Jie > > Gastón Kleiman wrote: > The goal of this patch is to resolve MESOS-6288 (The default executor > should maintain launcher_dir) and therefore to unblock MESOS-6119 (TCP health > checks are not portable). > > I agree that `launcher_dir` is not a good name for the flag and volunteer > for doing patch with a sweeping rename ss part of MESOS-6341 (Improve > environment variable setting for executors, tasks and nested containers). > > haosdent huang wrote: > +1 for `MESOS_LIBEXEC_DIR` and agree with we should rename it in separate > patches. Because we use `launcher_dir` in different files, should not mess up > the rename part with this. > > ``` > $ grep -r 'launcher_dir' src |wc -l > 61 > ``` Yeah, launcher_dir is unfortunate. Given that agent already has this flag, it's not easy to retire. We can probably introduce flag alias. Let's create a ticket and track this deprecation. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review152730 --- On Oct. 14, 2016, 5:18 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 14, 2016, 5:18 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
> On Oct. 14, 2016, 8:25 p.m., Jiang Yan Xu wrote: > > This is conversation carried over from /r/52556/ > > > > Now that we're in the business of (re)naming things for consistency, > > understandability and unambiguity, how about we just follow some widely > > used conventions: > > https://www.gnu.org/prep/standards/html_node/Directory-Variables.html > > > > The motivation in the renaming in /r/52556/ seems to be because the number > > of binaries in the directory has grown and not all related to the launcher > > anymore. `--libexec_dir` or `MESOS_LIBEXEC_DIR` feels pretty clear for what > > it is. > > > > BTW, when I first see the flag `--runtime_dir` on the agent, I thought it > > was for the "runtime binaries" as well. Would `--runstate_dir` or > > `MESOS_RUNSTATE_DIR` (from the linked doc) be better names? > > > > ^^ @Jie The goal of this patch is to resolve MESOS-6288 (The default executor should maintain launcher_dir) and therefore to unblock MESOS-6119 (TCP health checks are not portable). I agree that `launcher_dir` is not a good name for the flag and volunteer for doing patch with a sweeping rename ss part of MESOS-6341 (Improve environment variable setting for executors, tasks and nested containers). - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review152730 --- On Oct. 14, 2016, 5:18 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 14, 2016, 5:18 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review152775 --- Patch looks great! Reviews applied: [52878, 52879, 52880] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On Oct. 14, 2016, 5:18 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 14, 2016, 5:18 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/#review152730 --- This is conversation carried over from /r/52556/ Now that we're in the business of (re)naming things for consistency, understandability and unambiguity, how about we just follow some widely used conventions: https://www.gnu.org/prep/standards/html_node/Directory-Variables.html The motivation in the renaming in /r/52556/ seems to be because the number of binaries in the directory has grown and not all related to the launcher anymore. `--libexec_dir` or `MESOS_LIBEXEC_DIR` feels pretty clear for what it is. BTW, when I first see the flag `--runtime_dir` on the agent, I thought it was for the "runtime binaries" as well. Would `--runstate_dir` or `MESOS_RUNSTATE_DIR` (from the linked doc) be better names? ^^ @Jie - Jiang Yan Xu On Oct. 14, 2016, 10:18 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52880/ > --- > > (Updated Oct. 14, 2016, 10:18 a.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-6288 > https://issues.apache.org/jira/browse/MESOS-6288 > > > Repository: mesos > > > Description > --- > > Added "launcher_dir" to the default executor flags. > > > Diffs > - > > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 > > Diff: https://reviews.apache.org/r/52880/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Gastón Kleiman > >
Review Request 52880: Added "launcher_dir" to the default executor flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52880/ --- 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 --- Added "launcher_dir" to the default executor flags. Diffs - src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 Diff: https://reviews.apache.org/r/52880/diff/ Testing --- `make check` Thanks, Gastón Kleiman
