Review Request 69513: Manually copy test reports to host fs.

2018-12-05 Thread Vinod Kone
--- Manually copy test reports to host fs. Diffs - support/mesos-build/entrypoint.sh ec98cc8b1fdcd0a32ed32cfeb69dfb976f82b81d Diff: https://reviews.apache.org/r/69513/diff/1/ Testing --- Have been testing this over week in a custom Jenkins job. Thanks, Vinod Kone

Re: Review Request 61128: Improved log messages in master when adding/removing tasks/executors.

2018-11-15 Thread Vinod Kone
/ Changes: https://reviews.apache.org/r/61128/diff/2-3/ Testing --- make check Thanks, Vinod Kone

Re: Review Request 69293: Disabled parallel test execution for reviewbot.

2018-11-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69293/#review210401 --- Ship it! Ship It! - Vinod Kone On Nov. 7, 2018, 10:57 p.m

Review Request 69291: Simplified writing out test report xml files.

2018-11-07 Thread Vinod Kone
--- Simplified writing out test report xml files. Diffs - support/mesos-build/entrypoint.sh 012f003b3cf22b49a442df293f1b6224c074e383 Diff: https://reviews.apache.org/r/69291/diff/1/ Testing --- Thanks, Vinod Kone

Re: Review Request 69291: Simplified writing out test report xml files.

2018-11-07 Thread Vinod Kone
://reviews.apache.org/r/69291/diff/1-2/ Testing --- Thanks, Vinod Kone

Review Request 69279: Updated Build bot to write out test report xml files.

2018-11-07 Thread Vinod Kone
manually with a custom jenkins job. Thanks, Vinod Kone

Re: Review Request 69193: Added filters info to the `DECLINE` call log message.

2018-10-26 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69193/#review210099 --- Ship it! Ship It! - Vinod Kone On Oct. 26, 2018, 5:42 p.m

Re: Review Request 69123: Fixed an early fd close in the cgroups event notifier.

2018-10-23 Thread Vinod Kone
) <https://reviews.apache.org/r/69123/#comment294558> Didn't quite follow why you had make this an Option? - Vinod Kone On Oct. 23, 2018, 3:08 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 68951: Updated verify-reviews.py to use current interpreter in subprocesses.

2018-10-08 Thread Vinod Kone
- On Oct. 8, 2018, 6:06 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68951/ > -----

Re: Review Request 68951: Updated verify-reviews.py to use current interpreter in subprocesses.

2018-10-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68951/#review209324 --- Ship it! Ship It! - Vinod Kone On Oct. 8, 2018, 6:06 p.m

Re: Review Request 68912: Added a log line to `MesosContainerizer::kill()`.

2018-10-03 Thread Vinod Kone
f3b-439c-a27a-847a8acd066c-S0 > > I1003 16:05:33.824251 17340 process.cpp:926] Stopped the socket accept loop > > ``` @andy Is this a known issue with windows reviewbot? - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68912/#re

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

2018-10-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68866/#review209186 --- Ship it! Ship It! - Vinod Kone On Sept. 27, 2018, 5:41 p.m

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

2018-10-03 Thread Vinod Kone
> On Sept. 27, 2018, 6:23 p.m., Vinod Kone wrote: > > src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp > > Lines 294 (patched) > > <https://reviews.apache.org/r/68866/diff/1/?file=2092453#file2092453line294> > > > > Looks like you are doing `c

Review Request 68912: Added a log line to `MesosContainerizer::kill()`.

2018-10-03 Thread Vinod Kone
--- Added a log line to `MesosContainerizer::kill()`. Diffs - src/slave/containerizer/mesos/containerizer.cpp 82d64e9928abd5e22493fa697d7b0910211533f3 Diff: https://reviews.apache.org/r/68912/diff/1/ Testing --- make -j3 check Ran it through internal CI. Thanks, Vinod Kone

Re: Review Request 68899: Updated the MesosCon 2018 location.

2018-10-02 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68899/#review209156 --- Ship it! Ship It! - Vinod Kone On Oct. 2, 2018, 6:59 p.m

Re: Review Request 68865: Put `TerminateEvent` at the end of the queue in the Mesos library.

2018-09-27 Thread Vinod Kone
) <https://reviews.apache.org/r/68865/#comment293336> Can you add a comment on why you are doing this for posterity? - Vinod Kone On Sept. 27, 2018, 5:41 p.m., Alexander Rukletsov wrote: > > --- > This is an automatica

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

2018-09-27 Thread Vinod Kone
sleep in the client code instead of here for now. - Vinod Kone On Sept. 27, 2018, 5:41 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 68826: Fixed bug in `verify-reviews` due to mismatched types.

2018-09-24 Thread Vinod Kone
.org/r/68826/ > --- > > (Updated Sept. 24, 2018, 6:27 p.m.) > > > Review request for mesos, Benjamin Bannier and Vinod Kone. > > > Bugs: MESOS-9253 > https://issues.apache.org/jira/browse/MESOS-9253 > > > Repository: mesos > > > Description > ---

Re: Review Request 68257: Fixed incorrect `mnt` namespace detection of command executor's task.

2018-08-13 Thread Vinod Kone
> On Aug. 7, 2018, 4:03 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/utils.cpp > > Line 102 (original), 105 (patched) > > <https://reviews.apache.org/r/68257/diff/1/?file=2069850#file2069850line108> > > > > Are we guaranteed that there a

Re: Review Request 68257: Fixed incorrect `mnt` namespace detection of command executor's task.

2018-08-07 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/68257/#comment290125> Are we guaranteed that there are no short-lived processes, other than the task process, at the 2nd level? If not, we will have the same issue right? Modulo the above question, the change LGTM. - Vinod Kone On

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

2018-07-30 Thread Vinod Kone
-2018-cfp-is-now-open.md Lines 23 (patched) <https://reviews.apache.org/r/68111/#comment289640> s/2018/2017/ - Vinod Kone On July 30, 2018, 8:12 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e

Re: Review Request 67965: Optimized ranges subtraction operation.

2018-07-26 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67965/#review206530 --- Ship it! Ship It! - Vinod Kone On July 26, 2018, 7:14 p.m

Re: Review Request 67965: Optimized ranges subtraction operation.

2018-07-24 Thread Vinod Kone
s.apache.org/r/67965/#comment289371> See below. - Vinod Kone On July 24, 2018, 5:29 a.m., Meng Zhu wrote: > > --- > This is an automatically generated e-ma

Re: Review Request 67891: Added multi-framework scalability guidelines to the documentation.

2018-07-20 Thread Vinod Kone
) <https://reviews.apache.org/r/67891/#comment289182> Do we want to give any guidance on what values we considere large? Otherwise, it might add more confusion than necessary? - Vinod Kone On July 13, 2018, 9:33 p.m., Benjamin Mahler

Re: Review Request 67857: Improved release policy documentation visibility.

2018-07-09 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67857/#review205871 --- Ship it! Ship It! - Vinod Kone On July 9, 2018, 8:29 p.m

Re: Review Request 67791: Prevented master from asking agents to shutdown on auth failures.

2018-07-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67791/#review205758 --- Ship it! Ship It! - Vinod Kone On July 3, 2018, 11:57 p.m

Re: Review Request 67817: Improved logging for offers and inverse offers.

2018-07-03 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/67817/#comment288583> Can we log `*slave` here instead of just slave id? im assuming that pointer is still valid here. - Vinod Kone On July 3, 2018, 7:16 p.m., Gastón Kleiman

Re: Review Request 67403: Handled race condition when removing maintenance windows.

2018-05-31 Thread Vinod Kone
> On May 31, 2018, 4:41 p.m., Vinod Kone wrote: > > Can you add a unit test for this? > > Benno Evers wrote: > It's tricky because we need very precise control over the scheduling, and > I'm not sure our testing infrastructure provides it. But I'll look into

Re: Review Request 67403: Handled race condition when removing maintenance windows.

2018-05-31 Thread Vinod Kone
> On May 31, 2018, 4:41 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Line 9459 (original), 9459 (patched) > > <https://reviews.apache.org/r/67403/diff/1/?file=2033322#file2033322line9459> > > > > not yours, but can you log framework

Re: Review Request 67403: Handled race condition when removing maintenance windows.

2018-05-31 Thread Vinod Kone
variant here right? src/master/master.cpp Lines 9470 (patched) <https://reviews.apache.org/r/67403/#comment286511> log framework id? - Vinod Kone On May 31, 2018, 4:32 p.m., Benno Evers wrote: > > --- > This is an aut

Re: Review Request 66982: Added a `decline` flag in `recoverResources` in the allocator.

2018-05-07 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66982/#review202598 --- Do we need this optimization? - Vinod Kone On May 7, 2018, 6

Re: Review Request 66947: Increased the timeout for waiting for `reaped` to be invoked.

2018-05-07 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66947/#review202560 --- Ship it! LGTM. - Vinod Kone On May 4, 2018, 9:32 a.m., Qian

Re: Review Request 66963: Updated the 1.6 CHANGELOG.

2018-05-04 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66963/#review202456 --- Ship it! Ship It! - Vinod Kone On May 4, 2018, 5:18 p.m

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

2018-04-06 Thread Vinod Kone
do you mean by `SUBSCRIBED` calls? - Vinod Kone On April 6, 2018, 9 p.m., Gaston Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 65791: Removed stale generated HTTP endpoint documentation.

2018-02-23 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65791/#review198234 --- Ship it! Ship It! - Vinod Kone On Feb. 24, 2018, 12:29 a.m

Re: Review Request 65570: Attached/detached volume directory for task which has volume specified.

2018-02-12 Thread Vinod Kone
t277498> kill this. src/slave/slave.cpp Lines 1231 (patched) <https://reviews.apache.org/r/65570/#comment277499> taskDirectoryPath - Vinod Kone On Feb. 10, 2018, 2:59 p.m., Qian Zhang wrote: > > --- > Thi

Re: Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

2018-02-12 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/65551/#comment277455> If a launch for nested container fails, don't we get `NotFound`? - Vinod Kone On Feb. 10, 2018, 6:35 a.m., Gaston Kleiman wrote: > > --- > This is an automatica

Re: Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

2018-02-12 Thread Vinod Kone
> --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65551/ > --- > > (Updated Feb. 10, 2018, 6:35 a.m.) > > > Rev

Re: Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

2018-02-12 Thread Vinod Kone
u log here because this gets called from couple different sites? - Vinod Kone On Feb. 10, 2018, 6:35 a.m., Gaston Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http

Re: Review Request 65593: Added a test to ensure master removes fail-launched deafult executor.

2018-02-12 Thread Vinod Kone
/default executor that failed to launch/ src/tests/slave_tests.cpp Lines 4733 (patched) <https://reviews.apache.org/r/65593/#comment277431> s/unscheule/unschedule/ s/.So that/and that/ - Vinod Kone On Feb. 10, 2018, 12:36 a.m., Meng Zhu

Re: Review Request 65549: Improved some default executor log messages.

2018-02-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65549/#review197302 --- Ship it! Ship It! - Vinod Kone On Feb. 10, 2018, 6:34 a.m

Re: Review Request 65449: Fixed an bug where executor info lingers on master if failed to launch.

2018-02-12 Thread Vinod Kone
views.apache.org/r/65449/#comment277427> Ideally we should have tests for each of these scenarios here. Do we? src/slave/slave.cpp Lines 3402 (patched) <https://reviews.apache.org/r/65449/#comment277428> "Consider doing a `CHECK` here since this shouldn't be possible."

Re: Review Request 65504: Made master set `launch_executor` in the RunTask(Group)Message.

2018-02-12 Thread Vinod Kone
slave); if (launchExecutor) { addExecutor(task.executor(), framework, slave); consumed += task.executor().resources() } } ``` otherwise, the log line below says we are launching a command task on an existing executor which is wrong. - Vinod Kone On Feb. 5, 2018, 2:

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

2018-02-12 Thread Vinod Kone
> On Feb. 8, 2018, 8:22 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 2186 (patched) > > <https://reviews.apache.org/r/65571/diff/1/?file=1954571#file1954571line2186> > > > > s/Leader detector indicated no master elected/No master was ele

Re: Review Request 65518: Reaped the container process directly in Docker executor.

2018-02-09 Thread Vinod Kone
dy is set in case the `run` returns after `reapedContainer` is called? - Vinod Kone On Feb. 9, 2018, 1:03 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

2018-02-08 Thread Vinod Kone
ched) <https://reviews.apache.org/r/65448/#comment277267> Can you also add a test with default executor? And maybe instead of kill task path, try to exercise the authz failure path for that test? Feel free to do it in a subsequent review. More tests that exercise the code paths you changed the bett

Re: Review Request 65572: Fixed two slave recovery tests.

2018-02-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65572/#review197140 --- Ship it! Ship It! - Vinod Kone On Feb. 9, 2018, 1:12 a.m

Re: Review Request 65449: Fixed an issue where executor info linger on master if failed to launch.

2018-02-08 Thread Vinod Kone
tched) <https://reviews.apache.org/r/65449/#comment277232> s/And the master will no longer contain/Master then removes/ - Vinod Kone On Feb. 5, 2018, 3 a.m., Meng Zhu wrote: > > --- > This is an automatically generated e-

Re: Review Request 65446: Added helper function for the agent to send `ExitedExecutorMessage`.

2018-02-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65446/#review197105 --- Ship it! Ship It! - Vinod Kone On Feb. 5, 2018, 2:45 a.m

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

2018-02-08 Thread Vinod Kone
e want? Also, where in the interface does it say that None() is retryable. It says retryable errors are handled internally by the detector? - Vinod Kone On Feb. 8, 2018, 4:50 p.m., Benno Evers wrote: > > --- > This is a

Re: Review Request 65504: Made master set `launch_executor` in the RunTask(Group)Message.

2018-02-08 Thread Vinod Kone
stead of `totalResources` and use it here? src/master/master.cpp Lines 5203 (patched) <https://reviews.apache.org/r/65504/#comment277219> s/message.launch_executor()/executor/ - Vinod Kone On Feb. 5, 2018, 2:49 a.m., Meng Zhu wrote: > >

Re: Review Request 65573: Fixed an error in the 1.3.2 CHANGELOG.

2018-02-08 Thread Vinod Kone
- > > (Updated Feb. 8, 2018, 7:12 p.m.) > > > Review request for mesos, Jie Yu, Michael Park, and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Fixed an error in the 1.3.2 CHANGELOG. > > > Diffs > - > >

Re: Review Request 65445: Added new protobuf field `launch_executor` in RunTask(Group)Message.

2018-02-06 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65445/#review196924 --- Ship it! Ship It! - Vinod Kone On Feb. 5, 2018, 2:44 a.m

Re: Review Request 65449: Fixed an issue where executor info linger on master if failed to launch.

2018-02-02 Thread Vinod Kone
6 (patched) <https://reviews.apache.org/r/65449/#comment276432> you can put fraemworkInfo in the previous line? src/slave/slave.cpp Lines 2065 (patched) <https://reviews.apache.org/r/65449/#comment276433> do you need the temporary variable? - Vinod Kone On Fe

Re: Review Request 65437: Added documentation for fault domains.

2018-02-02 Thread Vinod Kone
> On Feb. 2, 2018, 10:11 p.m., Vinod Kone wrote: > > Fixed the issues before committing. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65437/#rev

Re: Review Request 65437: Added documentation for fault domains.

2018-02-02 Thread Vinod Kone
), 79 (patched) <https://reviews.apache.org/r/65437/#comment276578> s/London/San Francisco/ ? docs/fault-domains.md Line 84 (original), 89 (patched) <https://reviews.apache.org/r/65437/#comment276580> s/companies/company's/ - Vinod Kone On Feb. 2, 2018, 7:30 p.m., Benno

Re: Review Request 65437: Added documentation for fault domains.

2018-02-02 Thread Vinod Kone
> On Feb. 1, 2018, 10:18 p.m., Vinod Kone wrote: > > docs/fault-domains.md > > Lines 63 (patched) > > <https://reviews.apache.org/r/65437/diff/2/?file=1950950#file1950950line63> > > > > s/The default/By default, the/ > > Benno Evers wrote: >

Re: Review Request 65447: Refactored couple of launch task sanity checks into a single code path.

2018-02-01 Thread Vinod Kone
n a different line. src/slave/slave.cpp Lines 2123 (patched) <https://reviews.apache.org/r/65447/#comment276409> CHECK_NOTNULL(getFramework(frameworkId)) - Vinod Kone On Feb. 1, 2018, 2:03 a.m., Meng Zhu wrote: > >

Re: Review Request 65446: Added helper function for the agent to send `ExitedExecutorMessage`.

2018-02-01 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65446/#review196663 --- Ship it! Ship It! - Vinod Kone On Feb. 1, 2018, 2:01 a.m

Re: Review Request 65445: Added new protobuf field `launch_executor` in RunTask(Group)Message.

2018-02-01 Thread Vinod Kone
ttps://reviews.apache.org/r/65445/#comment276402> ditto. src/tests/mock_slave.hpp Lines 111 (patched) <https://reviews.apache.org/r/65445/#comment276405> s/toLaunchExecutor/launchExecutor/ here and below. - Vinod Kone On

Re: Review Request 65437: Added documentation for fault domains.

2018-02-01 Thread Vinod Kone
ns.md Lines 90-99 (patched) <https://reviews.apache.org/r/65437/#comment276394> This example is a bit too complicated. I would say lets use an example of one on-prem data center which is extended by a cloud provider hosted remote region. - Vinod Kone On Jan. 31, 2018, 5:5

Re: Review Request 65382: Reaped Docker executor only when it can be connected.

2018-01-30 Thread Vinod Kone
non-existent? - Vinod Kone On Jan. 30, 2018, 3:42 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 65420: Fixed a coding error in a log message of Docker containerizer.

2018-01-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65420/#review196537 --- Ship it! Ship It! - Vinod Kone On Jan. 30, 2018, 3:42 p.m

Re: Review Request 65382: Reaped Docker executor only when it can be connected.

2018-01-30 Thread Vinod Kone
<< "' of framework " << framework.id Can we log the SocketError here as well? - Vinod Kone On Jan. 30, 2018, 3:42 p.m., Qian Zhang wrote: > >

Re: Review Request 65344: Updated the docs for agent ping timeout flags.

2018-01-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65344/#review196534 --- Ship it! Ship It! - Vinod Kone On Jan. 25, 2018, 9:18 p.m

Re: Review Request 65423: Fixed duplicate protobuf linking in Python examples.

2018-01-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65423/#review196533 --- Ship it! Ship It! - Vinod Kone On Jan. 30, 2018, 9:35 p.m

Re: Review Request 65044: Added the v1 API 'GET_OPERATIONS' call for master and agent.

2018-01-26 Thread Vinod Kone
. src/slave/http.cpp Lines 1685 (patched) <https://reviews.apache.org/r/65044/#comment275992> Not yours, but can we make the logging consistent here and in master for operator api handlers? Looks like master doesn't even log here? - Vinod Kone On Jan. 25, 2018, 10:36 a.m., Jan Sc

Re: Review Request 65245: Renamed `LOG` by `Stream logs` in Web UI.

2018-01-24 Thread Vinod Kone
> On Jan. 22, 2018, 7:33 p.m., Vinod Kone wrote: > > src/webui/master/static/agent.html > > Line 45 (original), 45 (patched) > > <https://reviews.apache.org/r/65245/diff/1/?file=1943087#file1943087line45> > > > > Hmm. This is inconsistent

Re: Review Request 65106: Removed the misleading `isRemovable` helper in the master.

2018-01-22 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/65106/#comment275413> s/Indicated/Indicates/ ? src/master/master.cpp Lines 10301 (patched) <https://reviews.apache.org/r/65106/#comment275414> s/simply/simplify/ - Vinod Kone On Jan. 23, 2018, 12:11 a.m., Benjamin

Re: Review Request 65253: Fixed dropped events on the master operator API stream.

2018-01-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65253/#review195953 --- Ship it! Ship It! - Vinod Kone On Jan. 22, 2018, 11:36 p.m

Re: Review Request 65263: Updated `SlaveRecoveryTest.RecoverCompletedExecutor` to verify gc.

2018-01-22 Thread Vinod Kone
> On Jan. 22, 2018, 8:24 p.m., Vinod Kone wrote: > > Ship It! To confirm, would this new test have failed without the fix for https://issues.apache.org/jira/browse/MESOS-8460 ? - Vinod --- This is an automatically generat

Re: Review Request 65263: Updated `SlaveRecoveryTest.RecoverCompletedExecutor` to verify gc.

2018-01-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65263/#review195932 --- Ship it! Ship It! - Vinod Kone On Jan. 22, 2018, 1:21 p.m

Re: Review Request 65108: Added documentation for `protobuf::isTerminalState`.

2018-01-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65108/#review195931 --- Ship it! Ship It! - Vinod Kone On Jan. 12, 2018, 1:34 a.m

Re: Review Request 65107: Updated the task state metrics to be more readable.

2018-01-22 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65107/#review195930 --- Ship it! Ship It! - Vinod Kone On Jan. 12, 2018, 1:34 a.m

Re: Review Request 65106: Removed the misleading `isRemovable` helper in the master.

2018-01-22 Thread Vinod Kone
master.cpp Lines 11447 (patched) <https://reviews.apache.org/r/65106/#comment275336> Can you include taskid and frameworkid in the error message here for easy debuggability? - Vinod Kone On Jan. 12, 2018, 1:34 a.m., Benjamin Mahler wrote: > > ---

Re: Review Request 65245: Renamed `LOG` by `Stream logs` in Web UI.

2018-01-22 Thread Vinod Kone
erlink "LOG" that opens up pailer (which is what it is today) and a "Download" button next to it with some space (like we did for sandboxes). - Vinod Kone On Jan. 20, 2018, 4:14 p.m., Armand Grillet wrote: > > --

Re: Review Request 65253: Avoided dropping events on the master operator API stream.

2018-01-22 Thread Vinod Kone
By taking Event as Shared we are only avoiding copies for `TASK_{ADDED,UPDATED}` and `AGENT_REMOVED`. A bit inconsistent, but I guess worth it for the perf improvement? - Vinod Kone On Jan. 22, 2018, 7:14 p.m., Greg Mann wrote: > > --

Re: Review Request 65231: Fixed detaching task volume directories of destroyed frameworks.

2018-01-18 Thread Vinod Kone
g/r/65231/#comment275067> ditto. lets pass `ExecutorInfo` here too. - Vinod Kone On Jan. 19, 2018, 1:25 a.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 65203: Updated the CHANGELOG for 1.5.0 release.

2018-01-17 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/65203/#comment274941> s/Support/support for/ - Vinod Kone On Jan. 18, 2018, 12:21 a.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 64857: Updated example frameworks for mesos-local.

2018-01-16 Thread Vinod Kone
(patched) <https://reviews.apache.org/r/64857/#comment274709> don't you need to set roles in other example frameworks too for the local case? - Vinod Kone On Jan. 15, 2018, 12:29 a.m., Till Toenshoff

Re: Review Request 64849: Added authentication to some example frameworks.

2018-01-16 Thread Vinod Kone
view? src/examples/test_http_framework.cpp Line 401 (original), 390 (patched) <https://reviews.apache.org/r/64849/#comment274708> aah you fixed in this review! - Vinod Kone On Jan. 15, 2018, 12:29 a.m., Ti

Re: Review Request 65184: Added missing registry-related flags to the master config docs.

2018-01-16 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65184/#review195514 --- Ship it! Ship It! - Vinod Kone On Jan. 16, 2018, 11:16 p.m

Re: Review Request 64848: Updated example frameworks to make use of added flags.

2018-01-16 Thread Vinod Kone
/test_http_framework.cpp Lines 399-401 (original), 401-403 (patched) <https://reviews.apache.org/r/64848/#comment274705> hmm. this seems incorrect? how did this compile!? - Vinod Kone On Jan. 15, 2018, 12:29 a.m., Till Toenshoff

Re: Review Request 65167: Detached `virtualLatestPath` when recovering the executor.

2018-01-16 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65167/#review195466 --- Ship it! Ship It! - Vinod Kone On Jan. 15, 2018, 1:49 p.m

Re: Review Request 65070: Updated `ROOT_TaskSandboxPersistentVolume` to check `/files` endpoint.

2018-01-16 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65070/#review195465 --- Ship it! Nice test! - Vinod Kone On Jan. 16, 2018, 3:33 p.m

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-16 Thread Vinod Kone
be a CHECK too like you did for `detach` below? src/slave/slave.cpp Lines 9219 (patched) <https://reviews.apache.org/r/64978/#comment274636> ditto. move comment to the header. - Vinod Kone On Jan. 15,

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-14 Thread Vinod Kone
un directory is gc'ed or if the task goes out of completed. - Vinod Kone On Jan. 14, 2018, 2:30 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 65156: Detached the virtual paths regardless of the result of gc.

2018-01-14 Thread Vinod Kone
is necessary? - Vinod Kone On Jan. 14, 2018, 2:17 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

2018-01-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195380 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 1:56 a.m

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

2018-01-12 Thread Vinod Kone
reachable()); ``` if you kill the `slaves_*` per comment above, i'll leave it upto you whether you want the space or not. - Vinod Kone On Jan. 13, 2018, 12:11 a.m., Greg Mann wrote: > > --- > This

Re: Review Request 65141: Fixed the default executor flaky testes in tests/cluster.cpp.

2018-01-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65141/#review195368 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15 a.m

Re: Review Request 65140: Reverted "Updated composing containerizer tests.".

2018-01-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65140/#review195367 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15 a.m

Re: Review Request 65139: Reverted "Fixed `wait()` and `destroy()` in composing containerizer.".

2018-01-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65139/#review195366 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15 a.m

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

2018-01-12 Thread Vinod Kone
tps://reviews.apache.org/r/65056/#comment274523> lets move this to `GetMaster` response instead of adding it to MasterInfo. - Vinod Kone On Jan. 10, 2018, 11:11 p.m., Greg Mann wrote: > > --- > This is an automatically g

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

2018-01-12 Thread Vinod Kone
18, 3:40 p.m.) > > > Review request for mesos, James Peach and Vinod Kone. > > > Bugs: MESOS-8410 > https://issues.apache.org/jira/browse/MESOS-8410 > > > Repository: mesos > > > Description > --- > > The case where multiple re

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-12 Thread Vinod Kone
resolve them? - Vinod Kone On Jan. 12, 2018, 7:24 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

2018-01-12 Thread Vinod Kone
ttps://reviews.apache.org/r/65074/ > --- > > (Updated Jan. 12, 2018, 3:40 p.m.) > > > Review request for mesos, James Peach and Vinod Kone. > > > Bugs: MESOS-8410 > https://issues.apache.org/jira/browse/MESOS-8410 > > > Repository: mesos > > >

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-12 Thread Vinod Kone
> On Jan. 12, 2018, 2:24 a.m., Vinod Kone wrote: > > AFAICT, in 1.4.x we show unreachable terminal tasks on a removed agent as > > completed tasks. But now, we show them as unreachable tasks. If that's true > > it's an API change that needs to be called out. Is

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

2018-01-11 Thread Vinod Kone
tive policy either. src/tests/slave_compatibility_tests.cpp Lines 215-230 (patched) <https://reviews.apache.org/r/65074/#comment274467> do you really need these many disks for the regression test. are 2 not enough? - Vinod Kone On Jan. 11, 2018, 8:08

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-11 Thread Vinod Kone
scking/tracking/ src/tests/partition_tests.cpp Lines 2513-2514 (patched) <https://reviews.apache.org/r/64940/#comment274463> s/we have one finished task and lost task/the task state is still finished/ - Vinod Kone On Jan. 5, 2018, 7:37 p.m., James

  1   2   3   4   5   6   7   8   9   10   >