Re: Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On May 25, 2017, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59578/
> ---
> 
> (Updated May 25, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than depending on batch allocations to eventually occur, pause
> the clock and explicitly advance it when we want to trigger a batch
> allocation. This improves both test robustness and speed.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59578/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

[Kevin Klues]

> On May 26, 2017, 12:03 a.m., Kevin Klues wrote:
> > src/tests/master_tests.cpp
> > Lines 5635-5636 (patched)
> > 
> >
> > Is this something standard we've been doing recently to fix flaky 
> > tests? I'm not familiar with this pattern. of pausing the clocl and then 
> > advancing it manually.
> 
> Neil Conway wrote:
> We do this fairly often in the tests (`ag "Clock::pause" src/tests | wc 
> -l` => 366). In my opinion, the advantages are:
> 
> * Tests that rely on the clock making progress are more likely to be 
> flaky, because there is no synchronization between how fast the test executes 
> and when clock-based timers go off.
> * A test that is written to assume that clock advances at a certain point 
> has an _implicit_ timing dependency. Pausing the clock and advancing the 
> clock makes this dependency explicit.
> * The test usually runs faster; rather than waiting for the system clock 
> (which might be quite a while), we can immediately advance the clock the 
> appropriate amount.
> 
> Personally, I think we should pause the clock in all tests by default 
> (MESOS-4101).

Makes sense.


- Kevin


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


On May 25, 2017, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59578/
> ---
> 
> (Updated May 25, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than depending on batch allocations to eventually occur, pause
> the clock and explicitly advance it when we want to trigger a batch
> allocation. This improves both test robustness and speed.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59578/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59583: Added a test to verify executor driver message dropping behavior.

[Greg Mann]

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

(Updated May 27, 2017, 1:33 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a test which verifies that the executor driver will drop
RunTaskMessages when it is not connected to the agent.


Diffs (updated)
-

  src/tests/slave_tests.cpp 927b9c3fbc3373b49c799da6e3f770b73964f9e5 


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

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


Testing
---

`GTEST_FILTER="*DisconnectedExecutorDropsMessages*" bin/mesos-tests.sh 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

[Greg Mann]

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

(Updated May 27, 2017, 1:32 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a new test,
`SlaveRecoveryTest.PingTimeoutDuringRecovery`, which verifies
that the agent will reply to pings from the master while it
is performing recovery.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp df0c5c88786190be06df7ef3602834aa8985cefe 


Diff: https://reviews.apache.org/r/59463/diff/5/

Changes: https://reviews.apache.org/r/59463/diff/4-5/


Testing
---

`GTEST_FILTER="*PingTimeoutDuringRecovery*" bin/mesos-tests.sh 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann

Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

[Greg Mann]

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

(Updated May 27, 2017, 1:32 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a test to verify the correct behavior of
the agent flag 'executor_reregistration_timeout.


Diffs (updated)
-

  src/tests/slave_tests.cpp 927b9c3fbc3373b49c799da6e3f770b73964f9e5 


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

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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann

Review Request 59616: Add `six` python package in install_require.

[Zhitao Li]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
`import six` python.


Diffs
-

  src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.

[Joseph Wu]

> On May 26, 2017, 9:52 a.m., Zhitao Li wrote:
> > 3rdparty/cmake/Versions.cmake
> > Lines 30 (patched)
> > 
> >
> > Can you explain how is these hashes geneareted and used?
> > 
> > I'm working on upgrading protobuf library to 3.3.0 so this hash would 
> > change, but I don't know how I can generate a correct value from here...

You can use any SHA-generating utility to get these hashes, like:
```
shasum -a 256 
openssl sha -sha256 
```


- Joseph


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


On May 24, 2017, 12:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59536/
> ---
> 
> (Updated May 24, 2017, 12:13 p.m.)
> 
> 
> Review request for mesos, Aaron Wood and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ExternalProject_Add` uses this hash to verify the tarballs
> automatically. This removes the following warning:
> 
> File will not be verified since no URL_HASH specified
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 
>   3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 
> 
> 
> Diff: https://reviews.apache.org/r/59536/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Linux, cmake --build . --target mesos-tests and stout-tests on 
> Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ilya Pronin]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/port_mapping.hpp
Lines 82-85 (patched)


Isn't 1Mb = 128KB?



src/slave/containerizer/mesos/isolators/network/port_mapping.hpp
Lines 329 (patched)


What's the point of returning a `const Option`? Should be a `const` method 
:)



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 1720-1723 (patched)


Style: `return` statement is overindented.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 1590-1591 (original), 1728-1729 (patched)


Or if `flags.maximum_egress_rate_limit.isSome()`.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 4210-4213 (original), 4505-4507 (patched)


This empty line is needed.


- Ilya Pronin


On May 26, 2017, 7:23 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59294/
> ---
> 
> (Updated May 26, 2017, 7:23 p.m.)
> 
> 
> Review request for mesos, Dmitry Zhuk, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7508
> https://issues.apache.org/jira/browse/MESOS-7508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support to isolators/port_mapping for optionally scaling egress bandwidth 
> with CPU and with minimum and maximum limits.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
>   src/tests/containerizer/port_mapping_tests.cpp 
> d062f2f6bcf7b44dbcde951cdca23b0a2cd42115 
> 
> 
> Diff: https://reviews.apache.org/r/59294/diff/2/
> 
> 
> Testing
> ---
> 
> # added a new test 
> $ make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 59579: Fixed flakiness in MasterTest.EndpointsForHalfRemovedSlave.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On May 25, 2017, 7:05 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59579/
> ---
> 
> (Updated May 25, 2017, 7:05 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test had a race between setting up an expectation and dispatching an
> asynchronous operation that would satisfy the expectation (the former
> needs to happen first).
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59579/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ian Downes]

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

(Updated May 26, 2017, 11:23 a.m.)


Review request for mesos, Dmitry Zhuk, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

Add support to isolators/port_mapping for optionally scaling egress bandwidth 
with CPU and with minimum and maximum limits.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
  src/tests/containerizer/port_mapping_tests.cpp 
d062f2f6bcf7b44dbcde951cdca23b0a2cd42115 


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


Testing
---

# added a new test 
$ make check


Thanks,

Ian Downes

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ian Downes]

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

(Updated May 26, 2017, 11:22 a.m.)


Review request for .


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


Repository: mesos


Description
---

Add support to isolators/port_mapping for optionally scaling egress bandwidth 
with CPU and with minimum and maximum limits.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
  src/tests/containerizer/port_mapping_tests.cpp 
d062f2f6bcf7b44dbcde951cdca23b0a2cd42115 


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

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


Testing
---

# added a new test 
$ make check


Thanks,

Ian Downes

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ian Downes]

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

(Updated May 26, 2017, 11:22 a.m.)


Review request for .


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


Repository: mesos


Description
---

Add support to isolators/port_mapping for optionally scaling egress bandwidth 
with CPU and with minimum and maximum limits.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
  src/tests/containerizer/port_mapping_tests.cpp 
d062f2f6bcf7b44dbcde951cdca23b0a2cd42115 


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

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


Testing
---

# added a new test 
$ make check


Thanks,

Ian Downes

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

[Greg Mann]

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

(Updated May 26, 2017, 5:17 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This patch adds a new test,
`SlaveRecoveryTest.PingTimeoutDuringRecovery`, which verifies
that the agent will reply to pings from the master while it
is performing recovery.


Diffs
-

  src/tests/slave_recovery_tests.cpp 0aa87f534fbc655e3f1aa2ab7f56a1b6be7a8755 


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


Testing
---

`GTEST_FILTER="*PingTimeoutDuringRecovery*" bin/mesos-tests.sh 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann

Re: Review Request 58515: Update 3rdparty build to support protobuf 3.3.0.

[Zhitao Li]

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

(Updated May 26, 2017, 4:58 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

CMake now requires hash (value generated by `shasum -a 256 
3rdparty/protobuf-3.3.0.tar.gz`)


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


Repository: mesos


Description
---

Update 3rdparty build to support protobuf 3.3.0.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake a7a13402d9b8d9795634dc99f488b9c9e6f5d5a8 
  3rdparty/versions.am 55e170a496761c8b9aadb79d03c56d22549e167c 


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

Changes: https://reviews.apache.org/r/58515/diff/5-6/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53841: Added metrics for sorting in the role and quota sorters.

[Jiang Yan Xu]

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



For this and the next review, could you summarize how these metrics can be used 
to reason about the allocator/sorter's performance?

I agree that conceptually we'd like something that tells us how well the (dirty 
or overall) sort performs but it's not immediately clear how to derive that 
from the provided metrics because `sort` on each sorter is called many times 
during one allocation, multiple times per agent. The three sorters are of the 
same implementation, how to interpret the metrics from each? The amount of work 
split between each sorter seems to be pretty dynamic?

Also given the frequency that the timer (relatively expensive) is invoked, how 
much overhead would it cost the sort()? This is probably worth measuring if we 
add these.

- Jiang Yan Xu


On May 23, 2017, 9:23 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53841/
> ---
> 
> (Updated May 23, 2017, 9:23 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Following metrics have been added:
> a) allocator/mesos/roles/sort_runs: Number of role level sorts.
> b) allocator/mesos/roles/sort_run: Latency in role level sorts.
> c) allocator/mesos/quotas/sort_runs: Number of quota level sorts.
> d) allocator/mesos/quotas/sort_run: Latency in quota level sorts.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md cb2833642e7e41c03c98ea92f7300d156a216a2e 
>   src/master/allocator/mesos/hierarchical.hpp 
> 123f97cf495bff0f822838e09df0d88818f04da6 
>   src/master/allocator/sorter/drf/metrics.hpp 
> 61568cb520826ab59d675824b212e0d3deb63764 
>   src/master/allocator/sorter/drf/metrics.cpp 
> ff63fbac5bbcf54e1ae39c3b650c0dafe7ea46d4 
>   src/master/allocator/sorter/drf/sorter.hpp 
> fee58d6d1f08163e2a06a4a20c891fe535c3dcff 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 26b77f578f3235a8792c72d4575d607cdb2c7de7 
>   src/tests/hierarchical_allocator_tests.cpp 
> f90068a50c822aa90b864329ae87c9b5f8bb 
> 
> 
> Diff: https://reviews.apache.org/r/53841/diff/7/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.

[Zhitao Li]

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




3rdparty/cmake/Versions.cmake
Lines 30 (patched)


Can you explain how is these hashes geneareted and used?

I'm working on upgrading protobuf library to 3.3.0 so this hash would 
change, but I don't know how I can generate a correct value from here...


- Zhitao Li


On May 24, 2017, 7:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59536/
> ---
> 
> (Updated May 24, 2017, 7:13 p.m.)
> 
> 
> Review request for mesos, Aaron Wood and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ExternalProject_Add` uses this hash to verify the tarballs
> automatically. This removes the following warning:
> 
> File will not be verified since no URL_HASH specified
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 
>   3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 
> 
> 
> Diff: https://reviews.apache.org/r/59536/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Linux, cmake --build . --target mesos-tests and stout-tests on 
> Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Anand Mazumdar]

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


Fix it, then Ship it!




I would fix this minor thing before committing.


src/tests/protobuf_utils_tests.cpp
Lines 307 (patched)


Extra new line.


- Anand Mazumdar


On May 19, 2017, 5:57 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated May 19, 2017, 5:57 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6644 and MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-6644
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before protobuf 3.3.0, this test would fail because the 64MB limit
> imposed by older version of protobuf.
> 
> 
> Diffs
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58359: Update Mesos build library to use protobuf 3.3.0.

[Anand Mazumdar]

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


Ship it!




For posterity, we would be asking framework developers to upgrade. Mailing list 
discussion: 
http://mail-archives.apache.org/mod_mbox/mesos-user/201704.mbox/%3CCANMwFmV%2B-cbdXp0gSNY0yR_A6HS-GnoLow7zHpc2DFEceo4PjQ%40mail.gmail.com%3E

- Anand Mazumdar


On May 19, 2017, 5:55 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58359/
> ---
> 
> (Updated May 19, 2017, 5:55 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update Mesos build library to use protobuf 3.3.0.
> 
> 
> Diffs
> -
> 
>   configure.ac fe5b20b53ea007dd8f2e3349e9904f6c4f43a14b 
>   src/java/mesos.pom.in 080767f011782f048ab0244f93daea189326587c 
>   src/python/interface/setup.py.in 8e38f3feb705f08737cff93e0ed442a1f48dcbf9 
>   src/python/native_common/ext_modules.py.in 
> e0bb335e5ae663f398cea904a0e53e3c5fb210d7 
>   src/python/protocol/setup.py.in b7de10a2a099d21d6eba551ba071b6c31a1460be 
>   support/mesos-tidy/entrypoint.sh 5dbaa6086621934a7313dfa5c8008b6a6f5496f1 
> 
> 
> Diff: https://reviews.apache.org/r/58359/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 59583: Added a test to verify executor driver message dropping behavior.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [59583, 59582]

Failed command: python support/apply-reviews.py -n -r 59583

Error:
2017-05-26 15:19:47 URL:https://reviews.apache.org/r/59583/diff/raw/ 
[4481/4481] -> "59583.patch" [1]
error: patch failed: src/tests/slave_tests.cpp:7004
error: src/tests/slave_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/18191/console

- Mesos Reviewbot


On May 25, 2017, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59583/
> ---
> 
> (Updated May 25, 2017, 11:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7542
> https://issues.apache.org/jira/browse/MESOS-7542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test which verifies that the executor driver will drop
> RunTaskMessages when it is not connected to the agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59583/diff/1/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*DisconnectedExecutorDropsMessages*" bin/mesos-tests.sh 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 59584: Introduced executor reconnect retries on the agent.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 12:56 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59584/
> ---
> 
> (Updated May 26, 2017, 12:56 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-5332, MESOS-7057 and MESOS-7569
> https://issues.apache.org/jira/browse/MESOS-5332
> https://issues.apache.org/jira/browse/MESOS-7057
> https://issues.apache.org/jira/browse/MESOS-7569
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
> with the agent upon receiving the reconnect message. This avoids the
> executor replying on a half-open TCP connection to the old agent
> (possible if netfilter is dropping packets, see: MESOS-7057).
> However, PID-based executors using Mesos libraries < 1.1.2 do not
> re-link and are therefore prone to replying on a half-open connection
> after the agent restarts. If we only send a single reconnect message,
> these "old" executors will reply on their half-open connection,
> receive a RST, and think the agent just died. To ensure these "old"
> executors can reconnect in the presence of netfilter dropping packets,
> we introduced optional retries of the reconnect message. This results
> in "old" executors correctly establishing a link when processing the
> second reconnect message.
> 
> Generally, users should not enable this flag unless they are affected
> by this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59584/diff/2/
> 
> 
> Testing
> ---
> 
> Added tests in follow up reviews.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 59583: Added a test to verify executor driver message dropping behavior.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 25, 2017, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59583/
> ---
> 
> (Updated May 25, 2017, 11:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7542
> https://issues.apache.org/jira/browse/MESOS-7542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test which verifies that the executor driver will drop
> RunTaskMessages when it is not connected to the agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59583/diff/1/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*DisconnectedExecutorDropsMessages*" bin/mesos-tests.sh 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 59582: Made the executor driver drop some messages when not connected.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 25, 2017, 10:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59582/
> ---
> 
> (Updated May 25, 2017, 10:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7542
> https://issues.apache.org/jira/browse/MESOS-7542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the executor driver to drop RunTaskMessages,
> FrameworkMessages, and StatusUpdateAcknowledgements when it is
> not connected to the agent. This facilitates executor reconnect
> retry logic which is being added to the agent.
> 
> 
> Diffs
> -
> 
>   src/exec/exec.cpp f36e3707d8804b63ec1ca4e7f16728210f0029de 
> 
> 
> Diff: https://reviews.apache.org/r/59582/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 59605: Use glog for logging in executors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59605]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 26, 2017, 10:28 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59605/
> ---
> 
> (Updated May 26, 2017, 10:28 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6961
> https://issues.apache.org/jira/browse/MESOS-6961
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use glog for logging in executors.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 464a74859e0f477ec2e5525a5fc726ea420a00a2 
>   src/launcher/default_executor.cpp 1a60e3bb2b0e8312b349cddedb4cd55716c1b574 
>   src/launcher/executor.cpp 9ac3c3d84c0ec47954c5c72228ad0b8795ff3eec 
> 
> 
> Diff: https://reviews.apache.org/r/59605/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. jenkins: 
> https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI-build/1087/
> 
> 3. I've modified NoTaskKillingCapability from command_executor_tests.cpp to 
> launch command "sleep 0.1" (as well as "sleep 0"),
> then to wait for a task completion. The test was launched in an endless loop:
> 
> GLOG_v=1 ./bin/mesos-tests.sh  --gtest_repeat=-1 --gtest_break_on_failure 
> --gtest_filter="HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0"
>  --verbose
> 
> I've run this test multiple times for a while both on mac os and linux 
> (gru1.hw.ca1.mesosphere.com).
> Race condition problem described in a jira ticket wasn't reproduced.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 59605: Use glog for logging in executors.

[Andrei Budnik]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Use glog for logging in executors.


Diffs
-

  src/docker/executor.cpp 464a74859e0f477ec2e5525a5fc726ea420a00a2 
  src/launcher/default_executor.cpp 1a60e3bb2b0e8312b349cddedb4cd55716c1b574 
  src/launcher/executor.cpp 9ac3c3d84c0ec47954c5c72228ad0b8795ff3eec 


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


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. jenkins: 
https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI-build/1087/

3. I've modified NoTaskKillingCapability from command_executor_tests.cpp to 
launch command "sleep 0.1" (as well as "sleep 0"),
then to wait for a task completion. The test was launched in an endless loop:

GLOG_v=1 ./bin/mesos-tests.sh  --gtest_repeat=-1 --gtest_break_on_failure 
--gtest_filter="HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0"
 --verbose

I've run this test multiple times for a while both on mac os and linux 
(gru1.hw.ca1.mesosphere.com).
Race condition problem described in a jira ticket wasn't reproduced.


Thanks,

Andrei Budnik

Re: Review Request 59601: Removed an unused declaration in type_utils.hpp.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59091, 59094, 59596, 59597, 59598, 59599, 59600, 59601]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 26, 2017, 1:13 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59601/
> ---
> 
> (Updated May 26, 2017, 1:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, Jan 
> Schlicht, and Zhongbo Tian.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an unused declaration in type_utils.hpp.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/59601/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59129: Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.

[Avinash sridharan]

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

(Updated May 26, 2017, 6:38 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Addressed some of Ben's comments.


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


Repository: mesos


Description
---

Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/address.hpp 
6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
  3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 


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

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


Testing
---

make check.


Thanks,

Avinash sridharan