date:20150917

Re: Review Request 38158: Refactored Value::Ranges coalesce().

2015-09-17 Thread Joerg Schad


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

(Updated Sept. 18, 2015, 6:39 a.m.)


Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
Toenshoff.


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


Repository: mesos


Description
---

The goal of this refactoring was to reuse the Ranges objects as much as 
possible, as prior there was substantial time spend in allocation/destruction 
(MESOS-3051).


Diffs (updated)
-

  include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
  src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
  src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
  src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 

Diff: https://reviews.apache.org/r/38158/diff/


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 38470: Maintenance Primitives: Fix error in master's Accept/Decline for inverse offers.

2015-09-17 Thread Guangya Liu


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

Ship it!


Ship It!

- Guangya Liu


On 九月 17, 2015, 10:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38470/
> ---
> 
> (Updated 九月 17, 2015, 10:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add regression test.  Note that the test may be slow until inverse offers 
> filters are actually implemented.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
> 
> Diff: https://reviews.apache.org/r/38470/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38158]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 5:17 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 18, 2015, 5:17 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

2015-09-17 Thread Joerg Schad


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

(Updated Sept. 18, 2015, 5:17 a.m.)


Review request for mesos, Bernd Mathiske and Till Toenshoff.


Changes
---

removed duplicated line...


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


Repository: mesos


Description
---

The goal of this refactoring was to reuse the Ranges objects as much as 
possible, as prior there was substantial time spend in allocation/destruction 
(MESOS-3051).


Diffs (updated)
-

  include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
  src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
  src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
  src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 

Diff: https://reviews.apache.org/r/38158/diff/


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37532, 37873, 38119, 38120, 38121, 38124]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 3:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38124/
> ---
> 
> (Updated Sept. 18, 2015, 3:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add V1 Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler/scheduler.proto 
> 0118b46afca8a5adecb0e65981dd44bb98333bab 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38102: MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to UUID::random)

2015-09-17 Thread Ben Mahler


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

Ship it!


Thanks for your patience!


3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp (line 33)


Typically we name types with a capital in the first letter, like 
'Generator', but let's just remove the typedef here, it doesn't seem to be 
helping a lot?


- Ben Mahler


On Sept. 15, 2015, 9:39 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38102/
> ---
> 
> (Updated Sept. 15, 2015, 9:39 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3046
> https://issues.apache.org/jira/browse/MESOS-3046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> Performance downgrade
> 
> __Root Cause:__
> stout's UUID abstraction is re-seeding the random generator during each call 
> to UUID::random(), which is really expensive.
> 
> __Solution:__
> Seeding the random generator only once per thread.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp e90dabb 
> 
> Diff: https://reviews.apache.org/r/38102/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:57 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


Changes
---

Address vinod's comments


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


Repository: mesos


Description
---

Add V1 Support for QuiesceOffers


Diffs (updated)
-

  include/mesos/v1/scheduler/scheduler.proto 
0118b46afca8a5adecb0e65981dd44bb98333bab 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

Diff: https://reviews.apache.org/r/38124/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38120: Add Java Support for QuiesceOffers

2015-09-17 Thread Guangya Liu



> On 九月 15, 2015, 7:12 p.m., Vinod Kone wrote:
> >
> 
> Vinod Kone wrote:
> can you also update the java test framework to test this call?
> 
> Guangya Liu wrote:
> I see that the reviveOffers also do not have a example, I want to handle 
> those two APIs together in another patch, make sense? Thanks.
> 
> Vinod Kone wrote:
> sg, please create a ticket if you haven't already.

Filed a bug https://issues.apache.org/jira/browse/MESOS-3460 here, Thanks!


- Guangya


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


On 九月 18, 2015, 3:55 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38120/
> ---
> 
> (Updated 九月 18, 2015, 3:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Java Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> a89ebed00f93801fc5bfb18e947f7120ef77d095 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
> b9b2ea8734bb910c543708dd8adcdac45e03c34b 
>   src/java/src/org/apache/mesos/SchedulerDriver.java 
> 183eec898553ed25149c58d2c1f85cf6579d5660 
> 
> Diff: https://reviews.apache.org/r/38120/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38121: Add Python Support for QuiesceOffers

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:56 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add Python Support for QuiesceOffers


Diffs (updated)
-

  src/python/interface/src/mesos/interface/__init__.py 
686abb24d88765d7ec7cab5b76b57299339b7b00 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp 
b1ad8e529806a047480e8d7affe4e7ecdc7d9725 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
609dfa3735f19eea2242dfb82ed3a4af3a0b8b9b 

Diff: https://reviews.apache.org/r/38121/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:55 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


Changes
---

Address vinod's comments


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


Repository: mesos


Description
---

Add quiesce logic in allocator


Diffs (updated)
-

  include/mesos/master/allocator.hpp fb09e2a6502bc8c78ddcc8a595bcd9320da136ea 
  src/master/allocator/mesos/allocator.hpp 
171548b2017a0b97124f052c21345668e274d117 
  src/master/allocator/mesos/hierarchical.hpp 
3374d63b8311cf10b3108f56b7b167c12a9d7a37 
  src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

Diff: https://reviews.apache.org/r/37873/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38120: Add Java Support for QuiesceOffers

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:55 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add Java Support for QuiesceOffers


Diffs (updated)
-

  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
a89ebed00f93801fc5bfb18e947f7120ef77d095 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
b9b2ea8734bb910c543708dd8adcdac45e03c34b 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
183eec898553ed25149c58d2c1f85cf6579d5660 

Diff: https://reviews.apache.org/r/38120/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38119: Add metrics of messages_quiesce_offers

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:55 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add metrics of messages_quiesce_offers


Diffs (updated)
-

  src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
  src/master/metrics.hpp 2d07a16f2dc6811973c82259c3cccff07401b542 
  src/master/metrics.cpp d79206f08181c99641bef697be57b5d57b627285 
  src/tests/master_tests.cpp 06d74c39b9a1674d2809b13c02eae757ba75b58c 
  src/tests/metrics_tests.cpp 3e9d7c2eadfcdd2522d65e4a55dc76808ba39159 

Diff: https://reviews.apache.org/r/38119/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

2015-09-17 Thread Guangya Liu


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

(Updated 九月 18, 2015, 3:54 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs (updated)
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
19f548dba4b5d72fca8e692c5f15796feca17106 
  src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
  src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
  src/sched/sched.cpp a1723f3cdd05289b417b4ea8bdd9b000655eccf8 

Diff: https://reviews.apache.org/r/37532/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Jian Qiu



> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote:
> > Thanks for fixing this, but I'm wondering how this fixes the issue.
> > (in fact, not even what is it testing).
> > 
> > If you invoke `cat` without input, it will just hang there waiting on 
> > STDIN, how does this make the test work?
> > (sorry for all the questions, but unfortunately I'm not familiar w/ this 
> > particular test)
> > 
> > Can you please add more information in the Description field?
> > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's 
> > related to, but what folks need to understand is: (a) what the problem was; 
> > (b) how it affected the test and (c) what you did to fix it.
> > 
> > Also, how does this preserve the spirit of the test? I can fix any test by 
> > just having ASSERT(true) :)
> > 
> > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test!
> 
> haosdent huang wrote:
> Hi @marco. "cat" is equals to "sleep forever" command(we conld not 
> execute sleep forver only could sleep xxx). I think this patch should fix the 
> problem according I repeat test this patch tonight. And the cause of this 
> issue has beed documented in jira by @qiujian. The old command sometimes have 
> two processes when running so this test case is flaky.
> 
> haosdent huang wrote:
> Ref: Bash: infinite sleep (infinite blocking) 
> http://stackoverflow.com/questions/2935183/bash-infinite-sleep-infinite-blocking

Thanks for comments, I have udpated the description accordingly.


- Jian


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


On Sept. 18, 2015, 2:51 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 18, 2015, 2:51 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is to isolate one single process with on thread in cgroup, and 
> checks the number of processes and threads in the cgroup before/after 
> isolation. The previous test case uses "sh -c "while true; do sleep 1; 
> done;"" as the process, however, it periodically forks a child process "sleep 
> 1" in cgroup causing the test failure (expected number of process/thread 
> should be 1, but here is 2). This patch is to issue a "cat" command, so there 
> will be only one process in cgroup.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Jian Qiu


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

(Updated Sept. 18, 2015, 2:51 a.m.)


Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This test is to isolate one single process with on thread in cgroup, and checks 
the number of processes and threads in the cgroup before/after isolation. The 
previous test case uses "sh -c "while true; do sleep 1; done;"" as the process, 
however, it periodically forks a child process "sleep 1" in cgroup causing the 
test failure (expected number of process/thread should be 1, but here is 2). 
This patch is to issue a "cat" command, so there will be only one process in 
cgroup.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

Diff: https://reviews.apache.org/r/38454/diff/


Testing
---

./mesos-tests.sh 
--gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
--gtest_repeat=1000 --gtest_break_on_failure


Thanks,

Jian Qiu

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38443]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 1:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 18, 2015, 1:17 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese


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

(Updated Sept. 18, 2015, 1:17 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Added guard for as. Also moved arguments around.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

Diff: https://reviews.apache.org/r/38443/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38003]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 12:13 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 18, 2015, 12:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
>   src/tests/master_tests.cpp 06d74c3 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Klaus Ma


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

(Updated Sept. 18, 2015, 12:13 a.m.)


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


Changes
---

Address comments


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


Repository: mesos


Description
---

__Phenomenon:__
In some race condition, the slave was shutdown when after master failover.

__Root Cause:__
The slave was shutdown because of duplicated SlavID: in master, the SlaveID is 
genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
pid) maybe un-changed which lead to duplicated SlaveID. 

__Solution/Fix:__
Generate masterInfo.id by UUID instead of "date + ip + port + pid".


Diffs (updated)
-

  src/master/master.cpp 1c4e7af 
  src/tests/master_tests.cpp 06d74c3 

Diff: https://reviews.apache.org/r/38003/diff/


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Klaus Ma



> On Sept. 17, 2015, 9:13 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, line 3733
> > 
> >
> > why the change in formatting? we allow 80 char comments now?

do you mean the following comments? Unfortunately, its length is 82 if in one 
line.

// If both the slaves get the same SlaveID, the re-registration would
// fail here.


- Klaus


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


On Sept. 18, 2015, 12:13 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 18, 2015, 12:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
>   src/tests/master_tests.cpp 06d74c3 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38470: Maintenance Primitives: Fix error in master's Accept/Decline for inverse offers.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38470]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 10:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38470/
> ---
> 
> (Updated Sept. 17, 2015, 10:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add regression test.  Note that the test may be slow until inverse offers 
> filters are actually implemented.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
> 
> Diff: https://reviews.apache.org/r/38470/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

2015-09-17 Thread Joerg Schad


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

(Updated Sept. 17, 2015, 11:34 p.m.)


Review request for mesos, Bernd Mathiske and Till Toenshoff.


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


Repository: mesos


Description
---

The goal of this refactoring was to reuse the Ranges objects as much as 
possible, as prior there was substantial time spend in allocation/destruction 
(MESOS-3051).


Diffs (updated)
-

  include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
  src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
  src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
  src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 

Diff: https://reviews.apache.org/r/38158/diff/


Testing
---

make check


Thanks,

Joerg Schad

Review Request 38470: Maintenance Primitives: Fix error in master's Accept/Decline for inverse offers.

2015-09-17 Thread Joseph Wu


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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
Remoortere.


Repository: mesos


Description
---

Add regression test.  Note that the test may be slow until inverse offers 
filters are actually implemented.


Diffs
-

  src/master/master.cpp 1c4e7af7448c05f54c1068d6496ed21c8c359ac5 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 

Diff: https://reviews.apache.org/r/38470/diff/


Testing
---

`make check`


Thanks,

Joseph Wu

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [38447]

Failed command: ./support/apply-review.sh -n -r 38447

Error:
 2015-09-17 22:37:55 URL:https://reviews.apache.org/r/38447/diff/raw/ 
[17738/17738] -> "38447.patch" [1]
error: patch failed: src/slave/containerizer/isolators/filesystem/linux.hpp:100
error: src/slave/containerizer/isolators/filesystem/linux.hpp: patch does not 
apply
error: patch failed: src/slave/containerizer/isolators/filesystem/linux.cpp:360
error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does not 
apply
error: patch failed: src/tests/containerizer/filesystem_isolator_tests.cpp:26
error: src/tests/containerizer/filesystem_isolator_tests.cpp: patch does not 
apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 17, 2015, 10:03 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38447/
> ---
> 
> (Updated Sept. 17, 2015, 10:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3433
> https://issues.apache.org/jira/browse/MESOS-3433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented a TODO to clean up host mounts irrelevant to the container in the 
> container's mount namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> b6746345b70b462123c3254065e4f09a42f45e5b 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ca9f423d5f2f8f838c3371ee596f07537bb45210 
> 
> Diff: https://reviews.apache.org/r/38447/diff/
> 
> 
> Testing
> ---
> 
> Added a test that is not specific for this but observed successful cleanup of 
> the other container's mounts in logs.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Artem Harutyunyan


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


LGTM, thanks for doing this, Neil!

- Artem Harutyunyan


On Sept. 17, 2015, 2:28 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 17, 2015, 2:28 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38472: Made filesystem/posix the default filesystem isolator.

2015-09-17 Thread Jiang Yan Xu


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

Ship it!


Ship It!

- Jiang Yan Xu


On Sept. 17, 2015, 3:11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38472/
> ---
> 
> (Updated Sept. 17, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made filesystem/posix the default filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e8a89bfdd8508ec84b0ec2e09c2ef634ff28e455 
> 
> Diff: https://reviews.apache.org/r/38472/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

2015-09-17 Thread Vinod Kone

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

Can you write a test for this?

src/master/master.cpp (lines 5552 - 5554)

This comment is a bit confusing. The master receiving a second terminal 
update doesn't mean the slave sent it by mistake. It could happen if the 
executor sends it too. I would change this as follows

// Once a task's state has been transitioned to terminal state, no further
// terminal updates should result in a state change. These are the same
// semantics that are enforced by the slave.

- Vinod Kone

On Sept. 5, 2015, 11:41 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 5, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38471: Fixed MESOS-3430 by making the sandbox/work directory mount as shared and slave.

2015-09-17 Thread Jiang Yan Xu


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

Ship it!


Ship It!

- Jiang Yan Xu


On Sept. 17, 2015, 2:59 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38471/
> ---
> 
> (Updated Sept. 17, 2015, 2:59 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3430
> https://issues.apache.org/jira/browse/MESOS-3430
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed MESOS-3430 by making the sandbox/work directory mount as shared and 
> slave.
> 
> 'Shared and slave' is a state, see 
> https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt
> 
> This is mainly for those systems that mark '/' as a shared mount by default.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> b6746345b70b462123c3254065e4f09a42f45e5b 
> 
> Diff: https://reviews.apache.org/r/38471/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Tested on a box with '/' marked as a shared mount.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38468]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 9:28 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 17, 2015, 9:28 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 38472: Made filesystem/posix the default filesystem isolator.

2015-09-17 Thread Jie Yu


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

Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.


Repository: mesos


Description
---

Made filesystem/posix the default filesystem isolator.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
e8a89bfdd8508ec84b0ec2e09c2ef634ff28e455 

Diff: https://reviews.apache.org/r/38472/diff/


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Jie Yu


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

Ship it!


Ship It!

- Jie Yu


On Sept. 17, 2015, 10:03 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38447/
> ---
> 
> (Updated Sept. 17, 2015, 10:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3433
> https://issues.apache.org/jira/browse/MESOS-3433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented a TODO to clean up host mounts irrelevant to the container in the 
> container's mount namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> b6746345b70b462123c3254065e4f09a42f45e5b 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ca9f423d5f2f8f838c3371ee596f07537bb45210 
> 
> Diff: https://reviews.apache.org/r/38447/diff/
> 
> 
> Testing
> ---
> 
> Added a test that is not specific for this but observed successful cleanup of 
> the other container's mounts in logs.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Jiang Yan Xu


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

(Updated Sept. 17, 2015, 3:03 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.


Changes
---

Minor style fix.


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


Repository: mesos


Description
---

Implemented a TODO to clean up host mounts irrelevant to the container in the 
container's mount namespace.


Diffs (updated)
-

  src/slave/containerizer/isolators/filesystem/linux.hpp 
99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
b6746345b70b462123c3254065e4f09a42f45e5b 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
ca9f423d5f2f8f838c3371ee596f07537bb45210 

Diff: https://reviews.apache.org/r/38447/diff/


Testing
---

Added a test that is not specific for this but observed successful cleanup of 
the other container's mounts in logs.


Thanks,

Jiang Yan Xu

Re: Review Request 38471: Fixed MESOS-3430 by making the sandbox/work directory mount as shared and slave.

2015-09-17 Thread Jie Yu


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

(Updated Sept. 17, 2015, 9:59 p.m.)


Review request for mesos, haosdent huang, Timothy Chen, and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

Fixed MESOS-3430 by making the sandbox/work directory mount as shared and slave.

'Shared and slave' is a state, see 
https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt

This is mainly for those systems that mark '/' as a shared mount by default.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
b6746345b70b462123c3254065e4f09a42f45e5b 

Diff: https://reviews.apache.org/r/38471/diff/


Testing
---

sudo make check

Tested on a box with '/' marked as a shared mount.


Thanks,

Jie Yu

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Jiang Yan Xu


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

(Updated Sept. 17, 2015, 2:58 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.


Changes
---

The previous version had a bug which caused unable to clean up multiple mounts 
and it masked another issue in filesystem tests that rootfses are not put under 
a directory with the container ID in its path. This caused some mounts such as 
the persistent volumes which belonged to the same container to be mistakenly 
cleaned up!


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


Repository: mesos


Description
---

Implemented a TODO to clean up host mounts irrelevant to the container in the 
container's mount namespace.


Diffs (updated)
-

  src/slave/containerizer/isolators/filesystem/linux.hpp 
99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
b6746345b70b462123c3254065e4f09a42f45e5b 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
ca9f423d5f2f8f838c3371ee596f07537bb45210 

Diff: https://reviews.apache.org/r/38447/diff/


Testing
---

Added a test that is not specific for this but observed successful cleanup of 
the other container's mounts in logs.


Thanks,

Jiang Yan Xu

Review Request 38471: Fixed MESOS-3430 by making the sandbox/work directory mount as shared and slave.

2015-09-17 Thread Jie Yu


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

Review request for mesos, haosdent huang, Timothy Chen, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Fixed MESOS-3430 by making the sandbox/work directory mount as shared and slave.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
b6746345b70b462123c3254065e4f09a42f45e5b 

Diff: https://reviews.apache.org/r/38471/diff/


Testing
---

sudo make check

Tested on a box with '/' marked as a shared mount.


Thanks,

Jie Yu

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-17 Thread Vinod Kone


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



src/tests/mesos.hpp (line 1608)


Needs a test for the scheduler driver. I've only see one for the http 
scheduler library.

Also, can you send a review for CHANGELOG and upgrades.md updates for 
0.25.0? feel free to send that after this chain gets committed, to avoid making 
this chain even bigger.


- Vinod Kone


On Sept. 16, 2015, 5:50 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Sept. 16, 2015, 5:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp fb09e2a6502bc8c78ddcc8a595bcd9320da136ea 
>   src/master/allocator/mesos/allocator.hpp 
> 171548b2017a0b97124f052c21345668e274d117 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3374d63b8311cf10b3108f56b7b167c12a9d7a37 
>   src/master/master.cpp f26271c5b21685916c0654488ac1464f21d72e9a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-17 Thread Vinod Kone


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

Ship it!



src/tests/scheduler_tests.cpp (line 1021)


s/we enabled quiesce offer/the framework quiesced offers./



src/tests/scheduler_tests.cpp (lines 1030 - 1031)


// Framework should receive offers only after calling reviving offers.


- Vinod Kone


On Sept. 16, 2015, 6:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38124/
> ---
> 
> (Updated Sept. 16, 2015, 6:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add V1 Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler/scheduler.proto 
> 0118b46afca8a5adecb0e65981dd44bb98333bab 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38120: Add Java Support for QuiesceOffers

2015-09-17 Thread Vinod Kone



> On Sept. 15, 2015, 7:12 p.m., Vinod Kone wrote:
> >
> 
> Vinod Kone wrote:
> can you also update the java test framework to test this call?
> 
> Guangya Liu wrote:
> I see that the reviveOffers also do not have a example, I want to handle 
> those two APIs together in another patch, make sense? Thanks.

sg, please create a ticket if you haven't already.


- Vinod


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


On Sept. 16, 2015, 5:50 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38120/
> ---
> 
> (Updated Sept. 16, 2015, 5:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Java Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> a89ebed00f93801fc5bfb18e947f7120ef77d095 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
> b9b2ea8734bb910c543708dd8adcdac45e03c34b 
>   src/java/src/org/apache/mesos/SchedulerDriver.java 
> 183eec898553ed25149c58d2c1f85cf6579d5660 
> 
> Diff: https://reviews.apache.org/r/38120/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Neil Conway


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

(Updated Sept. 17, 2015, 9:28 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.


Repository: mesos


Description
---

Added suggestion that new contributors seek to find a shepherd for their work
before their begin writing code. Made a few other improvements.


Diffs (updated)
-

  docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
  docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 

Diff: https://reviews.apache.org/r/38468/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-17 Thread Vinod Kone


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

Ship it!



src/master/allocator/mesos/hierarchical.hpp (line 247)


s/quiesce/quiesces/ ?



src/master/allocator/mesos/hierarchical.hpp (line 1123)


s/is/has/


- Vinod Kone


On Sept. 16, 2015, 5:50 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Sept. 16, 2015, 5:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp fb09e2a6502bc8c78ddcc8a595bcd9320da136ea 
>   src/master/allocator/mesos/allocator.hpp 
> 171548b2017a0b97124f052c21345668e274d117 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3374d63b8311cf10b3108f56b7b167c12a9d7a37 
>   src/master/master.cpp f26271c5b21685916c0654488ac1464f21d72e9a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

2015-09-17 Thread Vinod Kone


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

Ship it!



include/mesos/scheduler.hpp (line 271)


s/, the/. The/


- Vinod Kone


On Sept. 16, 2015, 5:51 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37532/
> ---
> 
> (Updated Sept. 16, 2015, 5:51 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just part of MESOS-3037, this patch only add the interface
> of QUIESCE call.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   include/mesos/scheduler/scheduler.proto 
> 19f548dba4b5d72fca8e692c5f15796feca17106 
>   src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
>   src/master/master.cpp f26271c5b21685916c0654488ac1464f21d72e9a 
>   src/sched/sched.cpp a1723f3cdd05289b417b4ea8bdd9b000655eccf8 
> 
> Diff: https://reviews.apache.org/r/37532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Vinod Kone


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

Ship it!


feel free to resolve the open issues.


src/tests/master_tests.cpp (line 3696)


you don't need this. just do

StartSlave(&slaveDetector1);



src/tests/master_tests.cpp (line 3731)


why the change in formatting? we allow 80 char comments now?


- Vinod Kone


On Sept. 17, 2015, 3:07 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 17, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
>   src/tests/master_tests.cpp 06d74c3 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 2:01 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Add note above extra flags.


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


Repository: mesos


Description
---

Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
flags.


Diffs (updated)
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 

Diff: https://reviews.apache.org/r/38030/diff/


Testing
---

No testing done until the last patch in the chain.


Thanks,

Joseph Wu

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu



> On Sept. 17, 2015, 12:17 p.m., Jie Yu wrote:
> > src/Makefile.am, line 111
> > 
> >
> > Have you guys looked at the code in the picojson?
> > https://github.com/kazuho/picojson/blob/master/picojson.h#L61
> > 
> > Looks like that macro should be defined. I don't understand why this 
> > fixed the issue.
> 
> Joseph Wu wrote:
> So the issue is that other libraries like glog and protobuf have an 
> `#include ` too.  But the flag isn't defined when those libraries 
> call it.
> Later on, when we include `picojson.h`, the fact that it defines the flag 
> doesn't do anything, since `inttypes.h` has already been included.
> 
> I added a note in the next review to explain why we have an #undef (top 
> of the diff).  Would a similar note be appropriate here?
> 
> Jie Yu wrote:
> Aha, got it. Wondering if that'll break glog/protobuf because they are 
> not expecting this Macro to be defined?
> 
> Definitely worth adding what you've just said to the code as a 
> comment/note.

I think it's unlikely that these extra macros will break anything.  It does add 
a bunch of items to the global namespace (See: 
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/inttypes.h.html).  But 
the naming is odd/idiosyncratic enough that it's probably not going to conflict 
with anything that follows the google style guide.


- Joseph


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


On Sept. 17, 2015, 2:01 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38030/
> ---
> 
> (Updated Sept. 17, 2015, 2:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
> flags.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
> 
> Diff: https://reviews.apache.org/r/38030/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38468]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 6:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 17, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Jie Yu



> On Sept. 17, 2015, 7:17 p.m., Jie Yu wrote:
> > src/Makefile.am, line 111
> > 
> >
> > Have you guys looked at the code in the picojson?
> > https://github.com/kazuho/picojson/blob/master/picojson.h#L61
> > 
> > Looks like that macro should be defined. I don't understand why this 
> > fixed the issue.
> 
> Joseph Wu wrote:
> So the issue is that other libraries like glog and protobuf have an 
> `#include ` too.  But the flag isn't defined when those libraries 
> call it.
> Later on, when we include `picojson.h`, the fact that it defines the flag 
> doesn't do anything, since `inttypes.h` has already been included.
> 
> I added a note in the next review to explain why we have an #undef (top 
> of the diff).  Would a similar note be appropriate here?

Aha, got it. Wondering if that'll break glog/protobuf because they are not 
expecting this Macro to be defined?

Definitely worth adding what you've just said to the code as a comment/note.


- Jie


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


On Sept. 17, 2015, 7:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38030/
> ---
> 
> (Updated Sept. 17, 2015, 7:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
> flags.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
> 
> Diff: https://reviews.apache.org/r/38030/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38455: CMake: Remove thread_tests.cpp from Stout tests.

2015-09-17 Thread Joseph Wu


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


Looks like this test doesn't exist anymore.  At the same time, these exist but 
are not in the list:

* proc_tests.cpp
* recordio_tests.cpp
* try_tests.cpp

- Joseph Wu


On Sept. 17, 2015, 3:02 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38455/
> ---
> 
> (Updated Sept. 17, 2015, 3:02 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Remove thread_tests.cpp from Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> cf5fd5c9840fb58b604dc0cd49b2922efbb2a10e 
> 
> Diff: https://reviews.apache.org/r/38455/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38077: [5/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38028, 38030, 38031, 38076, 38077]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 7:14 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38077/
> ---
> 
> (Updated Sept. 17, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Add TODO's for refactoring some JSON parsing in the docker code (See 
> MESOS-3409).  Update how the JSON::Number is used.
> * Tweak some tests to match changes to JSON::Number.
> * Address a TODO on one test, which used a workaround for double-precision 
> comparison.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp c4c37cb98ef50beeae0030ac472c7bf77e10fb5e 
>   src/slave/containerizer/provisioner/docker/token_manager.cpp 
> cf52626554f5bddcbbac4d515d7dd599d269dd57 
>   src/tests/fault_tolerance_tests.cpp 
> 061e0998239f7b69d989523e341881cd4abfcdeb 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/monitor_tests.cpp f4049553d9f2b0e6419d9e6f88d3b0a54184f88d 
>   src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
>   src/tests/slave_tests.cpp 4a1a586f645424e30a4e989d1fd2f0bc2993b188 
> 
> Diff: https://reviews.apache.org/r/38077/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX | clang-6.1.0, CentOS 7.1 | g++ 4.8.3)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Vinod Kone


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



docs/reporting-a-bug.md (line 11)


I would actually recommend that newbies assign a ticket *after* they find a 
shepherd. Reduces the noise of people assigning tickets and unassigning when 
they realize it's out of scope, hard, etc.



docs/submitting-a-patch.md (line 26)


or asking on the JIRA ticket itself.


- Vinod Kone


On Sept. 17, 2015, 6:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 17, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu



> On Sept. 17, 2015, 12:17 p.m., Jie Yu wrote:
> > src/Makefile.am, line 111
> > 
> >
> > Have you guys looked at the code in the picojson?
> > https://github.com/kazuho/picojson/blob/master/picojson.h#L61
> > 
> > Looks like that macro should be defined. I don't understand why this 
> > fixed the issue.

So the issue is that other libraries like glog and protobuf have an `#include 
` too.  But the flag isn't defined when those libraries call it.
Later on, when we include `picojson.h`, the fact that it defines the flag 
doesn't do anything, since `inttypes.h` has already been included.

I added a note in the next review to explain why we have an #undef (top of the 
diff).  Would a similar note be appropriate here?


- Joseph


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


On Sept. 17, 2015, 12:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38030/
> ---
> 
> (Updated Sept. 17, 2015, 12:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
> flags.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
> 
> Diff: https://reviews.apache.org/r/38030/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Joseph Wu

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

Just a few comments related to what I picked up from the sync.

docs/submitting-a-patch.md (line 17)

Can you also add a note about preferring "Accepted" issues rather than 
"Open" issues?

docs/submitting-a-patch.md (line 62)

Can you add a line here about adding the shepherd to the list of reviewers?

docs/submitting-a-patch.md (line 64)

This line should probably be amended, since we don't want the behavior of 
wait-until-someone-picks-up-this-review.

You can mention something about pinging (pushing?) the shepherd to do the 
review, rather than the other way around.

- Joseph Wu

On Sept. 17, 2015, 11:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 17, 2015, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Jie Yu


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



src/Makefile.am (line 111)


Have you guys looked at the code in the picojson?
https://github.com/kazuho/picojson/blob/master/picojson.h#L61

Looks like that macro should be defined. I don't understand why this fixed 
the issue.


- Jie Yu


On Sept. 17, 2015, 7:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38030/
> ---
> 
> (Updated Sept. 17, 2015, 7:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
> flags.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
> 
> Diff: https://reviews.apache.org/r/38030/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38077: [5/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 12:14 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Timothy Chen, and Vinod Kone.


Changes
---

Re-opening with fixes for g++ 4.8/4.9 on CentOS 7.  (No changes other than 
rebase)


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


Repository: mesos


Description
---

* Add TODO's for refactoring some JSON parsing in the docker code (See 
MESOS-3409).  Update how the JSON::Number is used.
* Tweak some tests to match changes to JSON::Number.
* Address a TODO on one test, which used a workaround for double-precision 
comparison.


Diffs (updated)
-

  src/docker/docker.cpp c4c37cb98ef50beeae0030ac472c7bf77e10fb5e 
  src/slave/containerizer/provisioner/docker/token_manager.cpp 
cf52626554f5bddcbbac4d515d7dd599d269dd57 
  src/tests/fault_tolerance_tests.cpp 061e0998239f7b69d989523e341881cd4abfcdeb 
  src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
  src/tests/monitor_tests.cpp f4049553d9f2b0e6419d9e6f88d3b0a54184f88d 
  src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
  src/tests/slave_tests.cpp 4a1a586f645424e30a4e989d1fd2f0bc2993b188 

Diff: https://reviews.apache.org/r/38077/diff/


Testing (updated)
---

`make check` (OSX | clang-6.1.0, CentOS 7.1 | g++ 4.8.3)


Thanks,

Joseph Wu

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 12:14 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Michael Park, and Vinod Kone.


Changes
---

Re-opening with fixes for g++ 4.8/4.9 on CentOS 7.


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


Repository: mesos


Description
---

* Changes JSON::Number to keep track of whether it is floating, signed 
integral, or unsigned integral.
* Changes how JSON::Number is stringified, to ensure that stringified doubles 
are parsed as JSON doubles.
* Added one test for integer precision between String <-> JSON <-> Protobuf 
conversions.
* Added one test for JSON::Number comparisons.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
115cc0cbec04db1bd9e068263f3931f33c266e86 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
850650c269e9be24c0f1ae81b8aa8725f8a0c151 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
87737dd1669ee77db2f29dacd8b5cebb0cabc0e6 

Diff: https://reviews.apache.org/r/38031/diff/


Testing
---

No testing done until the last patch in the chain.

However, this patch does breaks some libprocess and mesos tests (because 
JSON::Number is stored differently), which are fixed in the following two 
reviews.


Thanks,

Joseph Wu

Re: Review Request 38076: [4/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 12:14 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Re-opening with fixes for g++ 4.8/4.9 on CentOS 7.  (No changes other than 
rebase).


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


Repository: mesos


Description
---

Modify one libprocess test to match changes to JSON::Number.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/metrics_tests.cpp 
29ed0330a498579896bbef3349572dd35299a40a 

Diff: https://reviews.apache.org/r/38076/diff/


Testing
---

Testing is done in the next review.


Thanks,

Joseph Wu

Re: Review Request 38030: [2/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 12:13 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Re-opening with fixes for g++ 4.8/4.9 on CentOS 7.


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


Repository: mesos


Description (updated)
---

Add `PICOJSON_USE_INT64` and `__STDC_FORMAT_MACROS` flag to mesos compilation 
flags.


Diffs (updated)
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 

Diff: https://reviews.apache.org/r/38030/diff/


Testing
---

No testing done until the last patch in the chain.


Thanks,

Joseph Wu

Re: Review Request 38028: [1/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-17 Thread Joseph Wu


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

(Updated Sept. 17, 2015, 12:13 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Re-opening with fixes for g++ 4.8/4.9 on CentOS 7.


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


Repository: mesos


Description (updated)
---

Updates PicoJson to version 1.3.0 and adds `PICOJSON_USE_INT64` and 
`__STDC_FORMAT_MACROS` flags to the compilation flags.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
d13ba666740b4f2e382a0b1852724cfd519f8f64 
  3rdparty/libprocess/3rdparty/Makefile.am 
eb34251d24b1e5d1540151b59cf1062ca85aeb03 
  3rdparty/libprocess/3rdparty/picojson-1.3.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/picojson-4f93734.tar.gz 
f52dacd1c51ef5c533d174f63539a9b25e3d9180 
  3rdparty/libprocess/3rdparty/versions.am 
f44c7153166225279b973615ef0441c6f945da5b 
  3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699 

Diff: https://reviews.apache.org/r/38028/diff/


Testing
---

No testing done until the last patch in the chain.


Thanks,

Joseph Wu

Re: Review Request 38279: Enabled resources/attributes discovery

2015-09-17 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [38279]

Failed command: ./support/apply-review.sh -n -r 38279

Error:
 2015-09-17 19:08:55 URL:https://reviews.apache.org/r/38279/diff/raw/ 
[4094/4094] -> "38279.patch" [1]
error: patch failed: src/examples/test_hook_module.cpp:193
error: src/examples/test_hook_module.cpp: patch does not apply
error: patch failed: src/hook/manager.hpp:72
error: src/hook/manager.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 14, 2015, 5:39 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38279/
> ---
> 
> (Updated Sept. 14, 2015, 5:39 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First API draft for MESOS-3366.
> 
> 1) Only supports resources for now, we can add another hook for attributes 
> with a very similar code.
> 2) The callback currently receives the full SlaveInfo structure and construct 
> a new Resources object.
> 3) If there is multiple callbacks, each callback will see the changes made by 
> previous callbacks and are free to override or merge the detected resources 
> as they see fit.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp d90bacc 
>   src/examples/test_hook_module.cpp bc13a8a 
>   src/hook/manager.hpp 30d8321 
>   src/hook/manager.cpp 754c238 
>   src/slave/slave.cpp 5e5522e 
> 
> Diff: https://reviews.apache.org/r/38279/diff/
> 
> 
> Testing
> ---
> 
> make clean && make && make check
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-17 Thread Neil Conway


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

Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.


Repository: mesos


Description
---

Added suggestion that new contributors seek to find a shepherd for their work
before their begin writing code. Made a few other improvements.


Diffs
-

  docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
  docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 

Diff: https://reviews.apache.org/r/38468/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38253]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 5:41 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 17, 2015, 5:41 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 899d52f 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 93353a1 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38279: Enabled resources/attributes discovery

2015-09-17 Thread Niklas Nielsen



> On Sept. 14, 2015, 1:20 p.m., Niklas Nielsen wrote:
> > Per Connors comment, we need a test to exercise the new code

Ping - let's get this in soon :)


- Niklas


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


On Sept. 14, 2015, 10:39 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38279/
> ---
> 
> (Updated Sept. 14, 2015, 10:39 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First API draft for MESOS-3366.
> 
> 1) Only supports resources for now, we can add another hook for attributes 
> with a very similar code.
> 2) The callback currently receives the full SlaveInfo structure and construct 
> a new Resources object.
> 3) If there is multiple callbacks, each callback will see the changes made by 
> previous callbacks and are free to override or merge the detected resources 
> as they see fit.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp d90bacc 
>   src/examples/test_hook_module.cpp bc13a8a 
>   src/hook/manager.hpp 30d8321 
>   src/hook/manager.cpp 754c238 
>   src/slave/slave.cpp 5e5522e 
> 
> Diff: https://reviews.apache.org/r/38279/diff/
> 
> 
> Testing
> ---
> 
> make clean && make && make check
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38443]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 5:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-17 Thread Klaus Ma


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

(Updated Sept. 17, 2015, 5:41 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
---

Address comments


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


Repository: mesos


Description
---

We should ensure that we are addressing the container which the QoS controller 
intended to kill. Without this check, we may run into a scenario where the 
executor has terminated and one with the same id has started in the interim 
i.e. running in a different container than the one the QoS controller targeted.

This most likely requires us to add containerId to the ResourceUsage message 
and encode the containerID in the QoS Correction message.


Diffs (updated)
-

  include/mesos/mesos.proto 899d52f 
  include/mesos/slave/oversubscription.proto fa69a95 
  src/slave/slave.cpp 93353a1 
  src/tests/oversubscription_tests.cpp 0c5edaf 

Diff: https://reviews.apache.org/r/38253/diff/


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese


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

(Updated Sept. 17, 2015, 5:06 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Added guard for array sizes.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

Diff: https://reviews.apache.org/r/38443/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese



> On Sept. 17, 2015, 2:54 a.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/registry_client.cpp, line 499
> > 
> >
> > If history array contains non JSON Object from the response I think it 
> > will just crash the slave.
> > 
> > I realize in other places too, this becomes a bit tricky when docker 
> > registry changes their impl, or when user is targeting some other version 
> > of the registry I tihnk we shouldn't crash the whole slave.

if you mean the array sizes differences between history and fslayers then i 
have fixed the patch for that.


- Jojy


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


On Sept. 17, 2015, 5:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-17 Thread Joseph Wu

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

Note: I was hoping you would put changes in the docs and to the endpoints in 
*separate* reviews.

Before you go any further on this review, I would **strongly suggest** that you 
open up a design document for these ACLs.  Although the ACL you're proposing 
does accurately reflect the _current_ state of the maintenance endpoint, there 
is some background that you do not have.  We (the ones that wrote these 
endpoints) can guide you through this background more effectively on a design 
document.

docs/authentication.md (line 14)

Are you sure this is correct?  The ACL changes shouldn't change 
authentication, only authorization.

src/master/http.cpp (lines 1473 - 1504)

Can you refactor this into a helper function?  No reason to copy-paste it 
three times.

src/master/http.cpp (lines 1516 - 1518)

Note this pattern of continuation.  You'll want to use this instead of the 
`_continuation` pattern.

src/master/master.hpp (lines 934 - 947)

We strongly prefer the other pattern of continuation.  See the note above.

- Joseph Wu

On Sept. 17, 2015, 1:31 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38399/
> ---
> 
> (Updated Sept. 17, 2015, 1:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: mesos-
> https://issues.apache.org/jira/browse/mesos-
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ACLs for the maintenance HTTP endpoints
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38399/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread haosdent huang



> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote:
> > Thanks for fixing this, but I'm wondering how this fixes the issue.
> > (in fact, not even what is it testing).
> > 
> > If you invoke `cat` without input, it will just hang there waiting on 
> > STDIN, how does this make the test work?
> > (sorry for all the questions, but unfortunately I'm not familiar w/ this 
> > particular test)
> > 
> > Can you please add more information in the Description field?
> > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's 
> > related to, but what folks need to understand is: (a) what the problem was; 
> > (b) how it affected the test and (c) what you did to fix it.
> > 
> > Also, how does this preserve the spirit of the test? I can fix any test by 
> > just having ASSERT(true) :)
> > 
> > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test!
> 
> haosdent huang wrote:
> Hi @marco. "cat" is equals to "sleep forever" command(we conld not 
> execute sleep forver only could sleep xxx). I think this patch should fix the 
> problem according I repeat test this patch tonight. And the cause of this 
> issue has beed documented in jira by @qiujian. The old command sometimes have 
> two processes when running so this test case is flaky.

Ref: Bash: infinite sleep (infinite blocking) 
http://stackoverflow.com/questions/2935183/bash-infinite-sleep-infinite-blocking


- haosdent


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


On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread haosdent huang



> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote:
> > Thanks for fixing this, but I'm wondering how this fixes the issue.
> > (in fact, not even what is it testing).
> > 
> > If you invoke `cat` without input, it will just hang there waiting on 
> > STDIN, how does this make the test work?
> > (sorry for all the questions, but unfortunately I'm not familiar w/ this 
> > particular test)
> > 
> > Can you please add more information in the Description field?
> > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's 
> > related to, but what folks need to understand is: (a) what the problem was; 
> > (b) how it affected the test and (c) what you did to fix it.
> > 
> > Also, how does this preserve the spirit of the test? I can fix any test by 
> > just having ASSERT(true) :)
> > 
> > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test!

Hi @marco. "cat" is equals to "sleep forever" command(we conld not execute 
sleep forver only could sleep xxx). I think this patch should fix the problem 
according I repeat test this patch tonight. And the cause of this issue has 
beed documented in jira by @qiujian. The old command sometimes have two 
processes when running so this test case is flaky.


- haosdent


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


On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-17 Thread Alexander Rukletsov

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

I've commented on some issues. Several patterns I would like to ask you to 
correct everywhere, even where I didn't comment:

- same tense and mood;
- no shortcuts ("is not");
- avoid references to the built-in allocator implementation;
- focus on when the particular function is called and what the invariants are, 
say less about expectations, since implementation is up to the allocator.

include/mesos/master/allocator.hpp (line 67)

An allocator should not necessarily be a process. For actor-based 
allocators we have child interface `MesosAllocator`. I would suggest you 
rephrase that in a way, that successful initialization implies an allocator is 
ready to allocate, which includes setting up an actor process if necessary.

include/mesos/master/allocator.hpp (lines 70 - 71)

either present or future, e.g. "it determines how often..."

s/update/perform

include/mesos/master/allocator.hpp (lines 72 - 73)

Let's stick to one tense and mood, I would propose present indicative: "... 
the allocator uses to send..."

Allocator does not have the notion of offers (yet), its output is 
allocations.

include/mesos/master/allocator.hpp (lines 74 - 75)

Let's also document what happens if a framework uses a role unknown to the 
allocator.

include/mesos/master/allocator.hpp (lines 92 - 94)

Let's avoid conflating different usages of "used" here. Possible 
alternatives: leverage, rely on

include/mesos/master/allocator.hpp (line 104)

Let's avoid full forms in comments for interfaces: "it is", "should not" 
and so on.

include/mesos/master/allocator.hpp (lines 105 - 106)

s/the removed framework's resources/they

include/mesos/master/allocator.hpp (lines 115 - 116)

s/can only be/are

include/mesos/master/allocator.hpp (line 122)

s/Deactivate/Deactivates
here and below

include/mesos/master/allocator.hpp (lines 124 - 125)

s/will not be/are not

include/mesos/master/allocator.hpp (line 131)

s/Update/Updates

include/mesos/master/allocator.hpp (line 133)

Is it just capabilities that can be updated? I do remember several 
dicsussions and JIRA tickets around this topic. Do you think it makes sense to 
reference them here?

include/mesos/master/allocator.hpp (lines 137 - 139)

I would refrain from providing here examples about how real allocators 
work. The reason is that such comments are very difficult to maintain and they 
very often become misleading, which is even worse than non-existent.

include/mesos/master/allocator.hpp (line 150)

Let's use new terminology: agent instead of slave. Here and everywhere.

include/mesos/master/allocator.hpp (line 152)

s/will be/is

include/mesos/master/allocator.hpp (line 153)

s/old slave recovery/agent failover

include/mesos/master/allocator.hpp (lines 172 - 173)

I would phrase that in something like: "an allocator must consider all 
related resources unavailable and do not offer them to frameworks".

include/mesos/master/allocator.hpp (line 181)

s/Update/Updates

include/mesos/master/allocator.hpp (line 198)

Activates

include/mesos/master/allocator.hpp (line 200)

s/will be/is

- Alexander Rukletsov

On Sept. 8, 2015, 3:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> ---
> 
> (Updated Sept. 8, 2015, 3:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
> https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository:

Re: Review Request 38457: CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.

2015-09-17 Thread Marco Massenzio


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


I'm excited about this!
Tested on OSX and (at least, as far as stout_tests go, it works, yay!)

A quick question: why, instead of moving the files, we don't simply add a 
symlink that will "look right" to the linker etc.
(this may be an exceedingly dumb question, due to my extremely limited 
understanding of the build system)

Thanks, Alex, for doing this - can't wait to bid farewell to CDT for good.

- Marco Massenzio


On Sept. 17, 2015, 10:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38457/
> ---
> 
> (Updated Sept. 17, 2015, 10:03 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few third-party libraries (libev, gmock) do not have `make install`
> commands available, so below we have to add our own "install" commands.
> 
> The reason is: if we do not, we get runtime library load problems on OS
> X. In particular, `dydl` will look for these libraries at the prefix we
> passed to `configure` (or in `/usr/local` if we did not pass a prefix
> in), but since they don't have a `make install` step, they never get
> placed in the prefix folder.
> 
> Our solution is to:
>   (1) make a lib directory inside the Mesos folder for each of the
>   libraries that has no install step, and
>   (2) copy all such libraries into their respective directories.
> 
> (Note that step (1) is not only convenient, but important: make will add
> a `lib` to the end of your prefix path when linking, and since the built
> libraries end up in a `.libs` folder, it's not enough to simply pass the
> build directory into `configure` as a prefix; so if we're going to move
> the libraries, we might as well move them to a library folder.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> d13ba666740b4f2e382a0b1852724cfd519f8f64 
> 
> Diff: https://reviews.apache.org/r/38457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Marco Massenzio

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

Thanks for fixing this, but I'm wondering how this fixes the issue.
(in fact, not even what is it testing).

If you invoke `cat` without input, it will just hang there waiting on STDIN, 
how does this make the test work?
(sorry for all the questions, but unfortunately I'm not familiar w/ this 
particular test)

Can you please add more information in the Description field?
It's pretty obvious (or it should be :) ) that a patch fixes the bug it's 
related to, but what folks need to understand is: (a) what the problem was; (b) 
how it affected the test and (c) what you did to fix it.

Also, how does this preserve the spirit of the test? I can fix any test by just 
having ASSERT(true) :)

BTW - it's fantastic this fixed the issue and you did the repeat-cycle test!

- Marco Massenzio

On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38456: CMake: Fix MESOS-3395, MESOS-3394, add canonical third-party lib source.

2015-09-17 Thread Joseph Wu


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

Ship it!


Changes looks good.

General question: How do you envision what updates to dependencies will look 
like?
The sources (like glog, protobufs, picojson, etc) are currently part of the 
mesos code base.  This won't exactly work with the `mesos-3rdparty` source.
i.e. If you have have a library source like `dep-1.0.tar.gz` and want to 
upgrade to `dep-1.1.tar.gz`, it seems like the 3rdparty repo would need to have 
both?  (And wouldn't that possibly get unruly?)

- Joseph Wu


On Sept. 17, 2015, 3:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38456/
> ---
> 
> (Updated Sept. 17, 2015, 3:03 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the current CMake build system, if we choose to download the
> third-party dependencies (instead of un-tar'ing the rebundled versions
> of those dependencies) we will download them from the GitHub Mesos
> mirror.
> 
> It would be better to have a canonical source of third-party
> dependencies, independent of the Mesos changes.
> 
> This commit points at a new canonical source of third-party tarballs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> d13ba666740b4f2e382a0b1852724cfd519f8f64 
> 
> Diff: https://reviews.apache.org/r/38456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38003]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 3:07 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 17, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
>   src/tests/master_tests.cpp 06d74c3 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-17 Thread Klaus Ma


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

(Updated Sept. 17, 2015, 3:07 p.m.)


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


Changes
---

Address comments


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


Repository: mesos


Description
---

__Phenomenon:__
In some race condition, the slave was shutdown when after master failover.

__Root Cause:__
The slave was shutdown because of duplicated SlavID: in master, the SlaveID is 
genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
pid) maybe un-changed which lead to duplicated SlaveID. 

__Solution/Fix:__
Generate masterInfo.id by UUID instead of "date + ip + port + pid".


Diffs (updated)
-

  src/master/master.cpp 1c4e7af 
  src/tests/master_tests.cpp 06d74c3 

Diff: https://reviews.apache.org/r/38003/diff/


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38457: CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38456, 38457]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 10:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38457/
> ---
> 
> (Updated Sept. 17, 2015, 10:03 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few third-party libraries (libev, gmock) do not have `make install`
> commands available, so below we have to add our own "install" commands.
> 
> The reason is: if we do not, we get runtime library load problems on OS
> X. In particular, `dydl` will look for these libraries at the prefix we
> passed to `configure` (or in `/usr/local` if we did not pass a prefix
> in), but since they don't have a `make install` step, they never get
> placed in the prefix folder.
> 
> Our solution is to:
>   (1) make a lib directory inside the Mesos folder for each of the
>   libraries that has no install step, and
>   (2) copy all such libraries into their respective directories.
> 
> (Note that step (1) is not only convenient, but important: make will add
> a `lib` to the end of your prefix path when linking, and since the built
> libraries end up in a `.libs` folder, it's not enough to simply pass the
> build directory into `configure` as a prefix; so if we're going to move
> the libraries, we might as well move them to a library folder.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> d13ba666740b4f2e382a0b1852724cfd519f8f64 
> 
> Diff: https://reviews.apache.org/r/38457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-17 Thread Guangya Liu


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


I think that you also need some test cases for 
src/tests/authorization_tests.cpp.


docs/authentication.md (line 14)


There are four endpoints:
/maintenance/schedule
/maintenance/status
/machine/down
/machine/up

Is it possible to list them here?

I see that you did not handle the ACL for /maintenance/status, does this 
needed? Thanks.


- Guangya Liu


On Sept. 17, 2015, 8:31 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38399/
> ---
> 
> (Updated Sept. 17, 2015, 8:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: mesos-
> https://issues.apache.org/jira/browse/mesos-
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ACLs for the maintenance HTTP endpoints
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38399/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 38455: CMake: Remove thread_tests.cpp from Stout tests.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38455]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 10:02 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38455/
> ---
> 
> (Updated Sept. 17, 2015, 10:02 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Remove thread_tests.cpp from Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> cf5fd5c9840fb58b604dc0cd49b2922efbb2a10e 
> 
> Diff: https://reviews.apache.org/r/38455/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38454]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Review Request 38457: CMake: Fix MESOS-3250, a dynamic load in Stout tests on OS X.

2015-09-17 Thread Alex Clemmer


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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

A few third-party libraries (libev, gmock) do not have `make install`
commands available, so below we have to add our own "install" commands.

The reason is: if we do not, we get runtime library load problems on OS
X. In particular, `dydl` will look for these libraries at the prefix we
passed to `configure` (or in `/usr/local` if we did not pass a prefix
in), but since they don't have a `make install` step, they never get
placed in the prefix folder.

Our solution is to:
  (1) make a lib directory inside the Mesos folder for each of the
  libraries that has no install step, and
  (2) copy all such libraries into their respective directories.

(Note that step (1) is not only convenient, but important: make will add
a `lib` to the end of your prefix path when linking, and since the built
libraries end up in a `.libs` folder, it's not enough to simply pass the
build directory into `configure` as a prefix; so if we're going to move
the libraries, we might as well move them to a library folder.)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
d13ba666740b4f2e382a0b1852724cfd519f8f64 

Diff: https://reviews.apache.org/r/38457/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38399]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 8:31 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38399/
> ---
> 
> (Updated Sept. 17, 2015, 8:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: mesos-
> https://issues.apache.org/jira/browse/mesos-
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ACLs for the maintenance HTTP endpoints
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38399/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 38457: CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.

2015-09-17 Thread Alex Clemmer


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

(Updated Sept. 17, 2015, 10:03 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Summary (updated)
-

CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.


Repository: mesos


Description
---

A few third-party libraries (libev, gmock) do not have `make install`
commands available, so below we have to add our own "install" commands.

The reason is: if we do not, we get runtime library load problems on OS
X. In particular, `dydl` will look for these libraries at the prefix we
passed to `configure` (or in `/usr/local` if we did not pass a prefix
in), but since they don't have a `make install` step, they never get
placed in the prefix folder.

Our solution is to:
  (1) make a lib directory inside the Mesos folder for each of the
  libraries that has no install step, and
  (2) copy all such libraries into their respective directories.

(Note that step (1) is not only convenient, but important: make will add
a `lib` to the end of your prefix path when linking, and since the built
libraries end up in a `.libs` folder, it's not enough to simply pass the
build directory into `configure` as a prefix; so if we're going to move
the libraries, we might as well move them to a library folder.)


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
d13ba666740b4f2e382a0b1852724cfd519f8f64 

Diff: https://reviews.apache.org/r/38457/diff/


Testing
---


Thanks,

Alex Clemmer

Review Request 38456: CMake: Fix MESOS-3395, MESOS-3394, add canonical third-party lib source.

2015-09-17 Thread Alex Clemmer


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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

In the current CMake build system, if we choose to download the
third-party dependencies (instead of un-tar'ing the rebundled versions
of those dependencies) we will download them from the GitHub Mesos
mirror.

It would be better to have a canonical source of third-party
dependencies, independent of the Mesos changes.

This commit points at a new canonical source of third-party tarballs.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
d13ba666740b4f2e382a0b1852724cfd519f8f64 

Diff: https://reviews.apache.org/r/38456/diff/


Testing
---


Thanks,

Alex Clemmer

Review Request 38455: CMake: Remove thread_tests.cpp from Stout tests.

2015-09-17 Thread Alex Clemmer


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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Remove thread_tests.cpp from Stout tests.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
cf5fd5c9840fb58b604dc0cd49b2922efbb2a10e 

Diff: https://reviews.apache.org/r/38455/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Jian Qiu


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

(Updated Sept. 17, 2015, 9:55 a.m.)


Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.


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


Repository: mesos


Description
---

Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

Diff: https://reviews.apache.org/r/38454/diff/


Testing (updated)
---

./mesos-tests.sh 
--gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
--gtest_repeat=1000 --gtest_break_on_failure


Thanks,

Jian Qiu

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Jian Qiu



> On Sept. 17, 2015, 8:48 a.m., haosdent huang wrote:
> > Could you use 
> > 
> > ```
> > sudo ./mesos-tests.sh 
> > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
> > --gtest_repeat=1000 --gtest_break_on_failure
> > ```
> > 
> > to test it?

Done, thanks for reminding!


- Jian


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


On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 8:28 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Guangya Liu


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

Ship it!


Ship It!

- Guangya Liu


On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 8:28 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38452]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 8:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38452/
> ---
> 
> (Updated Sept. 17, 2015, 8:09 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3450
> https://issues.apache.org/jira/browse/MESOS-3450
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated C++ style guide for namespace usage
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md f1ed32aa845f7cfc85d002f4306a3c32e2eeb1d6 
> 
> Diff: https://reviews.apache.org/r/38452/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread haosdent huang


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

Ship it!


Could you use 

```
sudo ./mesos-tests.sh 
--gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" 
--gtest_repeat=1000 --gtest_break_on_failure
```

to test it?

- haosdent huang


On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38454/
> ---
> 
> (Updated Sept. 17, 2015, 8:28 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3293
> https://issues.apache.org/jira/browse/MESOS-3293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38454/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-17 Thread Zhiwei Chen


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

(Updated Sept. 17, 2015, 4:31 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: mesos-
https://issues.apache.org/jira/browse/mesos-


Repository: mesos


Description
---

Add ACLs for the maintenance HTTP endpoints


Diffs (updated)
-

  docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
  docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  src/authorizer/local/authorizer.hpp 32de102fd588f029882ef121ca83a7410c65 
  src/authorizer/local/authorizer.cpp 6d7da87731a438c2180cf91003e09d4aa5a1c773 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

Diff: https://reviews.apache.org/r/38399/diff/


Testing (updated)
---

make check


Thanks,

Zhiwei Chen

Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

2015-09-17 Thread Jian Qiu


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

Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone.


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


Repository: mesos


Description
---

Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

Diff: https://reviews.apache.org/r/38454/diff/


Testing
---

./mesos-tests.sh 
--gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids"


Thanks,

Jian Qiu

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-17 Thread Zhiwei Chen


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

(Updated Sept. 17, 2015, 4:25 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: mesos-
https://issues.apache.org/jira/browse/mesos-


Repository: mesos


Description
---

Add ACLs for the maintenance HTTP endpoints


Diffs (updated)
-

  docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
  docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  src/authorizer/local/authorizer.hpp 32de102fd588f029882ef121ca83a7410c65 
  src/authorizer/local/authorizer.cpp 6d7da87731a438c2180cf91003e09d4aa5a1c773 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

Diff: https://reviews.apache.org/r/38399/diff/


Testing
---


Thanks,

Zhiwei Chen

Review Request 38452: Updated C++ style guide for namespace usage

2015-09-17 Thread Guangya Liu


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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Updated C++ style guide for namespace usage


Diffs
-

  docs/mesos-c++-style-guide.md f1ed32aa845f7cfc85d002f4306a3c32e2eeb1d6 

Diff: https://reviews.apache.org/r/38452/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38447]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 6:22 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38447/
> ---
> 
> (Updated Sept. 17, 2015, 6:22 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3433
> https://issues.apache.org/jira/browse/MESOS-3433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented a TODO to clean up host mounts irrelevant to the container in the 
> container's mount namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> b6746345b70b462123c3254065e4f09a42f45e5b 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ca9f423d5f2f8f838c3371ee596f07537bb45210 
> 
> Diff: https://reviews.apache.org/r/38447/diff/
> 
> 
> Testing
> ---
> 
> Added a test that is not specific for this but observed successful cleanup of 
> the other container's mounts in logs.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-17 Thread Jie Yu


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

Ship it!



src/slave/containerizer/isolators/filesystem/linux.cpp (lines 363 - 366)


Thanks for fixing this!



src/slave/containerizer/isolators/filesystem/linux.cpp (line 409)


No need for this variable, right?

```
out << "xxx" << xxx
<< ...
```

looks more natural to me.



src/slave/containerizer/isolators/filesystem/linux.cpp (line 411)


single quote as well?


- Jie Yu


On Sept. 17, 2015, 6:22 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38447/
> ---
> 
> (Updated Sept. 17, 2015, 6:22 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3433
> https://issues.apache.org/jira/browse/MESOS-3433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented a TODO to clean up host mounts irrelevant to the container in the 
> container's mount namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 99f939f8b254cf71b4c1c7829b5fa37abdc2771b 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> b6746345b70b462123c3254065e4f09a42f45e5b 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ca9f423d5f2f8f838c3371ee596f07537bb45210 
> 
> Diff: https://reviews.apache.org/r/38447/diff/
> 
> 
> Testing
> ---
> 
> Added a test that is not specific for this but observed successful cleanup of 
> the other container's mounts in logs.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-17 Thread Jian Qiu


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

Ship it!


Ship It!

- Jian Qiu


On Sept. 16, 2015, 6:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38124/
> ---
> 
> (Updated Sept. 16, 2015, 6:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add V1 Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler/scheduler.proto 
> 0118b46afca8a5adecb0e65981dd44bb98333bab 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

95 matches

Mail list logo