date:20150805


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


Patch looks great!

Reviews applied: [37162]

All tests passed.

- Mesos ReviewBot


On Aug. 6, 2015, 5:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37162/
> ---
> 
> (Updated Aug. 6, 2015, 5:12 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3141
> https://issues.apache.org/jira/browse/MESOS-3141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 
> 
> Diff: https://reviews.apache.org/r/37162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36837: Update gmock to 1.7.0.


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

(Updated Aug. 6, 2015, 6:26 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
711809e808ebd0ed95d62270220e016ba6f41dca 
  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
  3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.


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

(Updated Aug. 5, 2015, 10:54 p.m.)


Review request for mesos, Benjamin Hindman and Michael Park.


Changes
---

Reverting refactor (see comments).


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer/memory_test_helper.cpp 
5e40b747f4266e7532baf8fd02ea5db0955124d2 

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


Testing
---

make check


Thanks,

Artem Harutyunyan

Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.



> On Aug. 3, 2015, 11:36 p.m., Michael Park wrote:
> > src/tests/containerizer/memory_test_helper.cpp, line 81
> > 
> >
> > What's the significance in the order in which these are called? I 
> > would've expected something like:
> > 
> > ```
> >   // Make sure that all pages that are going to be mapped into the
> >   // address space of this process become unevictable. This is needed
> >   // for testing cgroup oom killer.
> > #ifdef __APPLE__
> >   if (mlock(rss, size.bytes()) != 0)
> > #else
> >   if (mlockall(MCL_FUTURE) != 0)
> > #endif
> >   {
> > return ErrnoError("Failed to make pages to be mapped unevictable");
> >   }
> > ```
> 
> Artem Harutyunyan wrote:
> The mlockall(MCL_FUTURE) has to be called before the memory allocation is 
> made (because it affects future allocations), whereas mlock() is called for 
> already allocated pages. I could probably change mlockall(MCL_FUTURE) to 
> mlockall(MCL_CURRENT) in your snippet and make it work that way. I'll test to 
> verify and will update the patch accordingly. Thanks!
> 
> James Peach wrote:
> IMHO it would be better to add an autoconf check for ```mlockall``` 
> availability that to check ```__APPLE__```.
> 
> Michael Park wrote:
> @jpeach: Do you mean we should fail to configure if `mlockall` is not 
> available? If we did that, we wouldn't be able to compile on OS X though.
> 
> James Peach wrote:
> In autoconf, do ```AC_CHECK_FUNC(mlockall)```, then make this code 
> conditional on ```#if HAVE_MLOCKALL```.
> 
> Michael Park wrote:
> Ah I see, great! I agree that's better since there may be other platforms 
> aside from OS X that don't support `mlockall`. @hartem: what do you think?
> 
> Artem Harutyunyan wrote:
> I refactored following MPark's suggestion, however using 
> AC_CHECK_FUNC(mlockall) did not work, because the function is present on OSX, 
> it just prints the error message instead of actually locking memory pages.

Actually, I was too hopeful too quickly :-). I had to revert the 
mlockall(MCL_CURRENT) change becasue it broke the OOM killer test on Ubuntu.


- Artem


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


On Aug. 5, 2015, 10:14 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37065/
> ---
> 
> (Updated Aug. 5, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Michael Park.
> 
> 
> Bugs: MESOS-3197
> https://issues.apache.org/jira/browse/MESOS-3197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_test_helper.cpp 
> 5e40b747f4266e7532baf8fd02ea5db0955124d2 
> 
> Diff: https://reviews.apache.org/r/37065/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2

2015-08-05 Thread Benjamin Hindman

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

3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp (line 44)

s/cmd/command/

3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp (line 45)

s/cmd/command/
s/empyt/empty/

3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp (lines 55 - 
57)

A couple of comments:

(1) This is a good place for a 'std::initializer_list' to clean up the call 
sites, i.e.,

os::shell("echo", {"hello", "world"});

versus:

os::shell("echo", vector{"hello", "world"});

(2) I'm a little confounded by the 'std::vector' approach (even after 
replacing with 'std::initializer_list'). In particular, it seems like the value 
it adds is that it keeps a programmer from having to do 'strings::join(" ", 
args)' but they still have to stringify their arguments which makes me wonder 
why you wouldn't just use 'strings::format' outside of the call everywhere? Or 
why not keep the original "printf" version and call 'strings::format' 
internally instead of 'strings::join'? With the 'std::initializer_list' 
approach I'll imagine we'll see a mix of:

(A) os::shell("echo", {"hello", stringify(arg)});

or:

(B) os::shell("echo hello " + stringify(arg));

or:

(C) os::shell(strings::join(" ", "echo", "hello", stringify(arg)));

or:

(D) os::shell(strings::format("echo hello %s", arg));

The existing version (or an improved variadic template version) would have 
looked something like:

(E) os::shell("echo hello %s", arg);

I acknowledge that the 'std::initializer_list' version let's you add a 
third 'ignoreErrors' parameter (which you couldn't do if instead of 'args' you 
used variadic parameters). But, do we really need the 'ignoreErrors' parameter? 
Why not just ask people to append '|| exit 0' to their command just like we ask 
people to append '2>&1'? I like the idea of just giving people a conduit to the 
shell, and however they'd do stuff in the shell world they do here too.

Now, if we were _not_ using 'popen' under the covers or passing a string to 
'/bin/sh' then I would definitely see the value in a 'std::initializer_list' 
because then I wouldn't need to quote the command! But unfortunately we're 
going to have to force people to quote the command no matter what.

Thus, my suggestion is to ask people to append '|| exit 0' and then go with 
a variadic template implementation, i.e., (E), or if you want to be dead simple 
do (D) and then use 'strings::format' everywhere in the next review.

(Note that we use '%s' with our 'strings::format' for all types kind of 
like we use '{}' in Python string templates.)

3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp (line 31)

s/cmd/command/ (matches the declaration).

- Benjamin Hindman

On Aug. 5, 2015, 7:56 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36978/
> ---
> 
> (Updated Aug. 5, 2015, 7:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3142
> https://issues.apache.org/jira/browse/MESOS-3142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactoring os::shell.
> See MESOS-3142 for more details.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> ab767ccc4553cc5f61e4fe1b67110a9b5b32f2bc 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 53f14e1869ed7a6e1ac7cc8a82c558ed77907dc9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 4b7a7bafe1c64183d021b39cf12523250491f0ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 2556bd428cc8990659e30e804b9c96c1659ef4a1 
> 
> Diff: https://reviews.apache.org/r/36978/diff/
> 
> 
> Testing
> ---
> 
> make check
> *Note*: this patch by itself breaks mesos - this only fixes the `stout` part: 
> see also https://reviews.apache.org/r/36979/
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 37147: Timeout perf stat command if it does not complete.

2015-08-05 Thread Cong Wang


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


Why perf stat can timeout here? Since it just executes `sleep $DURATION`. 
Please state it in description

- Cong Wang


On Aug. 5, 2015, 11:25 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37147/
> ---
> 
> (Updated Aug. 5, 2015, 11:25 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Timeout perf stat command if it does not complete.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 5d4cb613cf41e52c605dc89dabe4c29cf8f54c95 
> 
> Diff: https://reviews.apache.org/r/37147/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37159: Delegated the container root filesystem provisioning to the filesystem isolator.

2015-08-05 Thread Timothy Chen


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



src/slave/containerizer/mesos/containerizer.cpp 


Where does the provisioner go?
I see it's removed here but I don't see where else we're calling 
provisioner to populate the rootfs?


- Timothy Chen


On Aug. 6, 2015, 4:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37159/
> ---
> 
> (Updated Aug. 6, 2015, 4:20 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Delegated the container root filesystem provisioning to the filesystem 
> isolator.
> 
> The motivation is that: currently, container rootfs provisioning is done by 
> the containerizer while preparing the rest of the filesystem (i.e., bind 
> mount volumes) is done by the filesystem isolator. It'll be more natural if 
> all filesystem related preparation is done by one component.
> 
> Another reason is that we are going to provision images specified in the 
> volumes as well. So provisining rootfs in filesystem isolator makes it more 
> easy to implement.
> 
> Turns out that this change simplify the containerizer quite a bit.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 22f1e3686f50c3b9290561aa7e5073e24a702824 
>   include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd 
>   src/slave/containerizer/isolator.hpp 
> 710c584f95d60c1931b40ca041409aa819a06cba 
>   src/slave/containerizer/isolator.cpp 
> ed610f9f8fe328fb3b73f620858dc632725e51f8 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 6b980f26fe8bb51dd989a0578337bc13dbd087ad 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 907d7e78bfb591197e150ac053bb857d15a1e6dc 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> e831878ab47b8455a4831ebe305373130b194a40 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> e343d0b9751b46bc5a4a8ccd32c0b2745e110e6b 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> 73f245bc9166e1f7550466ddd97113c63ce44e73 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 0e421cb6ad3e04b71746033ab15d0f1695fcd5e7 
>   src/slave/containerizer/isolators/filesystem/posix.hpp 
> 4c7a6f2b7530c88c34d533dba9593006ad5284b2 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> 4861ee13fc34eef03d28f26d57a7d11aebed81a6 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 45e4ba09993e7b77f2df45a5c86bc00fa2d83977 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> b30ab3fd0013045a2843fe1e8843cc120ce180c6 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 858e43683c88ac62abfc74ff28e8073895cf6f64 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 8e643f4afae8c24cd4d68aa349148b6f402b286b 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 2599c9800e3edf12ec883b31c280324b24b195c5 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 8244c345b84108af7fa18d20e71401d6e1a0aeb0 
>   src/slave/containerizer/isolators/posix.hpp 
> ef19749c0d5b795fee54d67cfc0d983b0f7084ec 
>   src/slave/containerizer/isolators/posix/disk.hpp 
> 9fa584ff4a2f3c90c4d81aecefbcba57fa2294ad 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> 6dda77bad7ab135b6d339a80b98a291ea7120e95 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
>   src/tests/containerizer/isolator.hpp 
> fa2fc9bd6a59de130870f1ab199e05e85579d8dd 
>   src/tests/containerizer/isolator_tests.cpp 
> ff6e2b7e190a58a4809d6e71addb15dabe418e17 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 213fa4b0b9c50eba941ef6b52497eb32d539 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 4bee74acba2b1472c80cabbc9d0384bd04c543aa 
> 
> Diff: https://reviews.apache.org/r/37159/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework


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

(Updated Aug. 6, 2015, 5:23 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Address review comments from Vinod. Still wonder why the distcheck failed 
earlier though !


Repository: mesos


Description
---

This implements the tests for http framework subscribe/failover/upgrade from a 
pid based framework. The test are parameterized on content type and hence test 
both json/protobuf responses.


Diffs (updated)
-

  src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework



> On Aug. 5, 2015, 11:52 p.m., Vinod Kone wrote:
> > src/tests/http_api_tests.cpp, line 145
> > 
> >
> > don't need to do this anymore now that the default framework info sets 
> > the user, right?

Yep, it got committed yesterday. Fixed :)


- Anand


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


On Aug. 5, 2015, midnight, Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37082/
> ---
> 
> (Updated Aug. 5, 2015, midnight)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements the tests for http framework subscribe/failover/upgrade from 
> a pid based framework. The test are parameterized on content type and hence 
> test both json/protobuf responses.
> 
> 
> Diffs
> -
> 
>   src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 
> 
> Diff: https://reviews.apache.org/r/37082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 36837: Update gmock to 1.7.0.


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


Bad patch!

Reviews applied: [36837]

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

Error:
 2015-08-06 05:20:52 URL:https://reviews.apache.org/r/36837/diff/raw/ 
[3271/3271] -> "36837.patch" [1]
error: missing binary patch data for 
'3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz'
error: binary patch does not apply to 
'3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz'
error: 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Aug. 6, 2015, 5:08 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36837/
> ---
> 
> (Updated Aug. 6, 2015, 5:08 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3141
> https://issues.apache.org/jira/browse/MESOS-3141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 711809e808ebd0ed95d62270220e016ba6f41dca 
>   3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
> d45d9894b0214f5f02a88f6da5c258327110efd8 
>   3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 97727537778511ca5a10be4f3c25cd21d919 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
>   3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a 
> 
> Diff: https://reviews.apache.org/r/36837/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.



> On Aug. 3, 2015, 11:36 p.m., Michael Park wrote:
> > src/tests/containerizer/memory_test_helper.cpp, line 81
> > 
> >
> > What's the significance in the order in which these are called? I 
> > would've expected something like:
> > 
> > ```
> >   // Make sure that all pages that are going to be mapped into the
> >   // address space of this process become unevictable. This is needed
> >   // for testing cgroup oom killer.
> > #ifdef __APPLE__
> >   if (mlock(rss, size.bytes()) != 0)
> > #else
> >   if (mlockall(MCL_FUTURE) != 0)
> > #endif
> >   {
> > return ErrnoError("Failed to make pages to be mapped unevictable");
> >   }
> > ```
> 
> Artem Harutyunyan wrote:
> The mlockall(MCL_FUTURE) has to be called before the memory allocation is 
> made (because it affects future allocations), whereas mlock() is called for 
> already allocated pages. I could probably change mlockall(MCL_FUTURE) to 
> mlockall(MCL_CURRENT) in your snippet and make it work that way. I'll test to 
> verify and will update the patch accordingly. Thanks!
> 
> James Peach wrote:
> IMHO it would be better to add an autoconf check for ```mlockall``` 
> availability that to check ```__APPLE__```.
> 
> Michael Park wrote:
> @jpeach: Do you mean we should fail to configure if `mlockall` is not 
> available? If we did that, we wouldn't be able to compile on OS X though.
> 
> James Peach wrote:
> In autoconf, do ```AC_CHECK_FUNC(mlockall)```, then make this code 
> conditional on ```#if HAVE_MLOCKALL```.
> 
> Michael Park wrote:
> Ah I see, great! I agree that's better since there may be other platforms 
> aside from OS X that don't support `mlockall`. @hartem: what do you think?

I refactored following MPark's suggestion, however using 
AC_CHECK_FUNC(mlockall) did not work, because the function is present on OSX, 
it just prints the error message instead of actually locking memory pages.


- Artem


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


On Aug. 5, 2015, 10:14 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37065/
> ---
> 
> (Updated Aug. 5, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Michael Park.
> 
> 
> Bugs: MESOS-3197
> https://issues.apache.org/jira/browse/MESOS-3197
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_test_helper.cpp 
> 5e40b747f4266e7532baf8fd02ea5db0955124d2 
> 
> Diff: https://reviews.apache.org/r/37065/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.


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

(Updated Aug. 5, 2015, 10:14 p.m.)


Review request for mesos, Benjamin Hindman and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer/memory_test_helper.cpp 
5e40b747f4266e7532baf8fd02ea5db0955124d2 

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


Testing
---

make check


Thanks,

Artem Harutyunyan

Re: Review Request 37160: Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


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

(Updated Aug. 6, 2015, 5:13 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
d69920689932d2afc8416d0f8ba289695444d8b2 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.


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

(Updated Aug. 6, 2015, 5:12 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36837: Update gmock to 1.7.0.


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

(Updated Aug. 6, 2015, 5:08 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
711809e808ebd0ed95d62270220e016ba6f41dca 
  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
  3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 36837: Update gmock to 1.7.0.


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

(Updated Aug. 6, 2015, 5:07 a.m.)


Review request for mesos and Michael Park.


Summary (updated)
-

Update gmock to 1.7.0.


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


Repository: mesos


Description
---

Update gmock to 1.7.0 .


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
711809e808ebd0ed95d62270220e016ba6f41dca 
  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
  3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 37159: Delegated the container root filesystem provisioning to the filesystem isolator.


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


Patch looks great!

Reviews applied: [36929, 36930, 36954, 36956, 37054, 37055, 37091, 37105, 
37142, 37159]

All tests passed.

- Mesos ReviewBot


On Aug. 6, 2015, 4:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37159/
> ---
> 
> (Updated Aug. 6, 2015, 4:20 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Delegated the container root filesystem provisioning to the filesystem 
> isolator.
> 
> The motivation is that: currently, container rootfs provisioning is done by 
> the containerizer while preparing the rest of the filesystem (i.e., bind 
> mount volumes) is done by the filesystem isolator. It'll be more natural if 
> all filesystem related preparation is done by one component.
> 
> Another reason is that we are going to provision images specified in the 
> volumes as well. So provisining rootfs in filesystem isolator makes it more 
> easy to implement.
> 
> Turns out that this change simplify the containerizer quite a bit.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 22f1e3686f50c3b9290561aa7e5073e24a702824 
>   include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd 
>   src/slave/containerizer/isolator.hpp 
> 710c584f95d60c1931b40ca041409aa819a06cba 
>   src/slave/containerizer/isolator.cpp 
> ed610f9f8fe328fb3b73f620858dc632725e51f8 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 6b980f26fe8bb51dd989a0578337bc13dbd087ad 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 907d7e78bfb591197e150ac053bb857d15a1e6dc 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> e831878ab47b8455a4831ebe305373130b194a40 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> e343d0b9751b46bc5a4a8ccd32c0b2745e110e6b 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> 73f245bc9166e1f7550466ddd97113c63ce44e73 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 0e421cb6ad3e04b71746033ab15d0f1695fcd5e7 
>   src/slave/containerizer/isolators/filesystem/posix.hpp 
> 4c7a6f2b7530c88c34d533dba9593006ad5284b2 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> 4861ee13fc34eef03d28f26d57a7d11aebed81a6 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 45e4ba09993e7b77f2df45a5c86bc00fa2d83977 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> b30ab3fd0013045a2843fe1e8843cc120ce180c6 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 858e43683c88ac62abfc74ff28e8073895cf6f64 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 8e643f4afae8c24cd4d68aa349148b6f402b286b 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 2599c9800e3edf12ec883b31c280324b24b195c5 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 8244c345b84108af7fa18d20e71401d6e1a0aeb0 
>   src/slave/containerizer/isolators/posix.hpp 
> ef19749c0d5b795fee54d67cfc0d983b0f7084ec 
>   src/slave/containerizer/isolators/posix/disk.hpp 
> 9fa584ff4a2f3c90c4d81aecefbcba57fa2294ad 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> 6dda77bad7ab135b6d339a80b98a291ea7120e95 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
>   src/tests/containerizer/isolator.hpp 
> fa2fc9bd6a59de130870f1ab199e05e85579d8dd 
>   src/tests/containerizer/isolator_tests.cpp 
> ff6e2b7e190a58a4809d6e71addb15dabe418e17 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 213fa4b0b9c50eba941ef6b52497eb32d539 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 4bee74acba2b1472c80cabbc9d0384bd04c543aa 
> 
> Diff: https://reviews.apache.org/r/37159/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35874: Added template parameters and constructors to hashset to match the signature of hashmap


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

Ship it!


I'll get this committed for you with the minor changes below.


3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 62 - 66)


Let's omit the "Apparently a bug in the ..." part of the comment.
```
// An implementation based on the move constructor of 'hashmap'
// fails to compile on all major compilers except gcc 5.1 and up.
// See http://stackoverflow.com/q/31051466/118750?sem=2.
```



3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 67 - 71)


Let's keep this consistent with the patterns in the other constructors.
```
boost::unordered_set::reserve(set.size());

for (auto iterator = set.begin(); iterator != set.end(); ++iterator) {
  boost::unordered_set::emplace(std::move(*iterator));
}
```



3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 120 - 121)


2 indents after assignment operator.


- Michael Park


On July 7, 2015, 8:51 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35874/
> ---
> 
> (Updated July 7, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Michael Park, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-2924
> https://issues.apache.org/jira/browse/MESOS-2924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds extra template parameters to hashset as well as implicit constructors 
> from
> `std::set` and a initializer list constructor.
> 
> These changes keep hashset up to date with the changes in hashmap.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp 
> 75ed9db54dc9ab502e978f06c55a621cacb56b91 
>   3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp 
> 3c4b732432c0c155451d34ecd5f985318d118fe5 
> 
> Diff: https://reviews.apache.org/r/35874/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37160: Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


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

(Updated Aug. 6, 2015, 4:23 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
d69920689932d2afc8416d0f8ba289695444d8b2 

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


Testing
---


Thanks,

haosdent huang

Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.


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

Review request for mesos.


Repository: mesos


Description
---

Add GTEST_LANG_CXX11 to configure.ac when compile.


Diffs
-

  configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.


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

(Updated Aug. 6, 2015, 4:23 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Add GTEST_LANG_CXX11 to configure.ac when compile.


Diffs
-

  configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 

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


Testing
---


Thanks,

haosdent huang

Review Request 37160: Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


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

Review request for mesos.


Repository: mesos


Description
---

Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
d69920689932d2afc8416d0f8ba289695444d8b2 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37160: Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


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

(Updated Aug. 6, 2015, 4:22 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
d69920689932d2afc8416d0f8ba289695444d8b2 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36837: Update gmock to 1.7.0 .


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

(Updated Aug. 6, 2015, 4:22 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Update gmock to 1.7.0 .


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
711809e808ebd0ed95d62270220e016ba6f41dca 
  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
  3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a 

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


Testing
---

make check


Thanks,

haosdent huang

Review Request 37159: Delegated the container root filesystem provisioning to the filesystem isolator.


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

Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Delegated the container root filesystem provisioning to the filesystem isolator.

The motivation is that: currently, container rootfs provisioning is done by the 
containerizer while preparing the rest of the filesystem (i.e., bind mount 
volumes) is done by the filesystem isolator. It'll be more natural if all 
filesystem related preparation is done by one component.

Another reason is that we are going to provision images specified in the 
volumes as well. So provisining rootfs in filesystem isolator makes it more 
easy to implement.

Turns out that this change simplify the containerizer quite a bit.


Diffs
-

  include/mesos/slave/isolator.hpp 22f1e3686f50c3b9290561aa7e5073e24a702824 
  include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd 
  src/slave/containerizer/isolator.hpp 710c584f95d60c1931b40ca041409aa819a06cba 
  src/slave/containerizer/isolator.cpp ed610f9f8fe328fb3b73f620858dc632725e51f8 
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
6b980f26fe8bb51dd989a0578337bc13dbd087ad 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
907d7e78bfb591197e150ac053bb857d15a1e6dc 
  src/slave/containerizer/isolators/cgroups/mem.hpp 
e831878ab47b8455a4831ebe305373130b194a40 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
e343d0b9751b46bc5a4a8ccd32c0b2745e110e6b 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
73f245bc9166e1f7550466ddd97113c63ce44e73 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
0e421cb6ad3e04b71746033ab15d0f1695fcd5e7 
  src/slave/containerizer/isolators/filesystem/posix.hpp 
4c7a6f2b7530c88c34d533dba9593006ad5284b2 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
4861ee13fc34eef03d28f26d57a7d11aebed81a6 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
45e4ba09993e7b77f2df45a5c86bc00fa2d83977 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
b30ab3fd0013045a2843fe1e8843cc120ce180c6 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
858e43683c88ac62abfc74ff28e8073895cf6f64 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
8e643f4afae8c24cd4d68aa349148b6f402b286b 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
2599c9800e3edf12ec883b31c280324b24b195c5 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
8244c345b84108af7fa18d20e71401d6e1a0aeb0 
  src/slave/containerizer/isolators/posix.hpp 
ef19749c0d5b795fee54d67cfc0d983b0f7084ec 
  src/slave/containerizer/isolators/posix/disk.hpp 
9fa584ff4a2f3c90c4d81aecefbcba57fa2294ad 
  src/slave/containerizer/isolators/posix/disk.cpp 
6dda77bad7ab135b6d339a80b98a291ea7120e95 
  src/slave/containerizer/mesos/containerizer.hpp 
8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
  src/slave/containerizer/mesos/containerizer.cpp 
6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
  src/tests/containerizer/isolator.hpp fa2fc9bd6a59de130870f1ab199e05e85579d8dd 
  src/tests/containerizer/isolator_tests.cpp 
ff6e2b7e190a58a4809d6e71addb15dabe418e17 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
213fa4b0b9c50eba941ef6b52497eb32d539 
  src/tests/containerizer/port_mapping_tests.cpp 
4bee74acba2b1472c80cabbc9d0384bd04c543aa 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 36837: Update gmock to 1.7.0 .



> On Aug. 6, 2015, 3:27 a.m., Michael Park wrote:
> > I'm also getting the following error when I run `distcheck`. Do you happen 
> > to know why?
> > 
> > ```
> > ERROR: files left in build directory after distclean:
> > ./3rdparty/libprocess/3rdparty/._gmock-1.7.0
> > make[1]: *** [distcleancheck] Error 1
> > make[1]: Leaving directory `/mesos/mesos-0.24.0/_build'
> > make: *** [distcheck] Error 1
> > ```

Oh, let me check


- haosdent


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


On Aug. 6, 2015, 4:14 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36837/
> ---
> 
> (Updated Aug. 6, 2015, 4:14 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3141
> https://issues.apache.org/jira/browse/MESOS-3141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update gmock to 1.7.0 .
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
> d45d9894b0214f5f02a88f6da5c258327110efd8 
>   3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 97727537778511ca5a10be4f3c25cd21d919 
> 
> Diff: https://reviews.apache.org/r/36837/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36837: Update gmock to 1.7.0 .


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

(Updated Aug. 6, 2015, 4:14 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Update gmock to 1.7.0 .


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 

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


Testing
---

make check


Thanks,

haosdent huang

Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks


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

(Updated Aug. 6, 2015, 4:08 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added a continuation function for failoverFramework(...) that is common to both 
http and pid frameworks. Made the readerClosed callback as part of addFramework 
similar to link(...)


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


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

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


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar

Re: Review Request 36837: Update gmock to 1.7.0 .


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


I'm also getting the following error when I run `distcheck`. Do you happen to 
know why?

```
ERROR: files left in build directory after distclean:
./3rdparty/libprocess/3rdparty/._gmock-1.7.0
make[1]: *** [distcleancheck] Error 1
make[1]: Leaving directory `/mesos/mesos-0.24.0/_build'
make: *** [distcheck] Error 1
```

- Michael Park


On Aug. 5, 2015, 9:24 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36837/
> ---
> 
> (Updated Aug. 5, 2015, 9:24 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3141
> https://issues.apache.org/jira/browse/MESOS-3141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update gmock to 1.7.0 .
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
> d45d9894b0214f5f02a88f6da5c258327110efd8 
>   3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 97727537778511ca5a10be4f3c25cd21d919 
> 
> Diff: https://reviews.apache.org/r/36837/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36404: Added support for peek() to process::io


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

(Updated Aug. 5, 2015, 8:26 p.m.)


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


Changes
---

Added missing JIRA ticket.


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


Repository: mesos


Description
---

JIRA: https://issues.apache.org/jira/browse/MESOS-2964


Diffs
-

  3rdparty/libprocess/include/process/io.hpp 
975923f40f82357f31b89428f24d01df6a8ac9fc 
  3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 
  3rdparty/libprocess/src/tests/io_tests.cpp 
c642bab9e2845668767ad237985cb9ce1109 

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


Testing
---

- Added a test case for process::io::peek
- make check


Thanks,

Artem Harutyunyan

Re: Review Request 36837: Update gmock to 1.7.0 .


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


There are also `TODO` comments from @dhamon regarding `gmock-1.7.0` and passing 
`-DGTEST_LANG_CXX11`.
Are they still relevant?

- Michael Park


On Aug. 5, 2015, 9:24 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36837/
> ---
> 
> (Updated Aug. 5, 2015, 9:24 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3141
> https://issues.apache.org/jira/browse/MESOS-3141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update gmock to 1.7.0 .
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
> d45d9894b0214f5f02a88f6da5c258327110efd8 
>   3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 97727537778511ca5a10be4f3c25cd21d919 
> 
> Diff: https://reviews.apache.org/r/36837/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37147: Timeout perf stat command if it does not complete.


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


Bad patch!

Reviews applied: [37147]

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

Error:
 2015-08-06 00:09:00 URL:https://reviews.apache.org/r/37147/diff/raw/ 
[1484/1484] -> "37147.patch" [1]
error: patch failed: src/linux/perf.cpp:296
error: src/linux/perf.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Aug. 5, 2015, 11:25 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37147/
> ---
> 
> (Updated Aug. 5, 2015, 11:25 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Timeout perf stat command if it does not complete.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 5d4cb613cf41e52c605dc89dabe4c29cf8f54c95 
> 
> Diff: https://reviews.apache.org/r/37147/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework

2015-08-05 Thread Vinod Kone


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


Tests look good! Mainly stylistic issues and generous use of 'auto'. Please 
make sure you fix all the tests when you fix a given issue (I might not have 
caught all of them in all the tests).


src/tests/http_api_tests.cpp (line 69)


I would have this return a Try



src/tests/http_api_tests.cpp (line 73)


what if this fails? 

see above: this should return an Error.



src/tests/http_api_tests.cpp (line 79)


what if this fails? just return parse here.



src/tests/http_api_tests.cpp (line 91)


2 blank lines between outer elements.



src/tests/http_api_tests.cpp (line 97)


2 blank lines.



src/tests/http_api_tests.cpp (line 126)


2 blank lines.



src/tests/http_api_tests.cpp (line 127)


s/client/scheduler/



src/tests/http_api_tests.cpp (lines 135 - 136)


Put this line close to where it is used, above #150.



src/tests/http_api_tests.cpp (line 145)


don't need to do this anymore now that the default framework info sets the 
user, right?



src/tests/http_api_tests.cpp (line 164)


why auto? it's not clear at all what the return type is here.



src/tests/http_api_tests.cpp (line 174)


ditto. no auto please.



src/tests/http_api_tests.cpp (lines 185 - 186)


This comment and test name is misleading because this is not testing 
failover. This test is just testing subscription retry (simulating the 
situation of a ZK blip).

Mind fixing it?



src/tests/http_api_tests.cpp (line 192)


ditto.



src/tests/http_api_tests.cpp (line 201)


ditto.



src/tests/http_api_tests.cpp (line 219)


ditto.



src/tests/http_api_tests.cpp (line 229)


ditto.



src/tests/http_api_tests.cpp (line 238)


new line here.



src/tests/http_api_tests.cpp (line 244)


ditto.



src/tests/http_api_tests.cpp (line 251)


ditto.



src/tests/http_api_tests.cpp (line 258)


this is not a failover. please rephrase.



src/tests/http_api_tests.cpp (lines 265 - 266)


EXPECT_EQ(arg1,
  arg2);



src/tests/http_api_tests.cpp (line 273)


s/http/HTTP/



src/tests/http_api_tests.cpp (line 281)


ditto.



src/tests/http_api_tests.cpp (line 285)


ditto.



src/tests/http_api_tests.cpp (line 299)


s/http/HTTP/



src/tests/http_api_tests.cpp (line 311)


s/http/HTTP/



src/tests/http_api_tests.cpp (lines 317 - 318)


reorder.



src/tests/http_api_tests.cpp (lines 351 - 353)


you don't need to pull out value. we have equality operator defined for 
FrameworkID.

also alignment.



src/tests/http_api_tests.cpp (line 362)


s/pid/PID/
s/http/HTTP/



src/tests/http_api_tests.cpp (line 365)



s/UpdatePidToHttpSchedulerForceNotSetFailure/UpdatePidToHttpSchedulerWithoutForce/



src/tests/http_api_tests.cpp (line 376)


ditto.



src/tests/http_api_tests.cpp (line 395)


s/http/HTTP/

s/framework/framework without setting 'force' field/



src/tests/http_api_tests.cpp (line 419)


ditto.



src/tests/http_api_tests.cpp (line 429)

Re: Review Request 37142: Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.



> On Aug. 5, 2015, 11:45 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 480
> > 
> >
> > This means we no longer support default container info? I don't see 
> > this being used anywhere else then now?

It's still supported. This is in the recovery path (meaning that the container 
has already been launched, and volumes have already been mounted). We still 
apply the default container info in the launch path.


- Jie


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


On Aug. 5, 2015, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37142/
> ---
> 
> (Updated Aug. 5, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.
> 
> This is no longer needed since we are going to call 
> provisioner->destory(containerId) for every provisioners. See r37105 for more 
> details.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
> 
> Diff: https://reviews.apache.org/r/37142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 37142: Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.

2015-08-05 Thread Timothy Chen


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



src/slave/containerizer/mesos/containerizer.cpp 


This means we no longer support default container info? I don't see this 
being used anywhere else then now?


- Timothy Chen


On Aug. 5, 2015, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37142/
> ---
> 
> (Updated Aug. 5, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.
> 
> This is no longer needed since we are going to call 
> provisioner->destory(containerId) for every provisioners. See r37105 for more 
> details.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
> 
> Diff: https://reviews.apache.org/r/37142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 37055: Added Image to Volume as one of the sources.

2015-08-05 Thread Timothy Chen


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

Ship it!


Ship It!


include/mesos/mesos.proto (line 1247)


Should we add some commets around the Image?
Perhaps something like:

// Image that will be used to provision a new root filesystem by the Image 
provisioner.


- Timothy Chen


On Aug. 3, 2015, 11:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37055/
> ---
> 
> (Updated Aug. 3, 2015, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Image to Volume as one of the sources.
> 
> See the motivation here: 
> https://docs.google.com/document/d/1n2emC2ruTMur5nURvLgGYJxuP-tgBwgLDrmg_17QSmA/edit#
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba 
> 
> Diff: https://reviews.apache.org/r/37055/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 37147: Timeout perf stat command if it does not complete.

2015-08-05 Thread Paul Brett


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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Timeout perf stat command if it does not complete.


Diffs
-

  src/linux/perf.cpp 5d4cb613cf41e52c605dc89dabe4c29cf8f54c95 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37142: Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.


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


Patch looks great!

Reviews applied: [36929, 36930, 36954, 36956, 37054, 37055, 37091, 37105, 37142]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37142/
> ---
> 
> (Updated Aug. 5, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.
> 
> This is no longer needed since we are going to call 
> provisioner->destory(containerId) for every provisioners. See r37105 for more 
> details.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
> 
> Diff: https://reviews.apache.org/r/37142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks


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

(Updated Aug. 5, 2015, 10:37 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added a new HttpConnection struct instead of optimistically creating framework 
objects and deleting them for re-registration.


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


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

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


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar

Re: Review Request 36402: Adding 'Accept' header in request


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


Patch looks great!

Reviews applied: [37097, 36402]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 7:11 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36402/
> ---
> 
> (Updated Aug. 5, 2015, 7:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and 
> Vinod Kone.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding a method for Accept header in request + refactor of Accept-Encoding
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp b8d9300 
>   3rdparty/libprocess/src/http.cpp 4dcbd74 
>   3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 
> 
> Diff: https://reviews.apache.org/r/36402/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 37142: Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.

2015-08-05 Thread Guangya Liu


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

Ship it!


LGTM

- Guangya Liu


On 八月 5, 2015, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37142/
> ---
> 
> (Updated 八月 5, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.
> 
> This is no longer needed since we are going to call 
> provisioner->destory(containerId) for every provisioners. See r37105 for more 
> details.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
> 
> Diff: https://reviews.apache.org/r/37142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 36837: Update gmock to 1.7.0 .


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

(Updated Aug. 5, 2015, 9:24 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Update gmock to 1.7.0 .


Diffs
-

  3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz 
d45d9894b0214f5f02a88f6da5c258327110efd8 
  3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
97727537778511ca5a10be4f3c25cd21d919 

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


Testing
---

make check


Thanks,

haosdent huang

Review Request 37142: Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.


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

Review request for mesos, Timothy Chen, Vinod Kone, and Jiang Yan Xu.


Repository: mesos


Description
---

Removed the unneeded ExecutorInfo from Container struct in MesosContainerizer.

This is no longer needed since we are going to call 
provisioner->destory(containerId) for every provisioners. See r37105 for more 
details.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
  src/slave/containerizer/mesos/containerizer.cpp 
6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 37105: Removed the code of checkpointing container root filesystem path.


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

(Updated Aug. 5, 2015, 9:16 p.m.)


Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan 
Xu.


Changes
---

Removed rootfs from ContainerState proto as well.


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


Repository: mesos


Description
---

Removed the code of checkpointing container root filesystem path.

See ticket for the motivation.


Diffs (updated)
-

  include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd 
  src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a 
  src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a 
  src/slave/containerizer/mesos/containerizer.hpp 
8851d30af56b4f9fb95450ac1f42ab550e3df9ff 
  src/slave/containerizer/mesos/containerizer.cpp 
6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 
  src/slave/containerizer/provisioner.hpp 
cb4d511e8189b65df9b9803f23812dd98edc44ac 
  src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 
  src/slave/state.hpp cecf200e6e79172af57ae195a73a5161be7e604a 
  src/slave/state.cpp b9f2d8a0d6395b92bd50f1e0927f3ae9fd04b81c 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 37072: Added test for allocator update on scheduler failover


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


Patch looks great!

Reviews applied: [37072]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 6:51 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37072/
> ---
> 
> (Updated Aug. 5, 2015, 6:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2880
> https://issues.apache.org/jira/browse/MESOS-2880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for allocator update on scheduler failover
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> c7a2dacb600d7703de6090e7e47f453a3d08b53a 
> 
> Diff: https://reviews.apache.org/r/37072/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 37133: Add a frameworks parameter to the hierarchical allocator benchmark.


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


Patch looks great!

Reviews applied: [37133]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 5:34 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37133/
> ---
> 
> (Updated Aug. 5, 2015, 5:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3209
> https://issues.apache.org/jira/browse/MESOS-3209
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Parameterize the hierarchical allocator benchmark by the framework
> count as well as the slave count. This can be used to explore
> allocation behavior as the number of frameworks increases.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> c92d47aa0a06088f1f8fe749a629c27bfadd3c6d 
> 
> Diff: https://reviews.apache.org/r/37133/diff/
> 
> 
> Testing
> ---
> 
> make check
> make bench
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 37045: Convert Linux perf sampler to use process:await().

2015-08-05 Thread Ben Mahler

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

Ship it!

Will get this committed now, thanks! I've made some comments so that you can 
see what I've changed before committing.

src/linux/perf.cpp (lines 294 - 297)

This is where case 3 of wrapping is ok in the style guide.

src/linux/perf.cpp (line 300)

Looks like this isn't indented right? Seems to be even less indentation 
than the body of `sample`.. also note from the style guide that the brace goes 
on the line above.

src/linux/perf.cpp (lines 301 - 325)

I'd suggest wrapping these cases all together with an Option and 
doing the promise failure / termination once if error.isSome, e.g.:

```
Option error;

if (...) {
  error = ...;
} else if (...) {
  error = ...;
} else if (...) {
  error = ...;
}

if (error.isSome()) {
  promise.fail(error.get().message);
  terminate(self());
  return;
}
```

src/linux/perf.cpp (line 322)

Whoops, don't you want the same discarded logic here? Otherwise, failure() 
would crash.

- Ben Mahler

On Aug. 5, 2015, 6:23 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37045/
> ---
> 
> (Updated Aug. 5, 2015, 6:23 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Linux perf sampler to use process:await().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 
> 
> Diff: https://reviews.apache.org/r/37045/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 36814: Fill executor_id in state.json when task is run in CommandExecutor.

2015-08-05 Thread Ben Mahler



> On Aug. 4, 2015, 3:21 a.m., Adam B wrote:
> > LGTM. I like the idea of not setting the executorId=taskId in the actual 
> > TaskInfo/Task, since that could confuse other logic downstream that expects 
> > the executor/executorId to be empty for command executors. However, since 
> > this is exposed in state.json, there might be external tools/scripts that 
> > expect the same. Now they can just check if executorId==taskId, assuming 
> > there are no custom executors using the same naming scheme. Could you 
> > please add a note to docs/upgrades.md mentioning the API change, in case 
> > anybody does depend on the old behavior?
> > Does this actually solve the problem of: "The webui is broken for command 
> > executors because the master does not know the executor ID for the tasks 
> > using a command executor."?
> > @bmahler do you have any further thoughts/questions on this simple change?

Can we at least add comment in the slave code where we set the executor id for 
command executor, that the master is now assuming the slave uses the task id as 
the executor id?


- Ben


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


On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36814/
> ---
> 
> (Updated Aug. 4, 2015, 9:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Michael Park.
> 
> 
> Bugs: MESOS-527
> https://issues.apache.org/jira/browse/MESOS-527
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fill executor_id in state.json when task is run in CommandExecutor.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
>   src/tests/common/http_tests.cpp 38d062b2b4062e0a2fc912bad0cc2d73339eb66e 
> 
> Diff: https://reviews.apache.org/r/36814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.


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

(Updated Aug. 5, 2015, 7:12 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
Van Remoortere, and Vinod Kone.


Changes
---

Removed `Nothing`.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 
3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.



> On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
> > src/master/http.cpp, line 573
> > 
> >
> > What is 'Nothing' here?
> 
> Michael Park wrote:
> The `Nothing` here comes from the result of `master->apply` which returns 
> a `Future`. But I feel like you're not actually asking for an answer 
> here?
> 
> What would you like to see?
> 
> What I have currently is a comment above the code which reads:
> 
> ```cpp
> // Propogate the 'Future' as 'Future' where
> // 'Nothing' -> 'OK' and Failed -> 'Conflict'.
> ```
> 
> Jie Yu wrote:
> I mean if we remove 'Nothing' here, will it compile?

Oh, it does. I forgot about this behavior. Thanks, updated.


- Michael


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


On Aug. 5, 2015, 7:12 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated Aug. 5, 2015, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's
>   `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
> default `refuse_sec` of 5 seconds,
>   rather than `recoverResources(..., None())` so that we can virtually 
> always win the race against `allocate`.
>   In the rare case that we do lose, no disaster occurs. We simply fail to 
> satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> semi-optimistic and forward the
>   request to the allocator since there may be available resources to 
> satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `Conflict`.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
>   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

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

(Updated Aug. 5, 2015, 7:12 p.m.)

Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris
Van Remoortere, and Vinod Kone.

Changes
---

Removed `Nothing`

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

Repository: mesos

Description (updated)
---

This involved a lot more challenges than I anticipated, I've captured the
various approaches and limitations and deal-breakers of those approaches here:
[Master Endpoint Implementation
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
(1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
(2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
(3) `updateAvailable` never leaves the allocator in an over-allocated state
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like
"available" resources will be gone.
This is due to the race between the allocator scheduling an `allocate`
call to itself vs master's
`allocator->updateAvailable` invocation.
As such, we first try to satisfy the request only with the offered
resources.
* We greedily rescind one offer at a time until we've rescinded
sufficiently many offers.
IMPORTANT: We perform `recoverResources(..., Filters())` which has a
default `refuse_sec` of 5 seconds,
rather than `recoverResources(..., None())` so that we can virtually
always win the race against `allocate`.
In the rare case that we do lose, no disaster occurs. We simply fail to
satisfy the request.
* If we still don't have enough resources after resciding all offers, be
semi-optimistic and forward the
request to the allocator since there may be available resources to
satisfy the request.
* If the allocator returns a failure, report the error to the user with
`Conflict`.

This approach is clearly not ideal, since we would prefer to rescind as little
offers as possible.

Diffs (updated)
-

src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3
src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db
src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43
src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a
src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5

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

Testing
---

`make check`

Thanks,

Michael Park

Re: Review Request 36402: Adding 'Accept' header in request

2015-08-05 Thread Isabel Jimenez


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

(Updated Aug. 5, 2015, 7:11 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and 
Vinod Kone.


Repository: mesos-incubating


Description
---

Adding a method for Accept header in request + refactor of Accept-Encoding


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp b8d9300 
  3rdparty/libprocess/src/http.cpp 4dcbd74 
  3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 36979: Updating all references to os::shell


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


Patch looks great!

Reviews applied: [36978, 36979]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 5:10 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36979/
> ---
> 
> (Updated Aug. 5, 2015, 5:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3142
> https://issues.apache.org/jira/browse/MESOS-3142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updating all references to os::shell
> For more details see MESOS-3142.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 
>   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 8244c345b84108af7fa18d20e71401d6e1a0aeb0 
>   src/tests/containerizer/isolator_tests.cpp 
> ff6e2b7e190a58a4809d6e71addb15dabe418e17 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 4bee74acba2b1472c80cabbc9d0384bd04c543aa 
> 
> Diff: https://reviews.apache.org/r/36979/diff/
> 
> 
> Testing
> ---
> 
> make check
> *Note*: this patch fixes breakages introduce by the refactoring in: 
> https://reviews.apache.org/r/36978
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 37072: Added test for allocator update on scheduler failover

2015-08-05 Thread Vinod Kone


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

Ship it!


Can you please mark the issues as resolved?


src/tests/oversubscription_tests.cpp 


I would keep the new line. It's a bit dense otherwise.


- Vinod Kone


On Aug. 5, 2015, 6:51 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37072/
> ---
> 
> (Updated Aug. 5, 2015, 6:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2880
> https://issues.apache.org/jira/browse/MESOS-2880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for allocator update on scheduler failover
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> c7a2dacb600d7703de6090e7e47f453a3d08b53a 
> 
> Diff: https://reviews.apache.org/r/37072/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 37097: Fix 'Accept-Encoding' parsing

2015-08-05 Thread Isabel Jimenez


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

(Updated Aug. 5, 2015, 7 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Currently parsing only compares the begining of the header making 'gzipbug' 
match with candidate 'gzip'


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp b8d9300 
  3rdparty/libprocess/src/http.cpp 4dcbd74 
  3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 37072: Added test for allocator update on scheduler failover

2015-08-05 Thread Aditi Dixit


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

(Updated Aug. 5, 2015, 6:51 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added test for allocator update on scheduler failover


Diffs (updated)
-

  src/tests/oversubscription_tests.cpp c7a2dacb600d7703de6090e7e47f453a3d08b53a 

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


Testing
---

make check


Thanks,

Aditi Dixit

Re: Review Request 37045: Convert Linux perf sampler to use process:await().

2015-08-05 Thread Paul Brett


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

(Updated Aug. 5, 2015, 6:23 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Incorporate feedback from Ben


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


Repository: mesos


Description
---

Convert Linux perf sampler to use process:await().


Diffs (updated)
-

  src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.


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


Patch looks great!

Reviews applied: [37053]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 5:09 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37053/
> ---
> 
> (Updated Aug. 5, 2015, 5:09 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-3196
> https://issues.apache.org/jira/browse/MESOS-3196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Always set executor_id when sending StatusUpdate.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e 
>   src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
>   src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db 
>   src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 
> 
> Diff: https://reviews.apache.org/r/37053/diff/
> 
> 
> Testing
> ---
> 
> make check with a couple of added checks that verify that the executor_id is 
> set in the incoming TaskStatus in Master as well as Scheduler.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 37045: Convert Linux perf sampler to use process:await().

2015-08-05 Thread Ben Mahler


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


Mostly comments from the last review


src/linux/perf.cpp (lines 295 - 296)


This still isn't lined up?



src/linux/perf.cpp (line 297)


I think we're trying to avoid the blanket '=' if possible, looks like you 
only use 'this' here? Does this work if you capture 'this' only?



src/linux/perf.cpp (line 301)


Please add an explicit CHECK that this is not discarded, rather than 
relying on .get crashing



src/linux/perf.cpp (line 303)


Let's print the .failure of the future if it's failed.



src/linux/perf.cpp (lines 308 - 309)


No period for composition per the last comment, also WSTRINGIFY will print 
'exited with status' already if appropriate, so this is double logging it.



src/linux/perf.cpp (line 314)


Please add an explicit CHECK that this is not discarded (otherwise calling 
.failure / .get crashing)


- Ben Mahler


On Aug. 5, 2015, 5:25 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37045/
> ---
> 
> (Updated Aug. 5, 2015, 5:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Linux perf sampler to use process:await().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 
> 
> Diff: https://reviews.apache.org/r/37045/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.



> On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
> > src/master/http.cpp, line 534
> > 
> >
> > I don't like the name 'flatten' :(
> > 
> > Could you at least be more explicit about it (i.e., emphasize that 
> > 'remaining' only has unreserved resources). 
> > 
> > ```
> > Resources remaining = resources.flatten('*');
> > ```
> 
> Michael Park wrote:
> I don't like it either, but we currently have 9 instances of `flatten()` 
> but no instances of `flatten("*")`. Do you think it's worth breaking 
> consistency here? As far as I know, we seem to favor consistency.

OK, fair enough. Given the comment you just added, I think it's much more clear 
not.


- Jie


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


On Aug. 5, 2015, 10:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated Aug. 5, 2015, 10:44 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
> `recoverResources(..., None())` so that we can pretty much always win the 
> race against `allocate`.
>  In the case that we lose, no disaster occurs. We simply fail 
> to satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> optimistic and forward the request to the allocator since there may be 
> available resources to satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
> maybe as well. We'll pick one eventually.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> The challenges of implementing the ideal solution in the current state is 
> described in the document above.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
>   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.



> On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
> > src/master/http.cpp, line 573
> > 
> >
> > What is 'Nothing' here?
> 
> Michael Park wrote:
> The `Nothing` here comes from the result of `master->apply` which returns 
> a `Future`. But I feel like you're not actually asking for an answer 
> here?
> 
> What would you like to see?
> 
> What I have currently is a comment above the code which reads:
> 
> ```cpp
> // Propogate the 'Future' as 'Future' where
> // 'Nothing' -> 'OK' and Failed -> 'Conflict'.
> ```

I mean if we remove 'Nothing' here, will it compile?


- Jie


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


On Aug. 5, 2015, 10:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated Aug. 5, 2015, 10:44 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
> `recoverResources(..., None())` so that we can pretty much always win the 
> race against `allocate`.
>  In the case that we lose, no disaster occurs. We simply fail 
> to satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> optimistic and forward the request to the allocator since there may be 
> available resources to satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
> maybe as well. We'll pick one eventually.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> The challenges of implementing the ideal solution in the current state is 
> described in the document above.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
>   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 37133: Add a frameworks parameter to the hierarchical allocator benchmark.

2015-08-05 Thread James Peach


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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Parameterize the hierarchical allocator benchmark by the framework
count as well as the slave count. This can be used to explore
allocation behavior as the number of frameworks increases.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
c92d47aa0a06088f1f8fe749a629c27bfadd3c6d 

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


Testing
---

make check
make bench


Thanks,

James Peach

Re: Review Request 37073: Added process id prefix for cram_md5_authentication_tests.


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


Patch looks great!

Reviews applied: [37073]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 2:53 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37073/
> ---
> 
> (Updated Aug. 5, 2015, 2:53 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1457
> https://issues.apache.org/jira/browse/MESOS-1457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch was originally proposed by Palak Choudhary via 
> https://reviews.apache.org/r/31075/. This version is just rebased onto the 
> current master.
> 
> 
> Diffs
> -
> 
>   src/tests/cram_md5_authentication_tests.cpp 
> b216ae4821c5bd967ea1b0e6787270246ff224f4 
> 
> Diff: https://reviews.apache.org/r/37073/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 36979: Updating all references to os::shell

2015-08-05 Thread Marco Massenzio


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

(Updated Aug. 5, 2015, 5:10 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


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


Repository: mesos


Description
---

Updating all references to os::shell
For more details see MESOS-3142.


Diffs (updated)
-

  src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 
  src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
8244c345b84108af7fa18d20e71401d6e1a0aeb0 
  src/tests/containerizer/isolator_tests.cpp 
ff6e2b7e190a58a4809d6e71addb15dabe418e17 
  src/tests/containerizer/port_mapping_tests.cpp 
4bee74acba2b1472c80cabbc9d0384bd04c543aa 

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


Testing
---

make check
*Note*: this patch fixes breakages introduce by the refactoring in: 
https://reviews.apache.org/r/36978


Thanks,

Marco Massenzio

Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.

2015-08-05 Thread Kapil Arya


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

(Updated Aug. 5, 2015, 1:09 p.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Changes
---

Added warning message if executor ids mismatch.


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


Repository: mesos


Description
---

Always set executor_id when sending StatusUpdate.


Diffs (updated)
-

  include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e 
  src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 
  src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db 
  src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 

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


Testing
---

make check with a couple of added checks that verify that the executor_id is 
set in the incoming TaskStatus in Master as well as Scheduler.


Thanks,

Kapil Arya

Re: Review Request 36979: Updating all references to os::shell

2015-08-05 Thread Marco Massenzio



> On Aug. 5, 2015, 4:16 a.m., Artem Harutyunyan wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1269
> > 
> >
> > You are right that the awk did not actually seem to accomplish anything 
> > meaningful here.

my major concern is that these are ROOT Container tests that won't be run on 
OSX (and won't be run often either) - so wanted to mark it, as if the test 
fails we know who to blame (me!)

I'll double check on an Ubuntu server too.


> On Aug. 5, 2015, 4:16 a.m., Artem Harutyunyan wrote:
> > src/tests/containerizer/port_mapping_tests.cpp, lines 971-974
> > 
> >
> > Another illustration of why a tuple return type might be a better 
> > option for os::shell() :-)
> > 
> > But regardless, I'd change this code to something more suggestive (it's 
> > a test case after all), or at least would add a comment that clarifies the 
> > intention.

Added a comment, that was sorely needed, you're right!
As for the tuple, that's what `process::Subprocess` will be for.
We assume that `os::shell` usage is when one wants to just run a command and 
only cares: did it succeed?
(this was actually the **only** place in the code base where anyone cared about 
the exit code, believe it or not).


> On Aug. 5, 2015, 4:16 a.m., Artem Harutyunyan wrote:
> > src/tests/containerizer/port_mapping_tests.cpp, line 986
> > 
> >
> > ditto. 
> > + extra newline.

Having looked at both tests, I was being unnecessarily pedantic IMO: checking 
for the error code (256) to be present in the error string seems to me to be 
more than sufficient (and self-explanatory too - but added a comment all the 
same).

What thinks you?


- Marco


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


On Aug. 5, 2015, 12:55 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36979/
> ---
> 
> (Updated Aug. 5, 2015, 12:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3142
> https://issues.apache.org/jira/browse/MESOS-3142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updating all references to os::shell
> For more details see MESOS-3142.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 
>   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a 
>   src/tests/containerizer/isolator_tests.cpp 
> ff6e2b7e190a58a4809d6e71addb15dabe418e17 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 4bee74acba2b1472c80cabbc9d0384bd04c543aa 
> 
> Diff: https://reviews.apache.org/r/36979/diff/
> 
> 
> Testing
> ---
> 
> make check
> *Note*: this patch fixes breakages introduce by the refactoring in: 
> https://reviews.apache.org/r/36978
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 18723: Prevent gflags detection from glog configuration phase


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

Ship it!


Ship It!

- haosdent huang


On March 6, 2014, 6:48 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18723/
> ---
> 
> (Updated March 6, 2014, 6:48 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1010
> https://issues.apache.org/jira/browse/MESOS-1010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes the gflags detection from the glog configuration phase.
> 
> Patched both, configure.ac as well as configure to prevent us from having to 
> run autoreconf on this bundle by default.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/glog-0.3.3.patch 76b8c0f 
> 
> Diff: https://reviews.apache.org/r/18723/diff/
> 
> 
> Testing
> ---
> 
> make check (while having gflags previously installed).
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 37074: Added process id prefix for benchmarks and process tests.


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


Patch looks great!

Reviews applied: [37074]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 2:52 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37074/
> ---
> 
> (Updated Aug. 5, 2015, 2:52 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1457
> https://issues.apache.org/jira/browse/MESOS-1457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch was originally proposed by Palak Choudhary via 
> https://reviews.apache.org/r/31076/. This version is just rebased onto the 
> current master.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 95e3257b030128e9d03dde9aa048602c68c6a446 
> 
> Diff: https://reviews.apache.org/r/37074/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 37106: PortMappingIsolatorProcess shell script can silently fail.

2015-08-05 Thread Chi Zhang


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

Ship it!


Ship It!

- Chi Zhang


On Aug. 5, 2015, 12:58 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37106/
> ---
> 
> (Updated Aug. 5, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-3204
> https://issues.apache.org/jira/browse/MESOS-3204
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PortMappingIsolatorProcess shell script can silently fail.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 8244c345b84108af7fa18d20e71401d6e1a0aeb0 
> 
> Diff: https://reviews.apache.org/r/37106/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 36908: Added QuotaInfo Protobuf.


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


Patch looks great!

Reviews applied: [36908]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 2:03 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36908/
> ---
> 
> (Updated Aug. 5, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3164
> https://issues.apache.org/jira/browse/MESOS-3164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added QuotaInfo Protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/quota.hpp PRE-CREATION 
>   include/mesos/master/quota.proto PRE-CREATION 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
> 
> Diff: https://reviews.apache.org/r/36908/diff/
> 
> 
> Testing
> ---
> 
> make distcheck
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 37080: Introduced RecordIO response reader


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

(Updated Aug. 5, 2015, 3:31 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added the ".hpp" to the makefile ( would be needed by distcheck )


Repository: mesos


Description
---

Added a decorator class over the existing Record-IO decoder in stout 
(MESOS-3067). The decorator accepts a Pipe::Reader thereby abstracting away the 
record reading logic. The future returns true only when it sees a complete 
record. This cleaned up the tests as we no longer need to invoke the stout 
decoder multiple times till we got atleast one element.

Needed for MESOS-2294 testing.


Diffs (updated)
-

  src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
  src/common/recordio_response.hpp PRE-CREATION 
  src/tests/common/recordio_response_tests.cpp PRE-CREATION 

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


Testing
---

make check + added tests.


Thanks,

Anand Mazumdar

Re: Review Request 36916: Doxygenified a comment in the allocator.proto.


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

Ship it!


Ship It!

- Till Toenshoff


On July 29, 2015, 4:58 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36916/
> ---
> 
> (Updated July 29, 2015, 4:58 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Doxygenified a comment in the allocator.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.proto 
> c32b88ed0252bf71e738ee64d64ffa828e97b2a3 
> 
> Diff: https://reviews.apache.org/r/36916/diff/
> 
> 
> Testing
> ---
> 
> none.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 36910: Patch configure.ac to include $LIBS in the CRAM-MD5 check


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

Ship it!


Ship It!

- Till Toenshoff


On Aug. 4, 2015, 1:49 p.m., Chris Heller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36910/
> ---
> 
> (Updated Aug. 4, 2015, 1:49 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-3170
> https://issues.apache.org/jira/browse/MESOS-3170
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-3170] Add $LIBS to build path of the CRAM-MD5 test
> 
> 
> Diffs
> -
> 
>   configure.ac 9438d88 
> 
> Diff: https://reviews.apache.org/r/36910/diff/
> 
> 
> Testing
> ---
> 
> See https://github.com/apache/mesos/pull/51
> 
> Verified build against statically linked OpenSSL 1.0.1e and Cyrus-SASL 2.1.26
> 
> 
> Thanks,
> 
> Chris Heller
> 
>

Re: Review Request 36911: Removed unnecessary using directive.


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

Ship it!


Ship It!

- Till Toenshoff


On Aug. 3, 2015, 2:58 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36911/
> ---
> 
> (Updated Aug. 3, 2015, 2:58 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unnecessary using directive.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a 
> 
> Diff: https://reviews.apache.org/r/36911/diff/
> 
> 
> Testing
> ---
> 
> make check (clang on Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 37074: Added process id prefix for benchmarks and process tests.


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

Review request for mesos.


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


Repository: mesos


Description
---

This patch was originally proposed by Palak Choudhary via 
https://reviews.apache.org/r/31076/. This version is just rebased onto the 
current master.


Diffs
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
  3rdparty/libprocess/src/tests/process_tests.cpp 
95e3257b030128e9d03dde9aa048602c68c6a446 

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


Testing
---

make check


Thanks,

Till Toenshoff

Review Request 37073: Added process id prefix for cram_md5_authentication_tests.


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

Review request for mesos.


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


Repository: mesos


Description
---

This patch was originally proposed by Palak Choudhary via 
https://reviews.apache.org/r/31075/. This version is just rebased onto the 
current master.


Diffs
-

  src/tests/cram_md5_authentication_tests.cpp 
b216ae4821c5bd967ea1b0e6787270246ff224f4 

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.


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


Patch looks great!

Reviews applied: [37081]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 10:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37081/
> ---
> 
> (Updated Aug. 5, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Kapil Arya, Marco Massenzio, and 
> Sebastien Pahl.
> 
> 
> Bugs: MESOS-2337
> https://issues.apache.org/jira/browse/MESOS-2337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Let __init__.py getting installed to 
> $PREFIX/lib/pythonX.Y/site-packages/mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 
>   src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a 
> 
> Diff: https://reviews.apache.org/r/37081/diff/
> 
> 
> Testing
> ---
> 
> ## Test steps
> ### In CentOS 6.6
> ```
> ./bootstrap
> mkdir build
> cd build
> make -j4
> sudo make install
> ```
> 
> And then execute
> 
> ```
> export PYTHONPATH=/usr/local/lib/python2.6/site-packages
> python -c "import mesos; from mesos import native; from mesos import 
> interface"
> ```
> 
> Check it could success or not.
> 
> Also check the `__init__.py` file 
> ```
> cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36908: Added QuotaInfo Protobuf.

2015-08-05 Thread Joerg Schad


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

(Updated Aug. 5, 2015, 2:03 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Added QuotaInfo Protobuf.


Diffs (updated)
-

  include/mesos/master/quota.hpp PRE-CREATION 
  include/mesos/master/quota.proto PRE-CREATION 
  src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 

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


Testing
---

make distcheck


Thanks,

Joerg Schad

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 1:47 p.m., Sebastien Pahl wrote:
> > I would say ship it if everyone agrees that this now depends on this python 
> > package just as a hack to add an empty __init__.py to the google submodule 
> > for protobuf.
> > 
> > A better solution could be found later of course.
> 
> Sebastien Pahl wrote:
> https://pypi.python.org/pypi/google-common

Yes, I think we need add a way to install dependency before build.


- haosdent


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


On Aug. 5, 2015, 10:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37081/
> ---
> 
> (Updated Aug. 5, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Kapil Arya, Marco Massenzio, and 
> Sebastien Pahl.
> 
> 
> Bugs: MESOS-2337
> https://issues.apache.org/jira/browse/MESOS-2337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Let __init__.py getting installed to 
> $PREFIX/lib/pythonX.Y/site-packages/mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 
>   src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a 
> 
> Diff: https://reviews.apache.org/r/37081/diff/
> 
> 
> Testing
> ---
> 
> ## Test steps
> ### In CentOS 6.6
> ```
> ./bootstrap
> mkdir build
> cd build
> make -j4
> sudo make install
> ```
> 
> And then execute
> 
> ```
> export PYTHONPATH=/usr/local/lib/python2.6/site-packages
> python -c "import mesos; from mesos import native; from mesos import 
> interface"
> ```
> 
> Check it could success or not.
> 
> Also check the `__init__.py` file 
> ```
> cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 1:47 p.m., Sebastien Pahl wrote:
> > I would say ship it if everyone agrees that this now depends on this python 
> > package just as a hack to add an empty __init__.py to the google submodule 
> > for protobuf.
> > 
> > A better solution could be found later of course.

https://pypi.python.org/pypi/google-common


- Sebastien


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


On Aug. 5, 2015, 10:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37081/
> ---
> 
> (Updated Aug. 5, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Kapil Arya, Marco Massenzio, and 
> Sebastien Pahl.
> 
> 
> Bugs: MESOS-2337
> https://issues.apache.org/jira/browse/MESOS-2337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Let __init__.py getting installed to 
> $PREFIX/lib/pythonX.Y/site-packages/mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 
>   src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a 
> 
> Diff: https://reviews.apache.org/r/37081/diff/
> 
> 
> Testing
> ---
> 
> ## Test steps
> ### In CentOS 6.6
> ```
> ./bootstrap
> mkdir build
> cd build
> make -j4
> sudo make install
> ```
> 
> And then execute
> 
> ```
> export PYTHONPATH=/usr/local/lib/python2.6/site-packages
> python -c "import mesos; from mesos import native; from mesos import 
> interface"
> ```
> 
> Check it could success or not.
> 
> Also check the `__init__.py` file 
> ```
> cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.


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

Ship it!


I would say ship it if everyone agrees that this now depends on this python 
package just as a hack to add an empty __init__.py to the google submodule for 
protobuf.

A better solution could be found later of course.

- Sebastien Pahl


On Aug. 5, 2015, 10:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37081/
> ---
> 
> (Updated Aug. 5, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Kapil Arya, Marco Massenzio, and 
> Sebastien Pahl.
> 
> 
> Bugs: MESOS-2337
> https://issues.apache.org/jira/browse/MESOS-2337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Let __init__.py getting installed to 
> $PREFIX/lib/pythonX.Y/site-packages/mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 
>   src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a 
> 
> Diff: https://reviews.apache.org/r/37081/diff/
> 
> 
> Testing
> ---
> 
> ## Test steps
> ### In CentOS 6.6
> ```
> ./bootstrap
> mkdir build
> cd build
> make -j4
> sudo make install
> ```
> 
> And then execute
> 
> ```
> export PYTHONPATH=/usr/local/lib/python2.6/site-packages
> python -c "import mesos; from mesos import native; from mesos import 
> interface"
> ```
> 
> Check it could success or not.
> 
> Also check the `__init__.py` file 
> ```
> cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37127: Added framework authorization for dynamic reservation.


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


Patch looks great!

Reviews applied: [36987, 35702, 35983, 35984, 37002, 37110, 37125, 37126, 37127]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 11:17 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37127/
> ---
> 
> (Updated Aug. 5, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
> 
> Diff: https://reviews.apache.org/r/37127/diff/
> 
> 
> Testing
> ---
> 
> Added tests to `src/tests/reservation_tests.cpp` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 9:57 a.m., Sebastien Pahl wrote:
> > src/python/interface/setup.py.in, line 27
> > 
> >
> > This seems weird to me. Why do you need to add the bindings for the 
> > google search engine?
> > 
> > https://pypi.python.org/pypi/google
> 
> haosdent huang wrote:
> Yes, I think it is also a bit dirty here.
> 
> Because I try to fix this, could you see @marco comment above. I don't 
> have other ideas to let it contains `__init__.py` under google, so I add this 
> dependency.
> 
> ```
> ok - this almost worked :)
> Are we missing an __init__.py in google?
> 
> 
> $ ../configure --prefix=/Users/marco/mesos-inst/lib
> 
> then the usual make && make install - then:
> 
> $ find . |grep __init__
> ./lib/python/site-packages/google/protobuf/__init__.py
> ./lib/python/site-packages/google/protobuf/__init__.pyc
> ./lib/python/site-packages/google/protobuf/compiler/__init__.py
> ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc
> ./lib/python/site-packages/google/protobuf/internal/__init__.py
> ./lib/python/site-packages/google/protobuf/internal/__init__.pyc
> ./lib/python/site-packages/google/protobuf/pyext/__init__.py
> ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc
> ./lib/python/site-packages/mesos/__init__.py
> ./lib/python/site-packages/mesos/__init__.pyc
> ./lib/python/site-packages/mesos/interface/__init__.py
> ./lib/python/site-packages/mesos/interface/__init__.pyc
> ./lib/python/site-packages/mesos/native/__init__.py
> ./lib/python/site-packages/mesos/native/__init__.pyc
> ./libexec/mesos/python/mesos/__init__.py
> 
> as you can see, there are now the __init__.py files in the right places 
> under mesos (if you do the same from master there's nothing in there - so, 
> yay! :) but there is none under the google/ package:
> 
> [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/
> 
> [~/mesos-inst/lib] $ python
> Python 2.7.6 (default, Sep  9 2014, 15:04:36) 
> [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import mesos
> >>> from mesos import native
> Traceback (most recent call last):
>   File "", line 1, in 
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py",
>  line 17, in 
> from ._mesos import MesosExecutorDriverImpl
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py",
>  line 4, in 
> from google.protobuf.internal import enum_type_wrapper
> ImportError: No module named google.protobuf.internal
> >>> import google.protobuf.internal
> Traceback (most recent call last):
>   File "", line 1, in 
> ImportError: No module named google.protobuf.internal
> 
> This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an 
> Ubuntu 14.04 VM, brand new.
> ```
> 
> haosdent huang wrote:
> For openstack, we could find `python-openstack-common` which contains 
> `__init__.py` under [root 
> namespace](http://packages.ubuntu.com/precise/i386/python-openstack-common/filelist),
>  althrough openstack have multi submodules. But seems google don't provide 
> package like this.
> 
> haosdent huang wrote:
> I upload a `__init__.py` to https://pypi.python.org/pypi/google-common, 
> and change dependcy to this. But I think it is also tricky here.
> 
> Sebastien Pahl wrote:
> I can't seem to reproduce your problem.
> 
> Here is what I get:
> ```
> ?  ~  mkvirtualenv -p python2.7 mesos
> Running virtualenv with interpreter /usr/bin/python2.7
> New python executable in mesos/bin/python2.7
> Also creating executable in mesos/bin/python
> Installing setuptools, pip, wheel...done.
> ?  ~  pip install protobuf   
> Collecting protobuf
> Requirement already satisfied (use --upgrade to upgrade): setuptools in 
> ./.virtualenvs/mesos/lib/python2.7/site-packages (from protobuf)
> Installing collected packages: protobuf
> Successfully installed protobuf-2.6.1
> ?  ~  python
> Python 2.7.10 (default, May 26 2015, 04:16:29) 
> [GCC 5.1.0] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import google.protobuf
> >>> 
> ?  ~  pip freeze
> protobuf==2.6.1
> wheel==0.24.0
> ?  ~  ls -l ./.virtualenvs/mesos/lib/python2.7/site-packages/google 
> total 4
> drwxr-xr-x 5 seb seb 4096 Aug  5 04:18 protobuf
> ```
> 
> No need for the google-common package
> 
> haosdent huang wrote:
> If you already install protobuf, you could install this success. But if 
> you don't install protobuf before, You would get error. Because we 
>

Re: Review Request 37045: Convert Linux perf sampler to use process:await().


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


Patch looks great!

Reviews applied: [37045]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 5:25 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37045/
> ---
> 
> (Updated Aug. 5, 2015, 5:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Linux perf sampler to use process:await().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 
> 
> Diff: https://reviews.apache.org/r/37045/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 9:57 a.m., Sebastien Pahl wrote:
> > src/python/interface/setup.py.in, line 27
> > 
> >
> > This seems weird to me. Why do you need to add the bindings for the 
> > google search engine?
> > 
> > https://pypi.python.org/pypi/google
> 
> haosdent huang wrote:
> Yes, I think it is also a bit dirty here.
> 
> Because I try to fix this, could you see @marco comment above. I don't 
> have other ideas to let it contains `__init__.py` under google, so I add this 
> dependency.
> 
> ```
> ok - this almost worked :)
> Are we missing an __init__.py in google?
> 
> 
> $ ../configure --prefix=/Users/marco/mesos-inst/lib
> 
> then the usual make && make install - then:
> 
> $ find . |grep __init__
> ./lib/python/site-packages/google/protobuf/__init__.py
> ./lib/python/site-packages/google/protobuf/__init__.pyc
> ./lib/python/site-packages/google/protobuf/compiler/__init__.py
> ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc
> ./lib/python/site-packages/google/protobuf/internal/__init__.py
> ./lib/python/site-packages/google/protobuf/internal/__init__.pyc
> ./lib/python/site-packages/google/protobuf/pyext/__init__.py
> ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc
> ./lib/python/site-packages/mesos/__init__.py
> ./lib/python/site-packages/mesos/__init__.pyc
> ./lib/python/site-packages/mesos/interface/__init__.py
> ./lib/python/site-packages/mesos/interface/__init__.pyc
> ./lib/python/site-packages/mesos/native/__init__.py
> ./lib/python/site-packages/mesos/native/__init__.pyc
> ./libexec/mesos/python/mesos/__init__.py
> 
> as you can see, there are now the __init__.py files in the right places 
> under mesos (if you do the same from master there's nothing in there - so, 
> yay! :) but there is none under the google/ package:
> 
> [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/
> 
> [~/mesos-inst/lib] $ python
> Python 2.7.6 (default, Sep  9 2014, 15:04:36) 
> [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import mesos
> >>> from mesos import native
> Traceback (most recent call last):
>   File "", line 1, in 
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py",
>  line 17, in 
> from ._mesos import MesosExecutorDriverImpl
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py",
>  line 4, in 
> from google.protobuf.internal import enum_type_wrapper
> ImportError: No module named google.protobuf.internal
> >>> import google.protobuf.internal
> Traceback (most recent call last):
>   File "", line 1, in 
> ImportError: No module named google.protobuf.internal
> 
> This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an 
> Ubuntu 14.04 VM, brand new.
> ```
> 
> haosdent huang wrote:
> For openstack, we could find `python-openstack-common` which contains 
> `__init__.py` under [root 
> namespace](http://packages.ubuntu.com/precise/i386/python-openstack-common/filelist),
>  althrough openstack have multi submodules. But seems google don't provide 
> package like this.
> 
> haosdent huang wrote:
> I upload a `__init__.py` to https://pypi.python.org/pypi/google-common, 
> and change dependcy to this. But I think it is also tricky here.
> 
> Sebastien Pahl wrote:
> I can't seem to reproduce your problem.
> 
> Here is what I get:
> ```
> ?  ~  mkvirtualenv -p python2.7 mesos
> Running virtualenv with interpreter /usr/bin/python2.7
> New python executable in mesos/bin/python2.7
> Also creating executable in mesos/bin/python
> Installing setuptools, pip, wheel...done.
> ?  ~  pip install protobuf   
> Collecting protobuf
> Requirement already satisfied (use --upgrade to upgrade): setuptools in 
> ./.virtualenvs/mesos/lib/python2.7/site-packages (from protobuf)
> Installing collected packages: protobuf
> Successfully installed protobuf-2.6.1
> ?  ~  python
> Python 2.7.10 (default, May 26 2015, 04:16:29) 
> [GCC 5.1.0] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import google.protobuf
> >>> 
> ?  ~  pip freeze
> protobuf==2.6.1
> wheel==0.24.0
> ?  ~  ls -l ./.virtualenvs/mesos/lib/python2.7/site-packages/google 
> total 4
> drwxr-xr-x 5 seb seb 4096 Aug  5 04:18 protobuf
> ```
> 
> No need for the google-common package
> 
> haosdent huang wrote:
> If you already install protobuf, you could install this success. But if 
> you don't install protobuf before, You would get error. Because we 
>

Re: Review Request 37109: Removed ability to mutate user from scheduler library


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


Patch looks great!

Reviews applied: [37109]

All tests passed.

- Mesos ReviewBot


On Aug. 5, 2015, 1:23 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37109/
> ---
> 
> (Updated Aug. 5, 2015, 1:23 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler used to mutate the user field to the os::user if it was not 
> set. This patch removes this functionality and leaves it upto the client to 
> provide a user as part of SUBSCRIBE.
> 
> 
> Diffs
> -
> 
>   src/examples/event_call_framework.cpp 
> 8054068fa746f8635f1133ceac530e04eaa0e1d7 
>   src/scheduler/scheduler.cpp 97fa2c063db506dec69ff1edd851c96b4e1219a4 
> 
> Diff: https://reviews.apache.org/r/37109/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 37010: stout: Removed unused 'fatal' and 'fatalerror' macros.


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

(Updated Aug. 5, 2015, 4:56 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Changes
---

Added patch dependencies.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ce850337dd4591027993c242dae321f21516f065 
  3rdparty/libprocess/3rdparty/stout/include/stout/fatal.hpp 
053ef82d9d152984b5e7a1b915353356be2cdd2f 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 37011: libprocess: Removed unused 'fatal' and 'fatalerror' macros.


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

(Updated Aug. 5, 2015, 4:54 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Changes
---

This patch does not depend on anything.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/fatal.hpp 87a55dca4fe53c1f3fc7fb03914f3ec4270aa5b4 
  3rdparty/libprocess/src/fatal.cpp 76d5ee42be50651863f88189341d59cfd406bae4 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 37011: libprocess: Removed unused 'fatal' and 'fatalerror' macros.


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

(Updated Aug. 5, 2015, 4:52 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Changes
---

Fixed patch dependency chain.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/fatal.hpp 87a55dca4fe53c1f3fc7fb03914f3ec4270aa5b4 
  3rdparty/libprocess/src/fatal.cpp 76d5ee42be50651863f88189341d59cfd406bae4 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 37011: libprocess: Removed unused 'fatal' and 'fatalerror' macros.

2015-08-05 Thread Bernd Mathiske


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

Ship it!


Ship It!

- Bernd Mathiske


On Aug. 5, 2015, 4:51 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37011/
> ---
> 
> (Updated Aug. 5, 2015, 4:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3200
> https://issues.apache.org/jira/browse/MESOS-3200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/fatal.hpp 87a55dca4fe53c1f3fc7fb03914f3ec4270aa5b4 
>   3rdparty/libprocess/src/fatal.cpp 76d5ee42be50651863f88189341d59cfd406bae4 
> 
> Diff: https://reviews.apache.org/r/37011/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 37011: libprocess: Removed unused 'fatal' and 'fatalerror' macros.


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

(Updated Aug. 5, 2015, 4:51 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Changes
---

This patch does not actually depend on anything.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/fatal.hpp 87a55dca4fe53c1f3fc7fb03914f3ec4270aa5b4 
  3rdparty/libprocess/src/fatal.cpp 76d5ee42be50651863f88189341d59cfd406bae4 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 37012: mesos: Removed unused 'fatal' and 'fatalerror' macros.


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

(Updated Aug. 5, 2015, 4:47 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Changes
---

This patch does not depend on anything.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/exec/exec.cpp d59a7e1df0d4786ee72b410f4fbb3b71e585b39d 
  src/zookeeper/zookeeper.cpp fe862aa0b2f0124a786a805ec6be9615599da2fb 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 37012: mesos: Removed unused 'fatal' and 'fatalerror' macros.

2015-08-05 Thread Bernd Mathiske


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

Ship it!


Ship It!

- Bernd Mathiske


On Aug. 4, 2015, 10:43 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37012/
> ---
> 
> (Updated Aug. 4, 2015, 10:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3200
> https://issues.apache.org/jira/browse/MESOS-3200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/exec/exec.cpp d59a7e1df0d4786ee72b410f4fbb3b71e585b39d 
>   src/zookeeper/zookeeper.cpp fe862aa0b2f0124a786a805ec6be9615599da2fb 
> 
> Diff: https://reviews.apache.org/r/37012/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 37075: Protobuf definitions instructing the fetcher cache about checksums and their validation.

2015-08-05 Thread Bernd Mathiske


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

(Updated Aug. 5, 2015, 4:37 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy 
Chen.


Changes
---

Changed ticket to epic subticket.


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


Repository: mesos


Description
---

Protobuf definitions instructing the fetcher cache about checksums and their 
validation. First in a series of patches implementing phase 1 of MESOS-2073 
(see comments in the ticket). There are references to docs in this patch, but 
updating docs will be in the last patch.


Diffs
-

  include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba 

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


Testing
---


Thanks,

Bernd Mathiske

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 9:57 a.m., Sebastien Pahl wrote:
> > src/python/interface/setup.py.in, line 27
> > 
> >
> > This seems weird to me. Why do you need to add the bindings for the 
> > google search engine?
> > 
> > https://pypi.python.org/pypi/google
> 
> haosdent huang wrote:
> Yes, I think it is also a bit dirty here.
> 
> Because I try to fix this, could you see @marco comment above. I don't 
> have other ideas to let it contains `__init__.py` under google, so I add this 
> dependency.
> 
> ```
> ok - this almost worked :)
> Are we missing an __init__.py in google?
> 
> 
> $ ../configure --prefix=/Users/marco/mesos-inst/lib
> 
> then the usual make && make install - then:
> 
> $ find . |grep __init__
> ./lib/python/site-packages/google/protobuf/__init__.py
> ./lib/python/site-packages/google/protobuf/__init__.pyc
> ./lib/python/site-packages/google/protobuf/compiler/__init__.py
> ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc
> ./lib/python/site-packages/google/protobuf/internal/__init__.py
> ./lib/python/site-packages/google/protobuf/internal/__init__.pyc
> ./lib/python/site-packages/google/protobuf/pyext/__init__.py
> ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc
> ./lib/python/site-packages/mesos/__init__.py
> ./lib/python/site-packages/mesos/__init__.pyc
> ./lib/python/site-packages/mesos/interface/__init__.py
> ./lib/python/site-packages/mesos/interface/__init__.pyc
> ./lib/python/site-packages/mesos/native/__init__.py
> ./lib/python/site-packages/mesos/native/__init__.pyc
> ./libexec/mesos/python/mesos/__init__.py
> 
> as you can see, there are now the __init__.py files in the right places 
> under mesos (if you do the same from master there's nothing in there - so, 
> yay! :) but there is none under the google/ package:
> 
> [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/
> 
> [~/mesos-inst/lib] $ python
> Python 2.7.6 (default, Sep  9 2014, 15:04:36) 
> [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import mesos
> >>> from mesos import native
> Traceback (most recent call last):
>   File "", line 1, in 
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py",
>  line 17, in 
> from ._mesos import MesosExecutorDriverImpl
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py",
>  line 4, in 
> from google.protobuf.internal import enum_type_wrapper
> ImportError: No module named google.protobuf.internal
> >>> import google.protobuf.internal
> Traceback (most recent call last):
>   File "", line 1, in 
> ImportError: No module named google.protobuf.internal
> 
> This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an 
> Ubuntu 14.04 VM, brand new.
> ```
> 
> haosdent huang wrote:
> For openstack, we could find `python-openstack-common` which contains 
> `__init__.py` under [root 
> namespace](http://packages.ubuntu.com/precise/i386/python-openstack-common/filelist),
>  althrough openstack have multi submodules. But seems google don't provide 
> package like this.
> 
> haosdent huang wrote:
> I upload a `__init__.py` to https://pypi.python.org/pypi/google-common, 
> and change dependcy to this. But I think it is also tricky here.
> 
> Sebastien Pahl wrote:
> I can't seem to reproduce your problem.
> 
> Here is what I get:
> ```
> ?  ~  mkvirtualenv -p python2.7 mesos
> Running virtualenv with interpreter /usr/bin/python2.7
> New python executable in mesos/bin/python2.7
> Also creating executable in mesos/bin/python
> Installing setuptools, pip, wheel...done.
> ?  ~  pip install protobuf   
> Collecting protobuf
> Requirement already satisfied (use --upgrade to upgrade): setuptools in 
> ./.virtualenvs/mesos/lib/python2.7/site-packages (from protobuf)
> Installing collected packages: protobuf
> Successfully installed protobuf-2.6.1
> ?  ~  python
> Python 2.7.10 (default, May 26 2015, 04:16:29) 
> [GCC 5.1.0] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import google.protobuf
> >>> 
> ?  ~  pip freeze
> protobuf==2.6.1
> wheel==0.24.0
> ?  ~  ls -l ./.virtualenvs/mesos/lib/python2.7/site-packages/google 
> total 4
> drwxr-xr-x 5 seb seb 4096 Aug  5 04:18 protobuf
> ```
> 
> No need for the google-common package

If you already install protobuf, you could install this success. But if you 
don't install protobuf before, You would get error. Because we mesos.interface 
depends on protobuf

Re: Review Request 37081: Let init.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.



> On Aug. 5, 2015, 9:57 a.m., Sebastien Pahl wrote:
> > src/python/interface/setup.py.in, line 27
> > 
> >
> > This seems weird to me. Why do you need to add the bindings for the 
> > google search engine?
> > 
> > https://pypi.python.org/pypi/google
> 
> haosdent huang wrote:
> Yes, I think it is also a bit dirty here.
> 
> Because I try to fix this, could you see @marco comment above. I don't 
> have other ideas to let it contains `__init__.py` under google, so I add this 
> dependency.
> 
> ```
> ok - this almost worked :)
> Are we missing an __init__.py in google?
> 
> 
> $ ../configure --prefix=/Users/marco/mesos-inst/lib
> 
> then the usual make && make install - then:
> 
> $ find . |grep __init__
> ./lib/python/site-packages/google/protobuf/__init__.py
> ./lib/python/site-packages/google/protobuf/__init__.pyc
> ./lib/python/site-packages/google/protobuf/compiler/__init__.py
> ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc
> ./lib/python/site-packages/google/protobuf/internal/__init__.py
> ./lib/python/site-packages/google/protobuf/internal/__init__.pyc
> ./lib/python/site-packages/google/protobuf/pyext/__init__.py
> ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc
> ./lib/python/site-packages/mesos/__init__.py
> ./lib/python/site-packages/mesos/__init__.pyc
> ./lib/python/site-packages/mesos/interface/__init__.py
> ./lib/python/site-packages/mesos/interface/__init__.pyc
> ./lib/python/site-packages/mesos/native/__init__.py
> ./lib/python/site-packages/mesos/native/__init__.pyc
> ./libexec/mesos/python/mesos/__init__.py
> 
> as you can see, there are now the __init__.py files in the right places 
> under mesos (if you do the same from master there's nothing in there - so, 
> yay! :) but there is none under the google/ package:
> 
> [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/
> 
> [~/mesos-inst/lib] $ python
> Python 2.7.6 (default, Sep  9 2014, 15:04:36) 
> [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import mesos
> >>> from mesos import native
> Traceback (most recent call last):
>   File "", line 1, in 
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py",
>  line 17, in 
> from ._mesos import MesosExecutorDriverImpl
>   File 
> "/Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py",
>  line 4, in 
> from google.protobuf.internal import enum_type_wrapper
> ImportError: No module named google.protobuf.internal
> >>> import google.protobuf.internal
> Traceback (most recent call last):
>   File "", line 1, in 
> ImportError: No module named google.protobuf.internal
> 
> This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an 
> Ubuntu 14.04 VM, brand new.
> ```
> 
> haosdent huang wrote:
> For openstack, we could find `python-openstack-common` which contains 
> `__init__.py` under [root 
> namespace](http://packages.ubuntu.com/precise/i386/python-openstack-common/filelist),
>  althrough openstack have multi submodules. But seems google don't provide 
> package like this.
> 
> haosdent huang wrote:
> I upload a `__init__.py` to https://pypi.python.org/pypi/google-common, 
> and change dependcy to this. But I think it is also tricky here.

I can't seem to reproduce your problem.

Here is what I get:
```
?  ~  mkvirtualenv -p python2.7 mesos
Running virtualenv with interpreter /usr/bin/python2.7
New python executable in mesos/bin/python2.7
Also creating executable in mesos/bin/python
Installing setuptools, pip, wheel...done.
?  ~  pip install protobuf   
Collecting protobuf
Requirement already satisfied (use --upgrade to upgrade): setuptools in 
./.virtualenvs/mesos/lib/python2.7/site-packages (from protobuf)
Installing collected packages: protobuf
Successfully installed protobuf-2.6.1
?  ~  python
Python 2.7.10 (default, May 26 2015, 04:16:29) 
[GCC 5.1.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import google.protobuf
>>> 
?  ~  pip freeze
protobuf==2.6.1
wheel==0.24.0
?  ~  ls -l ./.virtualenvs/mesos/lib/python2.7/site-packages/google 
total 4
drwxr-xr-x 5 seb seb 4096 Aug  5 04:18 protobuf
```

No need for the google-common package


- Sebastien


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


On Aug. 5, 2015, 10:29 a.m., haosdent huang wrote:
> 
> ---
> This is an auto

Re: Review Request 37127: Added framework authorization for dynamic reservation.


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

(Updated Aug. 5, 2015, 11:17 a.m.)


Review request for mesos, Adam B and Jie Yu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 

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


Testing
---

Added tests to `src/tests/reservation_tests.cpp` + `make check`


Thanks,

Michael Park

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.


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

(Updated Aug. 5, 2015, 11:15 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
Van Remoortere, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 
3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.