Re: Review Request 64713: Fixed a crash when resubscribing resource providers.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64713/#review194320
---


Ship it!





src/resource_provider/manager.cpp
Lines 605-606 (patched)


nit: Fits on one line.


- Benjamin Bannier


On Dec. 20, 2017, 2:24 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64713/
> ---
> 
> (Updated Dec. 20, 2017, 2:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8346
> https://issues.apache.org/jira/browse/MESOS-8346
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a resource provider resubscribed while its old HTTP connection was
> still open, the agent would crash, as a continuation would be called
> erroneously. This continuation is now only called when a HTTP connection
> is closed by a remote side (i.e. the resource provider) and not when
> the resource provider manager closes the connection.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp e3fcb64b630924e1bb497625708cad3f0fdc064a 
> 
> 
> Diff: https://reviews.apache.org/r/64713/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 64683: Updated support/mesos-style.py to build the virtualenv less often.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64683/#review194327
---



PASS: Mesos patch 64683 was successfully built and tested.

Reviews applied: `['64683']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64683

- Mesos Reviewbot Windows


On Dec. 20, 2017, 4:31 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64683/
> ---
> 
> (Updated Dec. 20, 2017, 4:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, Kevin Klues, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-8217
> https://issues.apache.org/jira/browse/MESOS-8217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of just checking if the virtual environment exists, we now
> also check if it is required to lint the modified files or not.
> 
> The virtual environment will thus be built when using the script as
> a git hook only if JavaScript or Python files have been modified.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 315955e3533fce0cbf820fad03b3707049cdffe0 
> 
> 
> Diff: https://reviews.apache.org/r/64683/diff/3/
> 
> 
> Testing
> ---
> 
> Tested the commit hook with multiple combinations and checked that the result 
> was the one expected:
> 
> 1. Removed support/.virtualenv.
> 2. Updated a .cpp file, commit, the virtualenv is not built.
> 3. Updated a .hpp file and a .md file, commit, the virtualenv is not built.
> 4. Updated a .hpp file, a .md file, and a .py file, commit, the virtualenv is 
> built.
> 5. Removed support/.virtualenv.
> 6. Updated a .cpp file, commit --amend, the virtualenv is built.
> 7. Removed support/.virtualenv.
> 8. Updated a .py file, commit, the virtualenv is built.
> 9. Removed support/.virtualenv.
> 10. Updated a .py file and a .js file, commit, the virtualenv is built.
> 11. Removed support/.virtualenv.
> 12. Updated a .hpp file, a .md file, and a .js file, commit, the virtualenv 
> is built.
> 13. Removed support/.virtualenv.
> 14. Updated a .js file, commit, the virtualenv is built.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64033: Terminated driver-based executors if kill arrives before launch task.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64033/
---

(Updated Dec. 21, 2017, 1:52 p.m.)


Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
Vinod Kone.


Bugs: MESOS-8297
https://issues.apache.org/jira/browse/MESOS-8297


Repository: mesos


Description
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
without this patch an executor never starts a task and remains idle,
ignoring kill task request. This patch ensures all built-in driver-
based executors eventually shut down if kill task arrives before
the task has been started.


Diffs
-

  src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
  src/exec/exec.cpp 7fc46daa633ca42944123a7546ac4eceea93956d 
  src/launcher/executor.cpp 31a47106d7f511220afba4fb382c9252d4671f6e 


Diff: https://reviews.apache.org/r/64033/diff/6/


Testing (updated)
---

make check on MacOS 10.11.6

Manual testing using modified "exec.cpp" that drops executor registration 
confirmation.
```
TEST_P(CommandExecutorTest, KillWithNoLaunch)
{
  Try> master = StartMaster();
  ASSERT_SOME(master);

  Owned detector = master.get()->createDetector();

  slave::Flags flags = CreateSlaveFlags();
  flags.http_command_executor = false;

  Try> slave = StartSlave(detector.get(), flags);
  ASSERT_SOME(slave);

  MockScheduler sched;
  MesosSchedulerDriver driver(
  &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);

  EXPECT_CALL(sched, registered(&driver, _, _));

  Future> offers;
  EXPECT_CALL(sched, resourceOffers(&driver, _))
.WillOnce(FutureArg<1>(&offers))
.WillRepeatedly(Return()); // Ignore subsequent offers.

  driver.start();

  AWAIT_READY(offers);
  EXPECT_EQ(1u, offers->size());

  // Launch a task with the command executor.
  TaskInfo task = createTask(
  offers->front().slave_id(),
  offers->front().resources(),
  SLEEP_COMMAND(1000));

  Future runTaskMessage =
FUTURE_PROTOBUF(RunTaskMessage(), _, _);

  driver.launchTasks(offers->front().id(), {task});

  AWAIT_READY(runTaskMessage);

  // Wait for executor to startup.
  Clock::pause();
  Clock::settle();
  os::sleep(Seconds(1));
  Clock::settle();
  Clock::resume();

  // There should only be a TASK_FAILED update.
  Future statusFailed;
  EXPECT_CALL(sched, statusUpdate(_, _))
.WillOnce(FutureArg<1>(&statusFailed));

  driver.killTask(task.task_id());

  AWAIT_READY(statusFailed);
  EXPECT_EQ(TASK_FAILED, statusFailed->state());

  driver.stop();
  driver.join();
}
```


Thanks,

Alexander Rukletsov

Re: Review Request 64647: Updated tests related to containerizer refactoring.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64647/#review194331
---



PASS: Mesos patch 64647 was successfully built and tested.

Reviews applied: `['64646', '64647']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64647

- Mesos Reviewbot Windows


On Dec. 15, 2017, 11:41 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64647/
> ---
> 
> (Updated Dec. 15, 2017, 11:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is needed due to changes in Containerizer interface,
> where `wait()`, `destroy()` and `kill()` methods return the same
> ContainerTermination type.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.hpp d6a69d6327018065453f8a6cb275d921d10c9198 
>   src/tests/containerizer.cpp 13a290ff60e359ceced95b20cb9db8c25cd59dbc 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> 7c22f162b128c3fdf8d4b20cac73fdf442449d79 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 630bb2e352dd55ecb401730c84f823e0a1f2d310 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> f217d02bba2d1372392ad4da521940d27e4f69d4 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 0729464a7a9b290c8634536e58909f9b38b58a9c 
>   src/tests/containerizer/mock_containerizer.hpp 
> 01c617a55899a7125ca1dc80091aef5603419ee8 
> 
> 
> Diff: https://reviews.apache.org/r/64647/diff/3/
> 
> 
> Testing
> ---
> 
> make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 64683: Updated support/mesos-style.py to build the virtualenv less often.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64683/#review194334
---


Ship it!




Ship It!

- Till Toenshoff


On Dec. 20, 2017, 4:31 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64683/
> ---
> 
> (Updated Dec. 20, 2017, 4:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, Kevin Klues, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-8217
> https://issues.apache.org/jira/browse/MESOS-8217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of just checking if the virtual environment exists, we now
> also check if it is required to lint the modified files or not.
> 
> The virtual environment will thus be built when using the script as
> a git hook only if JavaScript or Python files have been modified.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 315955e3533fce0cbf820fad03b3707049cdffe0 
> 
> 
> Diff: https://reviews.apache.org/r/64683/diff/3/
> 
> 
> Testing
> ---
> 
> Tested the commit hook with multiple combinations and checked that the result 
> was the one expected:
> 
> 1. Removed support/.virtualenv.
> 2. Updated a .cpp file, commit, the virtualenv is not built.
> 3. Updated a .hpp file and a .md file, commit, the virtualenv is not built.
> 4. Updated a .hpp file, a .md file, and a .py file, commit, the virtualenv is 
> built.
> 5. Removed support/.virtualenv.
> 6. Updated a .cpp file, commit --amend, the virtualenv is built.
> 7. Removed support/.virtualenv.
> 8. Updated a .py file, commit, the virtualenv is built.
> 9. Removed support/.virtualenv.
> 10. Updated a .py file and a .js file, commit, the virtualenv is built.
> 11. Removed support/.virtualenv.
> 12. Updated a .hpp file, a .md file, and a .js file, commit, the virtualenv 
> is built.
> 13. Removed support/.virtualenv.
> 14. Updated a .js file, commit, the virtualenv is built.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64033: Terminated driver-based executors if kill arrives before launch task.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64033/#review194335
---


Ship it!




Ship It!

- Andrei Budnik


On Dec. 21, 2017, 1:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Dec. 21, 2017, 1:52 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8297
> https://issues.apache.org/jira/browse/MESOS-8297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> without this patch an executor never starts a task and remains idle,
> ignoring kill task request. This patch ensures all built-in driver-
> based executors eventually shut down if kill task arrives before
> the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/exec/exec.cpp 7fc46daa633ca42944123a7546ac4eceea93956d 
>   src/launcher/executor.cpp 31a47106d7f511220afba4fb382c9252d4671f6e 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/6/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> Manual testing using modified "exec.cpp" that drops executor registration 
> confirmation.
> ```
> TEST_P(CommandExecutorTest, KillWithNoLaunch)
> {
>   Try> master = StartMaster();
>   ASSERT_SOME(master);
> 
>   Owned detector = master.get()->createDetector();
> 
>   slave::Flags flags = CreateSlaveFlags();
>   flags.http_command_executor = false;
> 
>   Try> slave = StartSlave(detector.get(), flags);
>   ASSERT_SOME(slave);
> 
>   MockScheduler sched;
>   MesosSchedulerDriver driver(
>   &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
> 
>   EXPECT_CALL(sched, registered(&driver, _, _));
> 
>   Future> offers;
>   EXPECT_CALL(sched, resourceOffers(&driver, _))
> .WillOnce(FutureArg<1>(&offers))
> .WillRepeatedly(Return()); // Ignore subsequent offers.
> 
>   driver.start();
> 
>   AWAIT_READY(offers);
>   EXPECT_EQ(1u, offers->size());
> 
>   // Launch a task with the command executor.
>   TaskInfo task = createTask(
>   offers->front().slave_id(),
>   offers->front().resources(),
>   SLEEP_COMMAND(1000));
> 
>   Future runTaskMessage =
> FUTURE_PROTOBUF(RunTaskMessage(), _, _);
> 
>   driver.launchTasks(offers->front().id(), {task});
> 
>   AWAIT_READY(runTaskMessage);
> 
>   // Wait for executor to startup.
>   Clock::pause();
>   Clock::settle();
>   os::sleep(Seconds(1));
>   Clock::settle();
>   Clock::resume();
> 
>   // There should only be a TASK_FAILED update.
>   Future statusFailed;
>   EXPECT_CALL(sched, statusUpdate(_, _))
> .WillOnce(FutureArg<1>(&statusFailed));
> 
>   driver.killTask(task.task_id());
> 
>   AWAIT_READY(statusFailed);
>   EXPECT_EQ(TASK_FAILED, statusFailed->state());
> 
>   driver.stop();
>   driver.join();
> }
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64069: Ensured command executor always honors shutdown request.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64069/#review194337
---


Ship it!




Ship It!

- Andrei Budnik


On Dec. 20, 2017, 8:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64069/
> ---
> 
> (Updated Dec. 20, 2017, 8:14 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 31a47106d7f511220afba4fb382c9252d4671f6e 
> 
> 
> Diff: https://reviews.apache.org/r/64069/diff/2/
> 
> 
> Testing
> ---
> 
> Writing a proper test is tricky: our test harness does not allow to drop 
> outgoing messages and hence `RunTaskMessage` from the agent to the executor 
> can't be dropped (since executor is running in a separate process it can't be 
> dropped there as incoming message either). So I've tested this manually via 
> dropping `RunTaskMessage` in `exec.cpp` and using the following test:
> ```
> TEST_P(CommandExecutorTest, ShutdownWithNoLaunch)
> {
>   Try> master = StartMaster();
>   ASSERT_SOME(master);
> 
>   Owned detector = master.get()->createDetector();
> 
>   slave::Flags flags = CreateSlaveFlags();
>   flags.http_command_executor = false;
> 
>   Try> slave = StartSlave(detector.get(), flags);
>   ASSERT_SOME(slave);
> 
>   MockScheduler sched;
>   MesosSchedulerDriver driver(
>   &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
> 
>   EXPECT_CALL(sched, registered(&driver, _, _));
> 
>   Future> offers;
>   EXPECT_CALL(sched, resourceOffers(&driver, _))
> .WillOnce(FutureArg<1>(&offers))
> .WillRepeatedly(Return()); // Ignore subsequent offers.
> 
>   driver.start();
> 
>   AWAIT_READY(offers);
>   EXPECT_EQ(1u, offers->size());
> 
>   // Launch a task with the command executor.
>   TaskInfo task = createTask(
>   offers->front().slave_id(),
>   offers->front().resources(),
>   SLEEP_COMMAND(1000));
> 
>   driver.launchTasks(offers->front().id(), {task});
> 
>   driver.stop();
>   driver.join();
> }
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64770/
---

Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.


Bugs: MESOS-7506
https://issues.apache.org/jira/browse/MESOS-7506


Repository: mesos


Description
---

There was a race condition leading to flaky
`LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
This test launches successively multiple agents, while reusing the same
variable. After reassigning the value of the variable, agent's d'tor is
called. If agent recovery is not yet completed, then some orphaned
container might blink in the agent's d'tor as it described in the
comment to the code.


Diffs
-

  src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 


Diff: https://reviews.apache.org/r/64770/diff/1/


Testing
---

sudo make check (fedora 25)
internal CI


Thanks,

Andrei Budnik

Re: Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64070/#review194339
---




src/executor/v0_v1executor.cpp
Lines 192 (patched)


Can you please move this check to the beginning of the method for better 
code consistency here and in `shutdown`, `killTask` methods?


- Andrei Budnik


On Dec. 20, 2017, 8:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64070/
> ---
> 
> (Updated Dec. 20, 2017, 8:16 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, if an error, kill, or shutdown occurred during
> subscription / registration with the agent, it was not propagated back
> to the executor if the v0_v1 executor adapter was used. This happened
> because the adapter did not call the `connected` callback until after
> successful registration and hence the executor did not even try to
> send the `SUBSCRIBE` call, without which the adapter did not send any
> events to the executor.
> 
> A fix is to call the `connected` callback if an error occurred or
> shutdown / kill event arrived before the executor had subscribed.
> 
> 
> Diffs
> -
> 
>   src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 
> 
> 
> Diff: https://reviews.apache.org/r/64070/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64070/#review194340
---


Ship it!




Ship It!

- Andrei Budnik


On Dec. 20, 2017, 8:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64070/
> ---
> 
> (Updated Dec. 20, 2017, 8:16 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, if an error, kill, or shutdown occurred during
> subscription / registration with the agent, it was not propagated back
> to the executor if the v0_v1 executor adapter was used. This happened
> because the adapter did not call the `connected` callback until after
> successful registration and hence the executor did not even try to
> send the `SUBSCRIBE` call, without which the adapter did not send any
> events to the executor.
> 
> A fix is to call the `connected` callback if an error occurred or
> shutdown / kill event arrived before the executor had subscribed.
> 
> 
> Diffs
> -
> 
>   src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 
> 
> 
> Diff: https://reviews.apache.org/r/64070/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64683: Updated support/mesos-style.py to build the virtualenv less often.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64683/#review194342
---




support/mesos-style.py
Line 408 (original), 408 (patched)


The docstring needs to be updated.



support/mesos-style.py
Lines 438 (patched)


The regex feels a bit obscure, and it can actually crash if the file name 
contains unbalanced parenthesis (not that anybody would make commit a file like 
this, but still :D)

I believe the pythonic way would be more like (in pseudo-code)

```
# The virtualenv needs to be rebuilt if we modified a js or a python file 
and the directory doesn't exist

if set([js_and_python_files]) & set(basenames):
  return os.path.isdir(os.path.join('support', '.virtualenv'))
```

On the other hand, if the source_files variables actually contain 
regex-like search patterns, ignore the above comment ;)

In any case, I think the message saying that the virtualenv is being built 
should be moved to the place where the virtualenv is actually being built.


- Benno Evers


On Dec. 20, 2017, 4:31 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64683/
> ---
> 
> (Updated Dec. 20, 2017, 4:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, Kevin Klues, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-8217
> https://issues.apache.org/jira/browse/MESOS-8217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of just checking if the virtual environment exists, we now
> also check if it is required to lint the modified files or not.
> 
> The virtual environment will thus be built when using the script as
> a git hook only if JavaScript or Python files have been modified.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 315955e3533fce0cbf820fad03b3707049cdffe0 
> 
> 
> Diff: https://reviews.apache.org/r/64683/diff/3/
> 
> 
> Testing
> ---
> 
> Tested the commit hook with multiple combinations and checked that the result 
> was the one expected:
> 
> 1. Removed support/.virtualenv.
> 2. Updated a .cpp file, commit, the virtualenv is not built.
> 3. Updated a .hpp file and a .md file, commit, the virtualenv is not built.
> 4. Updated a .hpp file, a .md file, and a .py file, commit, the virtualenv is 
> built.
> 5. Removed support/.virtualenv.
> 6. Updated a .cpp file, commit --amend, the virtualenv is built.
> 7. Removed support/.virtualenv.
> 8. Updated a .py file, commit, the virtualenv is built.
> 9. Removed support/.virtualenv.
> 10. Updated a .py file and a .js file, commit, the virtualenv is built.
> 11. Removed support/.virtualenv.
> 12. Updated a .hpp file, a .md file, and a .js file, commit, the virtualenv 
> is built.
> 13. Removed support/.virtualenv.
> 14. Updated a .js file, commit, the virtualenv is built.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64070/
---

(Updated Dec. 21, 2017, 3:40 p.m.)


Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
Vinod Kone.


Repository: mesos


Description
---

Prior to this patch, if an error, kill, or shutdown occurred during
subscription / registration with the agent, it was not propagated back
to the executor if the v0_v1 executor adapter was used. This happened
because the adapter did not call the `connected` callback until after
successful registration and hence the executor did not even try to
send the `SUBSCRIBE` call, without which the adapter did not send any
events to the executor.

A fix is to call the `connected` callback if an error occurred or
shutdown / kill event arrived before the executor had subscribed.


Diffs (updated)
-

  src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 


Diff: https://reviews.apache.org/r/64070/diff/3/

Changes: https://reviews.apache.org/r/64070/diff/2-3/


Testing
---


Thanks,

Alexander Rukletsov

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64770/#review194344
---



PASS: Mesos patch 64770 was successfully built and tested.

Reviews applied: `['64770']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64770

- Mesos Reviewbot Windows


On Dec. 21, 2017, 2:57 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64770/
> ---
> 
> (Updated Dec. 21, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race condition leading to flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
> This test launches successively multiple agents, while reusing the same
> variable. After reassigning the value of the variable, agent's d'tor is
> called. If agent recovery is not yet completed, then some orphaned
> container might blink in the agent's d'tor as it described in the
> comment to the code.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 
> 
> 
> Diff: https://reviews.apache.org/r/64770/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 64778: Display time zone in log messages.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64778/
---

Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Repository: mesos


Description
---

This patches glog to always display the time zone of all
log messages.

To turn off this behaviour, the environment variable
`GLOG_LOG_TIMEZONE` or the command-line flag `--glog_log_timezone`
can be set to false.


Diffs
-

  3rdparty/CMakeLists.txt 579aed83fc5c9cee17cf9b4b289a91705f64ceda 
  3rdparty/Makefile.am 805c6e534555ca3ff4320d4cdb6bfb64a36c5600 
  3rdparty/glog-print-time-zone.patch PRE-CREATION 


Diff: https://reviews.apache.org/r/64778/diff/1/


Testing
---

https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/2446/


Thanks,

Benno Evers

Review Request 64777: Modified the autotools build for multiple patch files.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64777/
---

Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Repository: mesos


Description
---

Previously, the make rules assumed that there is at most
one patch file per dependencies, whose name follows the
pattern $(dependency-name)-$(version).patch

This was changed to allow specifying any number of files
to be applied to the source tree after unpacking the
archive.


Diffs
-

  3rdparty/Makefile.am 805c6e534555ca3ff4320d4cdb6bfb64a36c5600 


Diff: https://reviews.apache.org/r/64777/diff/1/


Testing
---


Thanks,

Benno Evers

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64770/
---

(Updated Dec. 21, 2017, 3:58 p.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.


Bugs: MESOS-7506
https://issues.apache.org/jira/browse/MESOS-7506


Repository: mesos


Description (updated)
---

There was a race condition leading to flaky
`LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
This test launches successively multiple agents, while reusing the same
variable. After reassigning the value of the variable, agent's d'tor is
called. If agent recovery is not yet completed, then some orphaned
container might blink in the agent's d'tor as it is described in the
comment to the code.


Diffs
-

  src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 


Diff: https://reviews.apache.org/r/64770/diff/1/


Testing
---

sudo make check (fedora 25)
internal CI


Thanks,

Andrei Budnik

Re: Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64070/
---

(Updated Dec. 21, 2017, 4:07 p.m.)


Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
Vinod Kone.


Changes
---

Removed debug traces.


Repository: mesos


Description
---

Prior to this patch, if an error, kill, or shutdown occurred during
subscription / registration with the agent, it was not propagated back
to the executor if the v0_v1 executor adapter was used. This happened
because the adapter did not call the `connected` callback until after
successful registration and hence the executor did not even try to
send the `SUBSCRIBE` call, without which the adapter did not send any
events to the executor.

A fix is to call the `connected` callback if an error occurred or
shutdown / kill event arrived before the executor had subscribed.


Diffs (updated)
-

  src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 


Diff: https://reviews.apache.org/r/64070/diff/4/

Changes: https://reviews.apache.org/r/64070/diff/3-4/


Testing
---


Thanks,

Alexander Rukletsov

Re: Review Request 64754: Removed `kilobytes()`, `megabytes()`, etc from `Bytes`.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64754/#review194346
---


Fix it, then Ship it!





3rdparty/stout/include/stout/bytes.hpp
Lines 145 (patched)


Do you need this?


- Jie Yu


On Dec. 21, 2017, 3:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64754/
> ---
> 
> (Updated Dec. 21, 2017, 3:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These functios are removed because they are lossy. The unit conversion
> for `Resource` should be explicit. For example:
>   `resource.scalar().set_value((double) size.bytes() / Bytes::MEGABYTES)`
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/bytes.hpp 
> cbe953662bb86c2f1eef3453f557ea17c0c6d13e 
> 
> 
> Diff: https://reviews.apache.org/r/64754/diff/2/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64778: Display time zone in log messages.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64778/#review194348
---



PASS: Mesos patch 64778 was successfully built and tested.

Reviews applied: `['64777', '64778']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64778

- Mesos Reviewbot Windows


On Dec. 21, 2017, 3:56 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64778/
> ---
> 
> (Updated Dec. 21, 2017, 3:56 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patches glog to always display the time zone of all
> log messages.
> 
> To turn off this behaviour, the environment variable
> `GLOG_LOG_TIMEZONE` or the command-line flag `--glog_log_timezone`
> can be set to false.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 579aed83fc5c9cee17cf9b4b289a91705f64ceda 
>   3rdparty/Makefile.am 805c6e534555ca3ff4320d4cdb6bfb64a36c5600 
>   3rdparty/glog-print-time-zone.patch PRE-CREATION 
>   docs/upgrades.md f2eb5a94d67b44a02d355981a673bd4ef5129653 
> 
> 
> Diff: https://reviews.apache.org/r/64778/diff/2/
> 
> 
> Testing
> ---
> 
> https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/2446/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64755: Removed the usage of `Bytes::kilobytes()` and `Bytes::megabytes()`.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64755/#review194351
---


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Lines 255-256 (original), 255-258 (patched)


Hum, I'd avoid using `parse`, instead, create it like you did before.



src/slave/containerizer/containerizer.cpp
Line 161 (original), 161 (patched)


Can we preserve the existing behavior?



src/slave/containerizer/containerizer.cpp
Line 189 (original), 189 (patched)


Ditto. Can we preserve the existing behavior?



src/slave/slave.cpp
Lines 5614 (patched)


I'd preserve the existing behavior in this patch


- Jie Yu


On Dec. 21, 2017, 3:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64755/
> ---
> 
> (Updated Dec. 21, 2017, 3:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the usage of `Bytes::kilobytes()` and `Bytes::megabytes()`.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_executor.cpp 7343ee72c1fc8d6e58527809ffb74fc5dd09ee0e 
>   src/examples/balloon_framework.cpp a36e04b033ba66ba911abde2ad3d6f58decae18a 
>   src/examples/disk_full_framework.cpp 
> d9d2d3513529f3a19f392b314001930746d5c91d 
>   src/master/validation.cpp 7f5a67d98dbbd5a5d64bc73d530f3d37b3cdfec6 
>   src/resource_provider/storage/provider.cpp 
> 833d929e93afd0bb99a9574df24168e82269bc81 
>   src/slave/containerizer/containerizer.cpp 
> 935dfc9ea787ae714a86c7bc54811703c23267c8 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 113e908743b718e7ad645e8e08b481ed97a4021f 
>   src/slave/slave.cpp 6271cfe56733ec121d0d7c691e3dd0792435bf2d 
>   src/tests/container_logger_tests.cpp 
> b65cf6a9096dfaa418fd7eb400771cc6df642ea2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 630bb2e352dd55ecb401730c84f823e0a1f2d310 
>   src/tests/hierarchical_allocator_tests.cpp 
> 173e4fbac184ad8d40c8adba19ad64225f11f1f2 
>   src/tests/mesos.hpp 41f47cf1b10fddd51e260e2127e2695ffe1aeba5 
>   src/tests/persistent_volume_tests.cpp 
> 854676918dba497e1a8c21d877631c6e5c75ee2e 
> 
> 
> Diff: https://reviews.apache.org/r/64755/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64770/#review194352
---



PASS: Mesos patch 64770 was successfully built and tested.

Reviews applied: `['64770']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64770

- Mesos Reviewbot Windows


On Dec. 21, 2017, 3:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64770/
> ---
> 
> (Updated Dec. 21, 2017, 3:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race condition leading to flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
> This test launches successively multiple agents, while reusing the same
> variable. After reassigning the value of the variable, agent's d'tor is
> called. If agent recovery is not yet completed, then some orphaned
> container might blink in the agent's d'tor as it is described in the
> comment to the code.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 
> 
> 
> Diff: https://reviews.apache.org/r/64770/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 64756: Fixed a bug that SLRP would fail if the state files do not exist.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64756/#review194355
---


Ship it!




Ship It!

- Jie Yu


On Dec. 20, 2017, 10:14 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64756/
> ---
> 
> (Updated Dec. 20, 2017, 10:14 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug that SLRP would fail if the state files do not exist.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 833d929e93afd0bb99a9574df24168e82269bc81 
> 
> 
> Diff: https://reviews.apache.org/r/64756/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64763: Fixed compilation for python eggs when gRPC is enabled.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64763/#review194356
---


Ship it!




Ship It!

- Jie Yu


On Dec. 21, 2017, 3:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64763/
> ---
> 
> (Updated Dec. 21, 2017, 3:41 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation for python eggs when gRPC is enabled.
> 
> 
> Diffs
> -
> 
>   src/python/native_common/ext_modules.py.in 
> 47c2d24a6e5e49ad5860d96108bbb1e2eeb8dd5b 
> 
> 
> Diff: https://reviews.apache.org/r/64763/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64755: Removed the usage of `Bytes::kilobytes()` and `Bytes::megabytes()`.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64755/
---

(Updated Dec. 21, 2017, 7:01 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Vinod 
Kone.


Changes
---

Addressed Jie's comments.


Repository: mesos


Description
---

Removed the usage of `Bytes::kilobytes()` and `Bytes::megabytes()`.


Diffs (updated)
-

  src/examples/balloon_executor.cpp 7343ee72c1fc8d6e58527809ffb74fc5dd09ee0e 
  src/examples/balloon_framework.cpp a36e04b033ba66ba911abde2ad3d6f58decae18a 
  src/examples/disk_full_framework.cpp d9d2d3513529f3a19f392b314001930746d5c91d 
  src/master/validation.cpp 1e1e75447b6ed12566bf66fced23606a0f2f6a15 
  src/resource_provider/storage/provider.cpp 
b6781d6ebddb00351dde28454d34b5fab6f36453 
  src/slave/containerizer/containerizer.cpp 
935dfc9ea787ae714a86c7bc54811703c23267c8 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
113e908743b718e7ad645e8e08b481ed97a4021f 
  src/slave/slave.cpp e091d822acf6b607c2bb265bd5e4011da145952a 
  src/tests/container_logger_tests.cpp b65cf6a9096dfaa418fd7eb400771cc6df642ea2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
630bb2e352dd55ecb401730c84f823e0a1f2d310 
  src/tests/hierarchical_allocator_tests.cpp 
c98bebb36f27480d4f21eb85ad9b1c27de8b9e82 
  src/tests/mesos.hpp 5e833a1e23bfba5cbc58f77bfcd6cd4e4647a99e 
  src/tests/persistent_volume_tests.cpp 
5962dd13e49c515fffdd62cc4883ba224e421a06 


Diff: https://reviews.apache.org/r/64755/diff/3/

Changes: https://reviews.apache.org/r/64755/diff/2-3/


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64686/
---

(Updated Dec. 21, 2017, 7:42 p.m.)


Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.


Bugs: MESOS-6616
https://issues.apache.org/jira/browse/MESOS-6616


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


Diff: https://reviews.apache.org/r/64686/diff/2/

Changes: https://reviews.apache.org/r/64686/diff/1-2/


Testing
---

make check on Mac OS 10.11.6 and various Linux distros


Thanks,

Alexander Rukletsov

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64686/#review194361
---


Fix it, then Ship it!




Thanks for the updated patch Alex, for me this version is much clearer.


src/linux/ns.cpp
Line 397 (original), 397-399 (patched)


Why do we put `struct` here? This is C++ (and the `memcpy` fits on a single 
line w/o it).

Not yours, but we should probably also include `sys/socket.h` for `ucred` 
in this file.



src/linux/ns.cpp
Line 455 (original), 459 (patched)


`struct`?



src/linux/ns.cpp
Lines 472 (patched)


`struct`?


- Benjamin Bannier


On Dec. 21, 2017, 8:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64686/
> ---
> 
> (Updated Dec. 21, 2017, 8:42 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6616
> https://issues.apache.org/jira/browse/MESOS-6616
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64686/diff/2/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64686/#review194363
---



PASS: Mesos patch 64686 was successfully built and tested.

Reviews applied: `['64685', '64686']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64686

- Mesos Reviewbot Windows


On Dec. 21, 2017, 7:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64686/
> ---
> 
> (Updated Dec. 21, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6616
> https://issues.apache.org/jira/browse/MESOS-6616
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64686/diff/2/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 62285: Added tombstone flag to NOP log action.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62285/#review194367
---


Ship it!




Ship It!

- Jie Yu


On Sept. 25, 2017, 1:05 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62285/
> ---
> 
> (Updated Sept. 25, 2017, 1:05 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> NOP action is used as a filler for holes in the "fill" procedure and as
> the action in a response to a promise request for a truncated position.
> The tombstone flag is introduced so that replicas are able to
> distinguish the truncated action from a real NOP.
> 
> 
> Diffs
> -
> 
>   src/log/leveldb.cpp 5310a123b0fb25f240429722b676fe46174cb2ce 
>   src/log/replica.cpp 39f2879b2e37c1ca9a9f987ce0a3b83e8dbc9b43 
>   src/messages/log.proto ca740bd011147f8f48cc5dfcacefa5fdbd95ccd0 
>   src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 
> 
> 
> Diff: https://reviews.apache.org/r/62285/diff/2/
> 
> 
> Testing
> ---
> 
> Added a test that verifies that a replica correctly handles the tombstone 
> NOP. Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62760: Exposed runRecoverProtocol() function in recover.hpp.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62760/#review194368
---


Ship it!




Ship It!

- Jie Yu


On Oct. 3, 2017, 11:15 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62760/
> ---
> 
> (Updated Oct. 3, 2017, 11:15 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function is needed catch-up missing positions of a non-leading
> VOTING replica.
> 
> 
> Diffs
> -
> 
>   src/log/recover.hpp 94d6bb283ea82df607fcc72de05cda3b12ad5d2d 
>   src/log/recover.cpp b366818767adb259ee6805f9978fd330b790a264 
> 
> 
> Diff: https://reviews.apache.org/r/62760/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62761: Moved retry logic from RecoverProtocolProcess to RecoverProcess.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62761/#review194369
---


Ship it!




Ship It!

- Jie Yu


On Oct. 3, 2017, 11:15 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62761/
> ---
> 
> (Updated Oct. 3, 2017, 11:15 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved retry logic from RecoverProtocolProcess to RecoverProcess.
> 
> 
> Diffs
> -
> 
>   src/log/recover.hpp 94d6bb283ea82df607fcc72de05cda3b12ad5d2d 
>   src/log/recover.cpp b366818767adb259ee6805f9978fd330b790a264 
>   src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 
> 
> 
> Diff: https://reviews.apache.org/r/62761/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test that verifies that retry logic is working in `recover()`. Ran 
> `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62286: Added CatchupMissing log process.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62286/#review194370
---


Ship it!




Ship It!

- Jie Yu


On Oct. 3, 2017, 11:16 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62286/
> ---
> 
> (Updated Oct. 3, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The process is used to catch-up a non-leading VOTING replica by running
> the recovery protocol to find current begin and end positions of the log
> and catching-up positions that are missing on the replica. This allows
> following replicas to serve eventually consistent reads.
> 
> 
> Diffs
> -
> 
>   src/log/catchup.hpp 123bc7a57e5e89f9ba75c36ba0cbe5ead807c518 
>   src/log/catchup.cpp 94e1b00db2cd9d5a2368a979c1fd155bb6cac1f2 
>   src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 
> 
> 
> Diff: https://reviews.apache.org/r/62286/diff/2/
> 
> 
> Testing
> ---
> 
> Added tests that verify that the new recovery process correctly performs and 
> produces meaningful result under various circumstances (recovered positions 
> were truncated, replica was lagging far behind). Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62287: Added Log::Reader::catchup() method.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62287/#review194371
---


Ship it!




Ship It!

- Jie Yu


On Oct. 5, 2017, 8:05 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62287/
> ---
> 
> (Updated Oct. 5, 2017, 8:05 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new method catches-up positions missing in the local non-leading
> replica to allow safe eventually consistent reads from it.
> 
> 
> Diffs
> -
> 
>   include/mesos/log/log.hpp d92d7a0b982c1c4bc0fdedde19fe012a6523a224 
>   src/log/log.hpp 24481aaae145301e7a8ea1c91b1c16e48ded4843 
>   src/log/log.cpp dcb66f7dd5da5059925b7b731e83f7320f645030 
>   src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 
> 
> 
> Diff: https://reviews.apache.org/r/62287/diff/2/
> 
> 
> Testing
> ---
> 
> Added a test that verifies that it's possible to catch-up the lagging replica 
> using the `Reader` and successfully read entries that were caught-up. Ran 
> `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62288: Added Log.Reader.catchup() method to Java bindings.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62288/#review194372
---


Ship it!




Ship It!

- Jie Yu


On Sept. 13, 2017, 4:40 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62288/
> ---
> 
> (Updated Sept. 13, 2017, 4:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new method is a wrapper for Log::Reader::catchup() method of the C++
> Replicated Log API.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_Log.cpp 
> eb0f54be39dfced0dd75dcefa808850f22a34dc6 
>   src/java/src/org/apache/mesos/Log.java 
> f53d9ebeed3fc30e9e9dc642320e94da18eb8ca1 
> 
> 
> Diff: https://reviews.apache.org/r/62288/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Review Request 64790: Fixed a compile error due to string literals and macros.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64790/
---

Review request for mesos and Chun-Hung Hsiao.


Repository: mesos


Description
---

Fixed a compile error due to string literals and macros.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
31d736d0dc4e28ebfd885d02c01fb0b807106a0a 


Diff: https://reviews.apache.org/r/64790/diff/1/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 64790: Fixed a compile error due to string literals and macros.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64790/#review194376
---


Ship it!




Ship It!

- Chun-Hung Hsiao


On Dec. 21, 2017, 11:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64790/
> ---
> 
> (Updated Dec. 21, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a compile error due to string literals and macros.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 31d736d0dc4e28ebfd885d02c01fb0b807106a0a 
> 
> 
> Diff: https://reviews.apache.org/r/64790/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 64790: Fixed a compile error due to string literals and macros.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64790/#review194381
---



PASS: Mesos patch 64790 was successfully built and tested.

Reviews applied: `['64790']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64790

- Mesos Reviewbot Windows


On Dec. 21, 2017, 3:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64790/
> ---
> 
> (Updated Dec. 21, 2017, 3:30 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a compile error due to string literals and macros.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 31d736d0dc4e28ebfd885d02c01fb0b807106a0a 
> 
> 
> Diff: https://reviews.apache.org/r/64790/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 64792: Fixed a bug where quota headroom is tracked incorrectly.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64792/
---

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed a bug where quota headroom is tracked incorrectly.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
7f7dae1950bebd22191189c2db0f6678d2e8fa3c 


Diff: https://reviews.apache.org/r/64792/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 64794: Added a test to verify the behavior of allocating resources w/o quota.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64794/
---

Review request for mesos and Benjamin Mahler.


Bugs: MESOS-8352
https://issues.apache.org/jira/browse/MESOS-8352


Repository: mesos


Description
---

This test verifies the behavior of allocating resource to quota roles
that has no quota set for that particular resource (e.g. allocating
memory to a role with only quota set for CPU). If a role has no quota
set for a resource, it will get that resource only when two conditions
are both met:

- It got some other resources on the same agent to meet its quota;

- After allocating the resource, there is still enough resource
available to meet all other role's quota guarantee for that resource.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
ad9d556f040b2b51b6c43757f8bad05d61a057e7 


Diff: https://reviews.apache.org/r/64794/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 64793: Fixed a bug where resource over allocation may break quota headroom.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64793/
---

Review request for mesos and Benjamin Mahler.


Bugs: MESOS-8352
https://issues.apache.org/jira/browse/MESOS-8352


Repository: mesos


Description
---

In the quota role allocation stage, if a role gets some resources
on an agent to meet its quota, it will also get all other resources
on the same agent that it does not have quota for. This may starve
roles behind it that have quotas set for those resources.

We fix this issue by enforcing that, in the quota role allocation
stage, if a role has no quota set for a scalar resource, it will
get that resource only when two conditions are both met:

(1) It got some other resources on the same agent to meet its quota;

(2) After allocating those resources, quota headroom is still above
the required amount.

Also refactored the fine-grained quota allocation logic.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
7f7dae1950bebd22191189c2db0f6678d2e8fa3c 


Diff: https://reviews.apache.org/r/64793/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 64792: Fixed a bug where quota headroom is tracked incorrectly.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64792/#review194387
---


Ship it!




Ship It!

- Benjamin Mahler


On Dec. 22, 2017, 12:47 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64792/
> ---
> 
> (Updated Dec. 22, 2017, 12:47 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only resources allocated towards a role's quota should be
> subtracted from `requiredHeadroom`. In addition to this,
> unreserved resources in the quota allocation stage may also
> contain resources that a role does not have quota for, they
> should not be subtracted from `requiredHeadroom`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f7dae1950bebd22191189c2db0f6678d2e8fa3c 
> 
> 
> Diff: https://reviews.apache.org/r/64792/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64794: Added a test to verify the behavior of allocating resources w/o quota.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64794/#review194392
---



PASS: Mesos patch 64794 was successfully built and tested.

Reviews applied: `['64793', '64794']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64794

- Mesos Reviewbot Windows


On Dec. 22, 2017, 12:48 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64794/
> ---
> 
> (Updated Dec. 22, 2017, 12:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8352
> https://issues.apache.org/jira/browse/MESOS-8352
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies the behavior of allocating resource to quota roles
> that has no quota set for that particular resource (e.g. allocating
> memory to a role with only quota set for CPU). If a role has no quota
> set for a resource, it will get that resource only when two conditions
> are both met:
> 
> - It got some other resources on the same agent to meet its quota;
> 
> - After allocating the resource, there is still enough resource
> available to meet all other role's quota guarantee for that resource.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ad9d556f040b2b51b6c43757f8bad05d61a057e7 
> 
> 
> Diff: https://reviews.apache.org/r/64794/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64793: Fixed a bug where resource over allocation may break quota headroom.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64793/#review194386
---



s/This may starve roles behind it that have quotas set for those 
resources./This may violate the guarantees of that other roles have set on 
these resources/ ?


src/master/allocator/mesos/hierarchical.cpp
Lines 1848-1890 (patched)


Perhaps something like shrink?

```
  // Returns true if we shrunk the resource to the target size,
  // or the resource is already within the target size. Returns
  // false otherwise (i.e. the resource is indivisible. E.g.
  // MOUNT volume).
  bool shrink = [](Resource* r, const Value::Scalar& target) {
if (r->scalar() <= target) {
  return true; // Already within target.
}

Resource copy = *r;
copy.mutable_scalar()->CopyFrom(target);

if (Resources(*r).contains(copy)) {
  r->CopyFrom(copy);
  return true;
}

return false;
  };
```

We could put this in the resources library.



src/master/allocator/mesos/hierarchical.cpp
Lines 1905-1917 (patched)


Perhaps this needs to be merged into the overall comment above? Seems like 
we just need to update the above comment to reflect the additional headroom 
check?



src/master/allocator/mesos/hierarchical.cpp
Lines 1893-1895 (original), 1952-1955 (patched)


Perhaps a formula here would be helpful?

```
// Compute the amount of resources that can be allocated
// without violating quota headroom:
//
//   Allocation Limit = (Available Headroom - Required Headroom) -
//  Tentative Allocation to Role
```

Something like this?



src/master/allocator/mesos/hierarchical.cpp
Lines 1894-1895 (original), 1953-1955 (patched)


I couldn't quite follow this formula, why do we need to subtract the last 
two?



src/master/allocator/mesos/hierarchical.cpp
Lines 1928-1929 (original), 1976-1978 (patched)


A little indirect but "progress towards quota" means that the resources are 
non empty, right? Maybe we want to explicitly mention that?


- Benjamin Mahler


On Dec. 22, 2017, 12:48 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64793/
> ---
> 
> (Updated Dec. 22, 2017, 12:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8352
> https://issues.apache.org/jira/browse/MESOS-8352
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the quota role allocation stage, if a role gets some resources
> on an agent to meet its quota, it will also get all other resources
> on the same agent that it does not have quota for. This may starve
> roles behind it that have quotas set for those resources.
> 
> We fix this issue by enforcing that, in the quota role allocation
> stage, if a role has no quota set for a scalar resource, it will
> get that resource only when two conditions are both met:
> 
> (1) It got some other resources on the same agent to meet its quota;
> 
> (2) After allocating those resources, quota headroom is still above
> the required amount.
> 
> Also refactored the fine-grained quota allocation logic.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f7dae1950bebd22191189c2db0f6678d2e8fa3c 
> 
> 
> Diff: https://reviews.apache.org/r/64793/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64794: Added a test to verify the behavior of allocating resources w/o quota.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64794/#review194395
---


Fix it, then Ship it!




Nice test! Just some minor suggestions below and what looks like an inaccurate 
comment?


src/tests/hierarchical_allocator_tests.cpp
Lines 3495 (patched)


a resource?



src/tests/hierarchical_allocator_tests.cpp
Lines 3501 (patched)


Maybe: It was allocated quota resources on the agent



src/tests/hierarchical_allocator_tests.cpp
Lines 3503-3504 (patched)


Maybe: It can be allocated without violating the quota headroom



src/tests/hierarchical_allocator_tests.cpp
Lines 3505 (patched)


QuotaRoleAllocateNonQuotaResource?



src/tests/hierarchical_allocator_tests.cpp
Lines 3581 (patched)


Shouldn't memory be absent here?


- Benjamin Mahler


On Dec. 22, 2017, 12:48 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64794/
> ---
> 
> (Updated Dec. 22, 2017, 12:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8352
> https://issues.apache.org/jira/browse/MESOS-8352
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies the behavior of allocating resource to quota roles
> that has no quota set for that particular resource (e.g. allocating
> memory to a role with only quota set for CPU). If a role has no quota
> set for a resource, it will get that resource only when two conditions
> are both met:
> 
> - It got some other resources on the same agent to meet its quota;
> 
> - After allocating the resource, there is still enough resource
> available to meet all other role's quota guarantee for that resource.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ad9d556f040b2b51b6c43757f8bad05d61a057e7 
> 
> 
> Diff: https://reviews.apache.org/r/64794/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/#review194396
---



Hm.. I was a little puzzled here, why do some use convert with POST and some 
use the reflection based upgrade? Shouldn't we just use the reflection based 
upgrade everywhere?

- Benjamin Mahler


On Dec. 20, 2017, 12:51 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> ---
> 
> (Updated Dec. 20, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 79d7f602bbc57bce308157dfbb6c2afaef806f23 
>   src/slave/containerizer/mesos/paths.cpp 
> 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64770/#review194394
---



This looks like a reasonable solution to me. However, it would be great if we 
could reproduce the bug and then verify the fix. Looking at the log of a failed 
test run in the JIRA, it seems to me that the problem occurs when cleanup of an 
orphaned container left over from a previous test is attempted by the agent 
destructor called during 
`LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags`. To attempt a repro, I 
would suggest the following:
1) Peg the CPU on the machine so that libprocess takes a long time to process 
messages in its queue
2) Run `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` and one other 
(fast-running) test which creates a container, setting '--gtest_repeat=-1'

Hopefully, this may recreate the circumstances which led to the failure in CI?


src/tests/cluster.cpp
Lines 619 (patched)


s/immidiately/immediately/



src/tests/cluster.cpp
Lines 620 (patched)


s/agent, so the containerizer might return/agent. Thus, the containerizer 
might return a/



src/tests/cluster.cpp
Lines 626 (patched)


s/ownedContainerizer.get()/ownedContainerizer.get() != nullptr/


- Greg Mann


On Dec. 21, 2017, 3:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64770/
> ---
> 
> (Updated Dec. 21, 2017, 3:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race condition leading to flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
> This test launches successively multiple agents, while reusing the same
> variable. After reassigning the value of the variable, agent's d'tor is
> called. If agent recovery is not yet completed, then some orphaned
> container might blink in the agent's d'tor as it is described in the
> comment to the code.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 
> 
> 
> Diff: https://reviews.apache.org/r/64770/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 64798: Added an utility to shrink scalar resource while keeping its meta-data.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64798/
---

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added an utility to shrink scalar resource while keeping its meta-data.


Diffs
-

  include/mesos/resources.hpp eefe9eac23dd1eeb35df9ba1d109a446526ee0c3 
  include/mesos/v1/resources.hpp 7a5b0e3cf156b04a232e014e87c49f8f1baa9133 
  src/common/resources.cpp 919a03b6fa19fc9db4242a32fba3c617d1705413 
  src/v1/resources.cpp 365ffa7b7f05d95321a8078a91f5bb17b2ee9419 


Diff: https://reviews.apache.org/r/64798/diff/1/


Testing
---


Thanks,

Meng Zhu

Re: Review Request 64793: Fixed a bug where resource over allocation may break quota headroom.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64793/
---

(Updated Dec. 21, 2017, 8:17 p.m.)


Review request for mesos and Benjamin Mahler.


Bugs: MESOS-8352
https://issues.apache.org/jira/browse/MESOS-8352


Repository: mesos


Description
---

In the quota role allocation stage, if a role gets some resources
on an agent to meet its quota, it will also get all other resources
on the same agent that it does not have quota for. This may starve
roles behind it that have quotas set for those resources.

We fix this issue by enforcing that, in the quota role allocation
stage, if a role has no quota set for a scalar resource, it will
get that resource only when two conditions are both met:

(1) It got some other resources on the same agent to meet its quota;

(2) After allocating those resources, quota headroom is still above
the required amount.

Also refactored the fine-grained quota allocation logic.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
78d7b2203532301ff26b7abb26f5d320831124a1 


Diff: https://reviews.apache.org/r/64793/diff/2/

Changes: https://reviews.apache.org/r/64793/diff/1-2/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 64794: Added a test to verify the behavior of allocating resources w/o quota.

[Meng Zhu]

> On Dec. 21, 2017, 6:52 p.m., Benjamin Mahler wrote:
> > Nice test! Just some minor suggestions below and what looks like an 
> > inaccurate comment?

Thanks! Comments fixed.


- Meng


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64794/#review194395
---


On Dec. 21, 2017, 4:48 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64794/
> ---
> 
> (Updated Dec. 21, 2017, 4:48 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8352
> https://issues.apache.org/jira/browse/MESOS-8352
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies the behavior of allocating resource to quota roles
> that has no quota set for that particular resource (e.g. allocating
> memory to a role with only quota set for CPU). If a role has no quota
> set for a resource, it will get that resource only when two conditions
> are both met:
> 
> - It got some other resources on the same agent to meet its quota;
> 
> - After allocating the resource, there is still enough resource
> available to meet all other role's quota guarantee for that resource.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ad9d556f040b2b51b6c43757f8bad05d61a057e7 
> 
> 
> Diff: https://reviews.apache.org/r/64794/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64798: Added an utility to shrink scalar resource while keeping its meta-data.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64798/#review194403
---



PASS: Mesos patch 64798 was successfully built and tested.

Reviews applied: `['64798']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64798

- Mesos Reviewbot Windows


On Dec. 22, 2017, 4:08 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64798/
> ---
> 
> (Updated Dec. 22, 2017, 4:08 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an utility to shrink scalar resource while keeping its meta-data.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp eefe9eac23dd1eeb35df9ba1d109a446526ee0c3 
>   include/mesos/v1/resources.hpp 7a5b0e3cf156b04a232e014e87c49f8f1baa9133 
>   src/common/resources.cpp 919a03b6fa19fc9db4242a32fba3c617d1705413 
>   src/v1/resources.cpp 365ffa7b7f05d95321a8078a91f5bb17b2ee9419 
> 
> 
> Diff: https://reviews.apache.org/r/64798/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64794: Added a test to verify the behavior of allocating resources w/o quota.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64794/#review194404
---



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['64798', '64793', '64794']`

Failed command: 
`D:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64794

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64794/logs/libprocess-tests-stdout.log):

```
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
D:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(446): error: 
(await_subprocess(client.get(), 0)).failure(): 
[++] Subprocess output.
[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from SSLClientTest

[ RUN  ] SSLClientTest.client

[   OK ] SSLClientTest.client (20 ms)

[--] 1 test from SSLClientTest (20 ms total)



[--] Global test environment tear-down

[==] 1 test from 1 test case ran. (20 ms total)

[  PASSED  ] 1 test.

[++]

[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false" (1028 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (959 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (2727 ms total)

[--] Global test environment tear-down
[==] 223 tests from 35 test cases ran. (35821 ms total)
[  PASSED  ] 222 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false"

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64794/logs/libprocess-tests-stderr.log):

```
LIBPROCESS_SSL_VERIFY_CERT set to true
I1222 05:32:39.823956  6652 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\SeGh36\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1222 05:32:40.865159  9968 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1222 05:32:40.866159  9968 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1222 05:32:40.866159  9968 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1222 05:32:40.866159  9968 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1222 05:32:40.867161  9968 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7NlRel\cert.pem
bject alternative name certificate extension.
I1222 05:32:38.952924   472 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1222 05:32:38.953924   472 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\BJGZv9\cert.pem
I1222 05:32:38.953924   472 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\BJGZv9
I1222 05:32:39.245934   472 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1222 05:32:39.245934   472 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1222 05:32:39.245934   472 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1222 05:32:39.245934   472 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\SeGh36\cert.pem
I1222 05:32:40.287140   472 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1222 05:32:40.287140   472 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1222 05:32:40.288141   472 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1222 05:32:40.288141   472 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1222 05:32:40.288141   472 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7NlRel\cert.pem
I1222 05:32:40.978648  4808 process.cpp:887] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Dec. 22, 2017, 12:48 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64794/
> ---
> 
> (Updated Dec. 22, 20