----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59605/#review176771 -----------------------------------------------------------
src/docker/executor.cpp Lines 747-750 (original), 747-750 (patched) <https://reviews.apache.org/r/59605/#comment250239> We should probably move this higher up before initializing logging, as we do in other executors. src/docker/executor.cpp Lines 752-770 (original), 752-770 (patched) <https://reviews.apache.org/r/59605/#comment250240> This looks like validation of flags, which is probably should be done before we initialize logging for consistency. src/docker/executor.cpp Lines 772-800 (original), 772-800 (patched) <https://reviews.apache.org/r/59605/#comment250241> I'd say this can use `EXIT(EXIT_FAILURE) << ` src/docker/executor.cpp Lines 814-816 (original), 814-816 (patched) <https://reviews.apache.org/r/59605/#comment250242> Looks like we use `EXIT(EXIT_FAILURE)` in similar cases in other executors. src/launcher/default_executor.cpp Line 1365 (original), 1367-1369 (patched) <https://reviews.apache.org/r/59605/#comment250238> Not yours, but could you also print warnings as we do it for example for command executor? In a separate patch, please. ``` // Log any flag warnings (after logging is initialized). foreach (const flags::Warning& warning, load->warnings) { LOG(WARNING) << warning.message; } ``` - Alexander Rukletsov On June 2, 2017, 1:35 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59605/ > ----------------------------------------------------------- > > (Updated June 2, 2017, 1:35 p.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Benjamin > Mahler, and Till Toenshoff. > > > Bugs: MESOS-6961 > https://issues.apache.org/jira/browse/MESOS-6961 > > > Repository: mesos > > > Description > ------- > > The rule for choosing between glog and cout is described by MESOS-7586. > > > Diffs > ----- > > src/docker/executor.cpp 464a74859e0f477ec2e5525a5fc726ea420a00a2 > src/launcher/default_executor.cpp 1a60e3bb2b0e8312b349cddedb4cd55716c1b574 > src/launcher/executor.cpp 9ac3c3d84c0ec47954c5c72228ad0b8795ff3eec > > > Diff: https://reviews.apache.org/r/59605/diff/2/ > > > Testing > ------- > > 1. make check (mac os x 10.12, fedora 25) > 2. jenkins: > https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI-build/1087/ > > 3. I've temporarily modified NoTaskKillingCapability from > command_executor_tests.cpp to launch command "sleep 0.1" (as well as "sleep > 0"), > then to wait for a task completion. The test was launched in an endless loop: > > GLOG_v=1 ./bin/mesos-tests.sh --gtest_repeat=-1 --gtest_break_on_failure > --gtest_filter="HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0" > --verbose > > I've run this test multiple times for a while both on mac os and Fedora > linux. Race condition problem described in a jira ticket wasn't reproduced. > > > Thanks, > > Andrei Budnik > >
