date:20161116

Re: Review Request 52544: Introduced `int_fd` class.

2016-11-16 Thread Daniel Pravat


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

(Updated Nov. 17, 2016, 7:09 a.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
Park.


Repository: mesos


Description
---

In POSIX the `socket`,`pipe` and the `filedescriptor` are
represented by an int type. In Windows a socket is kept in a
`SOCKET` type (64 bit wide), a pipe in a `HANDLE` (64 bit wide) and
a file descriptor in an int. This class unifies all Windows types.
In POSIX this class is an int.


Diffs (updated)
-

  3rdparty/stout/include/stout/os.hpp bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
  3rdparty/stout/include/stout/os/filedescriptor.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/filedescriptor.hpp PRE-CREATION 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52544: Introduced `int_fd` class.

2016-11-16 Thread Daniel Pravat



> On Nov. 17, 2016, 12:57 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 416-440
> > 
> >
> > What is this used for?

This is called from the function below. It is used when we create two pipes 
int_fd(s) holding anonimous pipes as the Posix pipe API. I think it can be 
avoided and default to a socket<->pipe model.


> On Nov. 17, 2016, 12:57 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 356-379
> > 
> >
> > Do we actually need this for something..?

There are several places where the int_fd is used in the logs. Hence '<<'. 
Furthermore the handles are serialized for the containerizer command line and 
are deserialized in the containerizer.


> On Nov. 17, 2016, 12:57 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, line 148
> > 
> >
> > What is `NONE` for? it doesn't seem like we use it?

None is used when pipe are used on both legs of the adapter.


> On Nov. 17, 2016, 12:57 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 36-38
> > 
> >
> > What are these for?

closed is used to detect on Mesos code a leaked. Also the IO from os:: 
namespace may use this flag to reject the operation on a closed handle. 
Otherwise we see strange error deep in CRT.


- Daniel


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


On Nov. 16, 2016, 6:37 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52544/
> ---
> 
> (Updated Nov. 16, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In POSIX the `socket`,`pipe` and the `filedescriptor` are
> represented by an int type. In Windows a socket is kept in a
> `SOCKET` type (64 bit wide), a pipe in a `HANDLE` (64 bit wide) and
> a file descriptor in an int. This class unifies all Windows types.
> In POSIX this class is an int.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
>   3rdparty/stout/include/stout/os/filedescriptor.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/filedescriptor.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52544/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Review Request 53842: Add role specific metrics for sorting runs.

2016-11-16 Thread Anindya Sinha


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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added the following 2 metrics which is maintained on a role level
(as long as there is at least one framework of that role):
a) allocator/mesos/frameworks/role//sort_runs: Number of framework
   level sorts (based on role) in DRF Sorter.
b) allocator/mesos/frameworks/role//sort_run: Latency in framework
   level sorts (based on role) in DRF Sorter.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
c8f9492ee1b69e125a1e841116d22a578a9b524e 
  src/master/allocator/mesos/metrics.hpp 
753f90acc52ada84883cbbe3350e61d1e1eaff48 
  src/master/allocator/mesos/metrics.cpp 
a36d21c297160bc1c9f43a22743fd5448d7ae5ac 
  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 53841: Added metrics for sorting of the sorters in the allocator.

2016-11-16 Thread Anindya Sinha


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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Following metrics have been added:
a) allocator/mesos/roles/sort_runs: Number of role/quota level sorts
   in DRF Sorter.
b) allocator/mesos/roles/sort_run: Latency in role/quota level sorts
   in DRF Sorter.
c) allocator/mesos/frameworks/sort_runs: Number of framework level sorts
   (across all roles) in DRF Sorter.
d) allocator/mesos/frameworks/sort_run: Latency in framework level sorts
   (across all roles) in DRF Sorter.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
c8f9492ee1b69e125a1e841116d22a578a9b524e 
  src/master/allocator/mesos/metrics.hpp 
753f90acc52ada84883cbbe3350e61d1e1eaff48 
  src/master/allocator/mesos/metrics.cpp 
a36d21c297160bc1c9f43a22743fd5448d7ae5ac 
  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 53840: Added a metric in the allocator to track previous allocation cycle.

2016-11-16 Thread Anindya Sinha


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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added a metric 'allocator/mesos/last_allocation_secs_since' which
indicates how long back (in seconds) did the previous allocation
cycle run. If no allocation cycle has run yet, it has a value of -1.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
2c31471ee0f5d6836393bf87ff9ecfd8df835013 
  src/master/allocator/mesos/hierarchical.cpp 
c8f9492ee1b69e125a1e841116d22a578a9b524e 
  src/master/allocator/mesos/metrics.hpp 
753f90acc52ada84883cbbe3350e61d1e1eaff48 
  src/master/allocator/mesos/metrics.cpp 
a36d21c297160bc1c9f43a22743fd5448d7ae5ac 
  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 53839: Added a metric in the master to track sending of previous offer.

2016-11-16 Thread Anindya Sinha


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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added a metric 'master/last_offer_secs_since' which indicates how long
back did the master sent the last offer to any framework. This metric
is in seconds. If no offer has been sent by the master, it has a
value of -1.


Diffs
-

  src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
  src/master/metrics.hpp 056d290c1e14a6afd5056873ef7bb07b5892ed32 
  src/master/metrics.cpp 1f049f3794c1bca45d2684cbbec3b08c1a78c494 
  src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Review Request 53837: Added a per container mesos-io-switchboard process.

2016-11-16 Thread Kevin Klues


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

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


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


Repository: mesos


Description
---

Currently, this process simply intercepts the stdout/stderr of a
container and writes it to the stdout/stderr FDs set up by the
container logger. In future commits, this process will be expanded to
allow dynamically attaching new input / output sources to a container
to support `docker attach` like functionality for a running container.


Diffs
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/slave/containerizer/mesos/containerizer.cpp 
ec4ae32485a7ab6c9f73c512004d1220482a188e 
  src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp PRE-CREATION 
  src/slave/containerizer/mesos/io_switchboard/main.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io_switchboard/main.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check
sudo src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 53704: Added a level of indirection for logger through IO Switchboard.

2016-11-16 Thread Kevin Klues


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

(Updated Nov. 17, 2016, 4:49 a.m.)


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


Changes
---

Updated to move out-of-order `#include`.


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


Repository: mesos


Description
---

We are adding a new component to the mesos containerizer called
IOSwitchboard. The purpose of this component is to feed stdin to a
container from an external source, as well as redirect the
stdin/stdout of a container to multiple targets.

The primary motivation of this component is to enable support in mesos
similar to 'docker attach' and 'docker exec' whereby an external
client can attach to the stdin/stdout/stderr of a running container as
well as launch arbitrary subcommands inside a container and attach to
its stdin/stdout/stderr.

to enable this functionatlity, we also introduce a new
IOSwitchboardProcess in this commit (although it is not strictly
necessary). We will be adding additional functionality that relies on
it in subsequent commits.


Diffs (updated)
-

  src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/slave/containerizer/mesos/containerizer.hpp 
272052ddf85b50f817a110a9a83566b011598985 
  src/slave/containerizer/mesos/containerizer.cpp 
ec4ae32485a7ab6c9f73c512004d1220482a188e 
  src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp PRE-CREATION 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
2f21b49535856186e153cd299dd1eda11495fa17 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
5aae23b1b470d5323ecc21fb5df7ad8ae2498dfa 

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


Testing
---

GTEST_FILTER="" make -j check
sudo src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 53475: Use explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [53474, 53475]

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

- Mesos ReviewBot


On Nov. 16, 2016, 10:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53475/
> ---
> 
> (Updated Nov. 16, 2016, 10:27 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass the error code down to the Error object explicitly, rather than
> setting the per-thread error and using it implicitly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 21f878ee81db32ad35878ec053c3f2de3637196c 
>   3rdparty/libprocess/src/poll_socket.cpp 
> f0ee1490e6fccb038f64a27b2c71458ad5b5e5a1 
> 
> Diff: https://reviews.apache.org/r/53475/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52544: Introduced `int_fd` class.

2016-11-16 Thread Michael Park


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




3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (line 33)


`s/adaptor/translator/`



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 36 - 38)


What are these for?



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (line 148)


What is `NONE` for? it doesn't seem like we use it?



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 356 - 379)


Do we actually need this for something..?



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 416 - 440)


What is this used for?


- Michael Park


On Nov. 16, 2016, 10:37 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52544/
> ---
> 
> (Updated Nov. 16, 2016, 10:37 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In POSIX the `socket`,`pipe` and the `filedescriptor` are
> represented by an int type. In Windows a socket is kept in a
> `SOCKET` type (64 bit wide), a pipe in a `HANDLE` (64 bit wide) and
> a file descriptor in an int. This class unifies all Windows types.
> In POSIX this class is an int.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
>   3rdparty/stout/include/stout/os/filedescriptor.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/filedescriptor.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52544/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 53538: Tweaked parameter name in libprocess.

2016-11-16 Thread Michael Park



> On Nov. 7, 2016, 7:56 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/include/process/time.hpp, line 105
> > 
> >
> > Could you please follow-up with a patch fixing the names of the 
> > `std::ostream`s in the code? We typically use `std::ostream& stream`.

This is tackled in https://reviews.apache.org/r/53547


- Michael


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


On Nov. 7, 2016, 7:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53538/
> ---
> 
> (Updated Nov. 7, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure consistency between declaration and definition. Spotted via
> clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/time.hpp 
> 554b291bc03d272fc92568f44da72ca7fcfe8838 
> 
> Diff: https://reviews.apache.org/r/53538/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note that there are lots of other mismatches in names between declarations 
> and definitions, but most of the ones I didn't fix seem to be intentional: 
> e.g., declaring a parameter named `t` but using `t_` in the definition 
> (typically because the implementation assigns `t_` to a member field named 
> `t`).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 53832: Fixing broken link for the app frameowrks development guide page.

2016-11-16 Thread Miguel Bernadin


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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Fixing broken link for the app frameowrks development guide. 
http://mesos.apache.org/documentation/latest/app-framework-development-guide/


Diffs
-

  docs/app-framework-development-guide.md 
de92c7570cd0d0ac8639ce50a79e5158844ac53c 

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


Testing
---

Validated the files exist locally and contained the correct name. Ensured the 
correction followed the existing standard.


Thanks,

Miguel Bernadin

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 17, 2016, 12:27 a.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
   there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
   EOF event was received before all sent data was read.

This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing
---

Testing details found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 53548: Fixed `operator<<` parameter naming in stout.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 9:42 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53548/
> ---
> 
> (Updated Nov. 7, 2016, 9:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By convention, we use `stream` for the first parameter to ostream
> `operator<<` implementations.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/proc.hpp 
> dd0d623e789b428d21fb98300e124fcc9288001e 
>   3rdparty/stout/include/stout/version.hpp 
> 5ae1c73d1b32d09e8a32e0a5df17ebfa2fe7c4bc 
> 
> Diff: https://reviews.apache.org/r/53548/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53549: Fixed `operator<<` parameter naming in Mesos.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 9:42 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53549/
> ---
> 
> (Updated Nov. 7, 2016, 9:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By convention, we use `stream` for the first parameter to ostream
> `operator<<` implementations.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/handle.hpp bafeec2509f76c61382549c8d47ff9c5c10ec4cb 
>   src/linux/routing/handle.cpp 3d8d2f147a1cefb3341a52de104eac25220792a6 
>   src/tests/common/recordio_tests.cpp 
> 3d14953ebb95ff26f1d91f7ddec2660c21ba005f 
> 
> Diff: https://reviews.apache.org/r/53549/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53474: Support explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread Michael Park



> On Nov. 16, 2016, 2:25 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/windows/error.hpp, lines 108-110
> > 
> >
> > I think we get these inherited from `WindowsErrorBase`, right?
> 
> James Peach wrote:
> Also need to add `WindowsSocketError(DWORD _code)` for consistency.

My bad about the "implicitly inherited"-ness of constructors. I was incorrect 
there. Discussed doing `using WindowsErrorBase::WindowsErrorBase;` to inherit 
the base constructors, but ultimately decided to declare them explicitly. This 
way it also looks consistent with the `ErrnoError` declarations.


- Michael


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


On Nov. 4, 2016, 4:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53474/
> ---
> 
> (Updated Nov. 4, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support explicit error codes in ErrnoError and SocketError.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/errorbase.hpp 
> 96d395d37cdad706c2f23fdd9caeefc2a33e0541 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 5657b5a18aaedf842b7b780ea7eada73118788cb 
>   3rdparty/stout/include/stout/windows/error.hpp 
> f296b82c47ef5ae36f6e5ee4dd289cb15bd200a6 
>   3rdparty/stout/tests/error_tests.cpp 
> 25b50141dd9bbf163aa816750210b298456740a8 
> 
> Diff: https://reviews.apache.org/r/53474/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53537: Avoided unnecessary copies in Mesos.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 11:32 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53537/
> ---
> 
> (Updated Nov. 7, 2016, 11:32 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted via clang-tidy.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
>   src/module/manager.cpp 18ee65796dd5744d36a645e7ca1f5ab0fbce98a5 
>   src/slave/http.cpp 381d9c3ca0e6f61e8649603cbf770b8f01de4bf9 
>   src/tests/containerizer.cpp c3bcb85d245a0e95b377059802cd89617eb5e25c 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> b5797574ca2bfe3a005c80a3ba6bb6965c54cc95 
>   src/tests/health_check_tests.cpp e473c98da8dfeb0e05b9dec7d4be1ce227510cf5 
> 
> Diff: https://reviews.apache.org/r/53537/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53536: Avoided unnecessary copy in stout.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 7:24 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53536/
> ---
> 
> (Updated Nov. 7, 2016, 7:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted via clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os_tests.cpp 0b7ee0723b6a608d6f110fa8ac16e0fd7b75ddea 
> 
> Diff: https://reviews.apache.org/r/53536/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

2016-11-16 Thread Zameer Manji


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


Ship it!




I'm not a mesos committer but this LGTM.

- Zameer Manji


On Nov. 16, 2016, 3:04 p.m., Vijay Srinivasaraghavan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53825/
> ---
> 
> (Updated Nov. 16, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zameer Manji.
> 
> 
> Bugs: MESOS-6597
> https://issues.apache.org/jira/browse/MESOS-6597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled java protos generation for all V1 proto files to support invoking 
> operator HTTP API from a standard java based Rest client.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
> 
> Diff: https://reviews.apache.org/r/53825/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vijay Srinivasaraghavan
> 
>

Re: Review Request 53540: Tweaked parameter names in Mesos.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 8:15 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53540/
> ---
> 
> (Updated Nov. 7, 2016, 8:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure consistency between declaration and definition. Spotted via
> clang-tidy.
> 
> 
> Diffs
> -
> 
>   include/mesos/state/protobuf.hpp 47beb45ecb9d3486e9f8f8ce38d48264f512107d 
>   src/common/http.hpp c2398177c9e5af12c7a20c02e92d3f3036a7a39a 
>   src/common/protobuf_utils.cpp 7362b875ce1ffca6bc6376169a11494bdb9cf062 
>   src/linux/capabilities.hpp 9c0bcf46c1bf8435eabb7410961ce52828ffbfea 
>   src/linux/capabilities.cpp 3e30c4d8966246778d59245794885139e9d14dfe 
>   src/log/coordinator.hpp b5f9c962a983af0d591e878d12b7fe8bb59cc867 
>   src/master/master.hpp 87186c6e733a686f96528b1722fda1c287e9c881 
>   src/master/master.cpp 15ad5a647fc71d36ffd149893b3b9448a7d99323 
>   src/slave/slave.hpp c0a17657e684e87d555731d2e35ac15894c3c05b 
>   src/slave/slave.cpp d6c337345707993b0729e9eaf36b5a9ecc52dc72 
>   src/tests/utils.cpp fc004a9c567898ffbc38a42cc2340dd57347a829 
> 
> Diff: https://reviews.apache.org/r/53540/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note that there are lots of other mismatches in names between declarations 
> and definitions, but most of the ones I didn't fix seem to be intentional: 
> e.g., declaring a parameter named `t` but using `t_` in the definition 
> (typically because the implementation assigns `t_` to a member field named 
> `t`).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53817: Added missing cleanup to libprocess `finalize()`.

2016-11-16 Thread Greg Mann



> On Nov. 16, 2016, 8:05 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 1221-1222
> > 
> >
> > It does look like this is a leak, but due to joining and recreating the 
> > thread pool between reinitializations, the `__executor__` will be recreated.
> > 
> > Deleting a thread local variable like this will only delete one 
> > instance of it, whereas there could be one instance per thread.  We should 
> > consider changing the `_executor_` to an `Owned<>` value.

Per our discussion, the `__executor__` in the main thread is not getting 
recreated, leading to the failure of `ProcessTest.Defer3` when run in 
repetition. We can add a `delete` to `process::finalize()` to solve this.

We should also add a `delete` statement into the worker threads themselves to 
prevent the `_executor_` pointers from leaking across reinitializations.


- Greg


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


On Nov. 16, 2016, 7:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53817/
> ---
> 
> (Updated Nov. 16, 2016, 7:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `finalize()` function in libprocess previously did
> not delete the thread-local `Executor` which is used to
> `defer` execution to an unspecified context. This object
> must be deleted during finalization so that the
> `__executor__` macro creates a new process if libprocess
> is subsequently reinitialized.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53817/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

2016-11-16 Thread Vijay Srinivasaraghavan


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

(Updated Nov. 16, 2016, 11:02 p.m.)


Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description (updated)
---

Enabled java protos generation for all V1 proto files to support invoking 
operator HTTP API from a standard java based Rest client.


Diffs
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 

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


Testing
---


Thanks,

Vijay Srinivasaraghavan

Re: Review Request 53538: Tweaked parameter name in libprocess.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 7:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53538/
> ---
> 
> (Updated Nov. 7, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure consistency between declaration and definition. Spotted via
> clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/time.hpp 
> 554b291bc03d272fc92568f44da72ca7fcfe8838 
> 
> Diff: https://reviews.apache.org/r/53538/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note that there are lots of other mismatches in names between declarations 
> and definitions, but most of the ones I didn't fix seem to be intentional: 
> e.g., declaring a parameter named `t` but using `t_` in the definition 
> (typically because the implementation assigns `t_` to a member field named 
> `t`).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53474: Support explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread James Peach



> On Nov. 16, 2016, 10:25 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/windows/error.hpp, lines 108-110
> > 
> >
> > I think we get these inherited from `WindowsErrorBase`, right?

Also need to add `WindowsSocketError(DWORD _code)` for consistency.


- James


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


On Nov. 4, 2016, 11:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53474/
> ---
> 
> (Updated Nov. 4, 2016, 11:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support explicit error codes in ErrnoError and SocketError.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/errorbase.hpp 
> 96d395d37cdad706c2f23fdd9caeefc2a33e0541 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 5657b5a18aaedf842b7b780ea7eada73118788cb 
>   3rdparty/stout/include/stout/windows/error.hpp 
> f296b82c47ef5ae36f6e5ee4dd289cb15bd200a6 
>   3rdparty/stout/tests/error_tests.cpp 
> 25b50141dd9bbf163aa816750210b298456740a8 
> 
> Diff: https://reviews.apache.org/r/53474/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53539: Tweaked parameter name in stout.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2016, 8:44 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53539/
> ---
> 
> (Updated Nov. 7, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure consistency between declaration and definition. Spotted via
> clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
> 
> Diff: https://reviews.apache.org/r/53539/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note that there are lots of other mismatches in names between declarations 
> and definitions, but most of the ones I didn't fix seem to be intentional: 
> e.g., declaring a parameter named `t` but using `t_` in the definition 
> (typically because the implementation assigns `t_` to a member field named 
> `t`).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

2016-11-16 Thread Vijay Srinivasaraghavan


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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

MESOS-6597 Enabled java protos generation for all V1 proto files.


Diffs
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 

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


Testing
---


Thanks,

Vijay Srinivasaraghavan

Re: Review Request 53474: Support explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread Michael Park



> On Nov. 16, 2016, 2:25 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/posix/rmdir.hpp, lines 57-58
> > 
> >
> > This is great!
> > 
> > It seems like we have more opportunities to update here:
> > 
> > ```
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp
> > 166:  errno = ENOTCONN;
> > 167-  return ErrnoError();
> > ```
> > ```
> > 3rdparty/libprocess/src/poll_socket.cpp
> > 138-#ifdef __WINDOWS__
> > 139-::WSASetLastError(opt);
> > 140-#else
> > 141:errno = opt;
> > 142-#endif
> > 143-
> > 144-return Failure(SocketError("Failed to connect to " + 
> > stringify(to)));
> > ```
> > ```
> > 3rdparty/stout/include/stout/windows/os.hpp
> > 254:errno = ENOSYS;
> > 255-return ErrnoError(
> > 261:errno = ENOSYS;
> > 262-return ErrnoError(
> > 294:errno = ECHILD;
> > 295-return WindowsError(
> > 305:errno = ECHILD;
> > 306-return WindowsError(
> > 324:errno = ECHILD;
> > 325-return WindowsError(
> > ```
> > ```
> > 3rdparty/stout/include/stout/os/windows/su.hpp
> > 55:  SetLastError(ERROR_NOT_SUPPORTED);
> > 56-  return WindowsError();
> > ```
> > ```
> > 3rdparty/stout/include/stout/windows/fs.hpp
> > 122:::SetLastError(error);
> > 123-return WindowsError(
> > 124-"'fs::list': 'FindNextFile' failed when searching for files 
> > with "
> > 125-"'pattern '" + pattern + "'");
> > ```

Ah, I see that the first two `libevent_ssl_socket.cpp` and `poll_socket.cpp` 
are covered by https://reviews.apache.org/r/53475/.


- Michael


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


On Nov. 4, 2016, 4:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53474/
> ---
> 
> (Updated Nov. 4, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support explicit error codes in ErrnoError and SocketError.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/errorbase.hpp 
> 96d395d37cdad706c2f23fdd9caeefc2a33e0541 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 5657b5a18aaedf842b7b780ea7eada73118788cb 
>   3rdparty/stout/include/stout/windows/error.hpp 
> f296b82c47ef5ae36f6e5ee4dd289cb15bd200a6 
>   3rdparty/stout/tests/error_tests.cpp 
> 25b50141dd9bbf163aa816750210b298456740a8 
> 
> Diff: https://reviews.apache.org/r/53474/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53475: Use explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread Michael Park


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


Ship it!




Ship It!

- Michael Park


On Nov. 4, 2016, 8:04 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53475/
> ---
> 
> (Updated Nov. 4, 2016, 8:04 a.m.)
> 
> 
> Review request for mesos and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass the error code down to the Error object explicitly, rather than
> setting the per-thread error and using it implicitly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 21f878ee81db32ad35878ec053c3f2de3637196c 
>   3rdparty/libprocess/src/poll_socket.cpp 
> f0ee1490e6fccb038f64a27b2c71458ad5b5e5a1 
> 
> Diff: https://reviews.apache.org/r/53475/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53474: Support explicit error codes in ErrnoError and SocketError.

2016-11-16 Thread Michael Park


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


Fix it, then Ship it!





3rdparty/stout/include/stout/errorbase.hpp (lines 51 - 55)


Can we flip these? Above, we have the ctor that uses `errno` implicitly 
followed by the one that takes an explicit error code.



3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 57)


This is great!

It seems like we have more opportunities to update here:

```
3rdparty/libprocess/src/libevent_ssl_socket.cpp
166:  errno = ENOTCONN;
167-  return ErrnoError();
```
```
3rdparty/libprocess/src/poll_socket.cpp
138-#ifdef __WINDOWS__
139-::WSASetLastError(opt);
140-#else
141:errno = opt;
142-#endif
143-
144-return Failure(SocketError("Failed to connect to " + 
stringify(to)));
```
```
3rdparty/stout/include/stout/windows/os.hpp
254:errno = ENOSYS;
255-return ErrnoError(
261:errno = ENOSYS;
262-return ErrnoError(
294:errno = ECHILD;
295-return WindowsError(
305:errno = ECHILD;
306-return WindowsError(
324:errno = ECHILD;
325-return WindowsError(
```
```
3rdparty/stout/include/stout/os/windows/su.hpp
55:  SetLastError(ERROR_NOT_SUPPORTED);
56-  return WindowsError();
```
```
3rdparty/stout/include/stout/windows/fs.hpp
122:::SetLastError(error);
123-return WindowsError(
124-"'fs::list': 'FindNextFile' failed when searching for files 
with "
125-"'pattern '" + pattern + "'");
```



3rdparty/stout/include/stout/windows/error.hpp (lines 108 - 110)


I think we get these inherited from `WindowsErrorBase`, right?



3rdparty/stout/include/stout/windows/error.hpp (lines 120 - 124)


I think we get these inherited from `WindowsErrorBase`, right?


- Michael Park


On Nov. 4, 2016, 4:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53474/
> ---
> 
> (Updated Nov. 4, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6520
> https://issues.apache.org/jira/browse/MESOS-6520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support explicit error codes in ErrnoError and SocketError.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/errorbase.hpp 
> 96d395d37cdad706c2f23fdd9caeefc2a33e0541 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 5657b5a18aaedf842b7b780ea7eada73118788cb 
>   3rdparty/stout/include/stout/windows/error.hpp 
> f296b82c47ef5ae36f6e5ee4dd289cb15bd200a6 
>   3rdparty/stout/tests/error_tests.cpp 
> 25b50141dd9bbf163aa816750210b298456740a8 
> 
> Diff: https://reviews.apache.org/r/53474/diff/
> 
> 
> Testing
> ---
> 
> make check on Fedora 24
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53473: Add new param user to logrotate's prepare function.

2016-11-16 Thread Sivaram Kannan



> On Nov. 15, 2016, 2:42 p.m., Sivaram Kannan wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1388
> > 
> >
> > Passing None() has the agent crashing when we have switch_user=false.
> 
> Sivaram Kannan wrote:
> That is the reason I thought I would have the current user of the agent 
> set to logger.
> 
> Joseph Wu wrote:
> The reason the agent/test would fail is because of this line in your 
> fourth diff:
> https://reviews.apache.org/r/53473/diff/4#7
> 
> `container->config.user()` has a return type of `string`.  When the value 
> is not set, calling `container->config.user()` returns the empty string.  You 
> need to manually convert it to an `Option`.
> 
> `container->config` is a protobuf defined here: 
> 
> https://github.com/apache/mesos/blob/1.1.x/include/mesos/slave/containerizer.proto#L112
> 
> Sivaram Kannan wrote:
> https://reviews.apache.org/r/53473/diff/6/ has an explicit conversion to 
> Option. When container->config.user() is empty, I tried to set 
> 
> Option user = None()
> 
> But the call logger->prepare(..., user), still crashes the agent. Doesn't 
> the below code more logical than setting the value to None()?
> 
> Result result = os::user();
> user = result.get();
> 
> Joseph Wu wrote:
> What's the stack trace for the crash?  (And what are the steps taken to 
> repro?)
> 
> Sivaram Kannan wrote:
> Sorry - should have added when I created the RBR. 
> 
> Steps to reproduce: 
> 1. In the containerizer.cpp, change the below code in the rbr 
> 2.   Option user = None();
>   if (container->config.has_user()) {
> user = container->config.user();
>   } else {
> Result result = os::user();
> //user = result.get(); <= Current code
> user = None(); <=== Change it to this
>   }
>  3. make and make check. 
>  4. Run the below test
>  5. sudo bin/mesos-tests.sh --verbose 
> --gtest_filter="UserContainerLoggerTest.ROOT_LOGROTATE_RotateAsSwitchUserFalse"
>  6. The above test would end up in a crash like below.
>  7. Stack Trace:
>  8. I1116 15:58:16.660096 15282 slave.cpp:2031] Queued task 
> 'bffed9be-9830-4840-a313-e2ff3741b2fc' for executor 
> 'bffed9be-9830-4840-a313-e2ff3741b2fc' of framework 
> 16b6e12e-3344-4aad-adf1-a3460b3ad2a1-
> lt-mesos-tests: ../../3rdparty/stout/include/stout/option.hpp:111: const 
> T& Option::get() const & [with T = std::basic_string]: Assertion 
> `isSome()' failed.
> *** Aborted at 1479311896 (unix time) try "date -d @1479311896" if you 
> are using GNU date ***
> PC: @ 0x7f89e65b0c37 (unknown)
> *** SIGABRT (@0x3ba2) received by PID 15266 (TID 0x7f89dd0db700) from PID 
> 15266; stack trace: ***
> @ 0x7f89e694f330 (unknown)
> @ 0x7f89e65b0c37 (unknown)
> @ 0x7f89e65b4028 (unknown)
> @ 0x7f89e65a9bf6 (unknown)
> @ 0x7f89e65a9ca2 (unknown)
> @   0xa8542f _ZNKR6OptionISsE3getEv
> @ 0x7f89dfba6a9b 
> mesos::internal::logger::LogrotateContainerLoggerProcess::prepare()
> @ 0x7f89dfbbcb5e 
> _ZZN7process8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS1_8internal6logger31LogrotateContainerLoggerProcessERKNS1_12ExecutorInfoERKSsRK6OptionISsES8_SsSE_EENS_6FutureIT_EERKNS_3PIDIT0_EEMSL_FSJ_T1_T2_T3_ET4_T5_T6_ENKUlPNS_11ProcessBaseEE_clESY_
> @ 0x7f89dfbcb54d 
> _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS5_8internal6logger31LogrotateContainerLoggerProcessERKNS5_12ExecutorInfoERKSsRK6OptionISsESC_SsSI_EENS0_6FutureIT_EERKNS0_3PIDIT0_EEMSP_FSN_T1_T2_T3_ET4_T5_T6_EUlS2_E_E9_M_invokeERKSt9_Any_dataS2_
> @ 0x7f89eca9168d std::function<>::operator()()
> @ 0x7f89eca73e0d process::ProcessBase::visit()
> @ 0x7f89eca7c2ec process::DispatchEvent::visit()
> @   0xa674b2 process::ProcessBase::serve()
> @ 0x7f89eca700fa process::ProcessManager::resume()
> @ 0x7f89eca6cc39 
> _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv
> @ 0x7f89eca7ba94 
> _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIIEEEvSt12_Index_tupleIIXspT_EEE
> @ 0x7f89eca7b9eb 
> _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv
> @ 0x7f89eca7b984 
> _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv
> @ 0x7f89e712aa60 (unknown)
> @ 0x7f89e6947184 start_thread
> @ 0x7f89e667437d (unknown)
> 
> Joseph Wu wrote:
> With your latest review diff, this stack trace shouldn't be possible, as 
> there is no code in the `LogrotateContainerLoggerProcess` that calls 
> `user.get()`.  Can you

Re: Review Request 53487: Wired the libprocess code to use the streaming decoder.

2016-11-16 Thread Joseph Wu



> On Nov. 11, 2016, 1:27 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2710-2713
> > 
> >
> > It looks like this may be introducing another problematic path for 
> > libprocess finalization, in that your continuation may run after libprocess 
> > is finalized and it expects 'this' to be valid.
> > 
> > It would be great to check with Joesph Wu about this path being 
> > introduced since he has worked on libprocess finalization, and see if he 
> > has any input.

There shouldn't be much of an impact to finalization.  

When the process manager is finalized (but not deleted yet), it will close all 
existing HTTP connections and join all worker threads.  That means the 
continuation to `parse(*request)` can only run asynchronously before the 
`process_manager` is finalized, or synchronously while the 
`process_manager`/`socket_manager` are finalized.  In both of those cases, the 
`process_manager` pointer is still valid.


- Joseph


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


On Nov. 11, 2016, 11:57 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53487/
> ---
> 
> (Updated Nov. 11, 2016, 11:57 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6466
> https://issues.apache.org/jira/browse/MESOS-6466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old libprocess style messages and routes not supporting request
> streaming read the body from the piped reader. Otherwise, the
> request is forwarded to the handler when the route supports
> streaming.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/process.hpp 
> f389d3d3b671e301a7ac911ad87ab13289e8c82f 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53487/diff/
> 
> 
> Testing
> ---
> 
> make check (Tests are added in https://reviews.apache.org/r/53490 i.e., after 
> we add support to the `Connection` abstraction for request streaming)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53817: Added missing cleanup to libprocess `finalize()`.

2016-11-16 Thread Joseph Wu


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




3rdparty/libprocess/src/process.cpp (lines 1221 - 1222)


It does look like this is a leak, but due to joining and recreating the 
thread pool between reinitializations, the `__executor__` will be recreated.

Deleting a thread local variable like this will only delete one instance of 
it, whereas there could be one instance per thread.  We should consider 
changing the `_executor_` to an `Owned<>` value.


- Joseph Wu


On Nov. 16, 2016, 11:07 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53817/
> ---
> 
> (Updated Nov. 16, 2016, 11:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `finalize()` function in libprocess previously did
> not delete the thread-local `Executor` which is used to
> `defer` execution to an unspecified context. This object
> must be deleted during finalization so that the
> `__executor__` macro creates a new process if libprocess
> is subsequently reinitialized.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53817/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53741: Display maintainance info in the webui.

2016-11-16 Thread Tomasz Janiszewski


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

(Updated Nov. 16, 2016, 7:54 p.m.)


Review request for mesos, haosdent huang and Joseph Wu.


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


Repository: mesos


Description
---

Create new page with Maintenance schedule. Schedule is downloaded on
page refresh. Schedule is not live like stats and tasks so there is
no need to poll it periodically.
Diable sorting when data-key is not defined in table header.


Diffs (updated)
-

  src/webui/master/static/index.html 6211892e7d689df9bf3b2a9071a76ad4c60d0485 
  src/webui/master/static/js/app.js c32177aa23c962f2bdf03d98272fb6d21a565382 
  src/webui/master/static/js/controllers.js 
dd0cc810748a9bd378d9c6b15e9fe89b7c0f11d9 
  src/webui/master/static/maintenance.html PRE-CREATION 

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


Testing
---

[Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)

Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
entires schedule generated with 
[generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)


Thanks,

Tomasz Janiszewski

Re: Review Request 53473: Add new param user to logrotate's prepare function.

2016-11-16 Thread Joseph Wu



> On Nov. 15, 2016, 6:42 a.m., Sivaram Kannan wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1388
> > 
> >
> > Passing None() has the agent crashing when we have switch_user=false.
> 
> Sivaram Kannan wrote:
> That is the reason I thought I would have the current user of the agent 
> set to logger.
> 
> Joseph Wu wrote:
> The reason the agent/test would fail is because of this line in your 
> fourth diff:
> https://reviews.apache.org/r/53473/diff/4#7
> 
> `container->config.user()` has a return type of `string`.  When the value 
> is not set, calling `container->config.user()` returns the empty string.  You 
> need to manually convert it to an `Option`.
> 
> `container->config` is a protobuf defined here: 
> 
> https://github.com/apache/mesos/blob/1.1.x/include/mesos/slave/containerizer.proto#L112
> 
> Sivaram Kannan wrote:
> https://reviews.apache.org/r/53473/diff/6/ has an explicit conversion to 
> Option. When container->config.user() is empty, I tried to set 
> 
> Option user = None()
> 
> But the call logger->prepare(..., user), still crashes the agent. Doesn't 
> the below code more logical than setting the value to None()?
> 
> Result result = os::user();
> user = result.get();
> 
> Joseph Wu wrote:
> What's the stack trace for the crash?  (And what are the steps taken to 
> repro?)
> 
> Sivaram Kannan wrote:
> Sorry - should have added when I created the RBR. 
> 
> Steps to reproduce: 
> 1. In the containerizer.cpp, change the below code in the rbr 
> 2.   Option user = None();
>   if (container->config.has_user()) {
> user = container->config.user();
>   } else {
> Result result = os::user();
> //user = result.get(); <= Current code
> user = None(); <=== Change it to this
>   }
>  3. make and make check. 
>  4. Run the below test
>  5. sudo bin/mesos-tests.sh --verbose 
> --gtest_filter="UserContainerLoggerTest.ROOT_LOGROTATE_RotateAsSwitchUserFalse"
>  6. The above test would end up in a crash like below.
>  7. Stack Trace:
>  8. I1116 15:58:16.660096 15282 slave.cpp:2031] Queued task 
> 'bffed9be-9830-4840-a313-e2ff3741b2fc' for executor 
> 'bffed9be-9830-4840-a313-e2ff3741b2fc' of framework 
> 16b6e12e-3344-4aad-adf1-a3460b3ad2a1-
> lt-mesos-tests: ../../3rdparty/stout/include/stout/option.hpp:111: const 
> T& Option::get() const & [with T = std::basic_string]: Assertion 
> `isSome()' failed.
> *** Aborted at 1479311896 (unix time) try "date -d @1479311896" if you 
> are using GNU date ***
> PC: @ 0x7f89e65b0c37 (unknown)
> *** SIGABRT (@0x3ba2) received by PID 15266 (TID 0x7f89dd0db700) from PID 
> 15266; stack trace: ***
> @ 0x7f89e694f330 (unknown)
> @ 0x7f89e65b0c37 (unknown)
> @ 0x7f89e65b4028 (unknown)
> @ 0x7f89e65a9bf6 (unknown)
> @ 0x7f89e65a9ca2 (unknown)
> @   0xa8542f _ZNKR6OptionISsE3getEv
> @ 0x7f89dfba6a9b 
> mesos::internal::logger::LogrotateContainerLoggerProcess::prepare()
> @ 0x7f89dfbbcb5e 
> _ZZN7process8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS1_8internal6logger31LogrotateContainerLoggerProcessERKNS1_12ExecutorInfoERKSsRK6OptionISsES8_SsSE_EENS_6FutureIT_EERKNS_3PIDIT0_EEMSL_FSJ_T1_T2_T3_ET4_T5_T6_ENKUlPNS_11ProcessBaseEE_clESY_
> @ 0x7f89dfbcb54d 
> _ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS5_8internal6logger31LogrotateContainerLoggerProcessERKNS5_12ExecutorInfoERKSsRK6OptionISsESC_SsSI_EENS0_6FutureIT_EERKNS0_3PIDIT0_EEMSP_FSN_T1_T2_T3_ET4_T5_T6_EUlS2_E_E9_M_invokeERKSt9_Any_dataS2_
> @ 0x7f89eca9168d std::function<>::operator()()
> @ 0x7f89eca73e0d process::ProcessBase::visit()
> @ 0x7f89eca7c2ec process::DispatchEvent::visit()
> @   0xa674b2 process::ProcessBase::serve()
> @ 0x7f89eca700fa process::ProcessManager::resume()
> @ 0x7f89eca6cc39 
> _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv
> @ 0x7f89eca7ba94 
> _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIIEEEvSt12_Index_tupleIIXspT_EEE
> @ 0x7f89eca7b9eb 
> _ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv
> @ 0x7f89eca7b984 
> _ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv
> @ 0x7f89e712aa60 (unknown)
> @ 0x7f89e6947184 start_thread
> @ 0x7f89e667437d (unknown)

With your latest review diff, this stack trace shouldn't be possible, as there 
is no code in the `LogrotateContainerLoggerProcess` that calls `user.get()`.  
Can you double check?  If you've added a

Review Request 53817: Added missing cleanup to libprocess `finalize()`.

2016-11-16 Thread Greg Mann


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

Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

The `finalize()` function in libprocess previously did
not delete the thread-local `Executor` which is used to
`defer` execution to an unspecified context. This object
must be deleted during finalization so that the
`__executor__` macro creates a new process if libprocess
is subsequently reinitialized.


Diffs
-

  3rdparty/libprocess/src/process.cpp ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 

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


Testing
---

Testing details are found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:08 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


Changes
---

Updated dependency.


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


Repository: mesos


Description (updated)
---

Eliminated an EOF race condition in libprocess SSL socket.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing
---

Testing details found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 53805: Updated libprocess test to use new 'Socket::shutdown' parameter.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:22 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Updated libprocess test to use new 'Socket::shutdown' parameter.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing
---

NOTE that there is currently an issue when running the entire test suite: 
`Scheme/HTTPTest.SocketEOF/0` fails with a file permission issue. Will update 
this chain with the fix soon.


Thanks,

Greg Mann

Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:21 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Fixed v1 namespace.


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


Repository: mesos


Description
---

Modified a scheduler test to run with SSL enabled.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp c031823843a8ae5ecbccf60b9edf83ec1afcc5e6 

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


Testing
---

The test was run in repetition as follows:

`GTEST_REPEAT=-1 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown*" 
bin/mesos-tests.sh`


Thanks,

Greg Mann

Re: Review Request 53803: Added a new libprocess HTTP test.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:20 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

This patch adds HTTPTest.SocketEOF to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.


Diffs
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing
---

Testing details are at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 53803: Added a new libprocess HTTP test.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:20 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


Changes
---

Rebase.


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


Repository: mesos


Description (updated)
---

Added a new libprocess HTTP test.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing
---

Testing details are at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

2016-11-16 Thread Greg Mann


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

(Updated Nov. 16, 2016, 7:19 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


Changes
---

Fixed test setup/teardown order.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing
---

Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`


Thanks,

Greg Mann

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

2016-11-16 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [53790, 53791]

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

- Mesos ReviewBot


On Nov. 15, 2016, 11:34 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53791/
> ---
> 
> (Updated Nov. 15, 2016, 11:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Kevin Klues, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6588
> https://issues.apache.org/jira/browse/MESOS-6588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The dependencies for the programs we need in the Linux root
> filesystem vary over time and across distributions. Since Stout
> has support for parsing the library dependencies of ELF binaries,
> use this to collect the necessary dependencies when constructing
> a root filesystem for the tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53791/diff/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 24)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52544: Introduced `int_fd` class.

2016-11-16 Thread Daniel Pravat


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

(Updated Nov. 16, 2016, 6:37 p.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
Park.


Repository: mesos


Description
---

In POSIX the `socket`,`pipe` and the `filedescriptor` are
represented by an int type. In Windows a socket is kept in a
`SOCKET` type (64 bit wide), a pipe in a `HANDLE` (64 bit wide) and
a file descriptor in an int. This class unifies all Windows types.
In POSIX this class is an int.


Diffs (updated)
-

  3rdparty/stout/include/stout/os.hpp bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
  3rdparty/stout/include/stout/os/filedescriptor.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/filedescriptor.hpp PRE-CREATION 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 53712: Added `system` environement variables in ` execvpe.cpp`.

2016-11-16 Thread Daniel Pravat


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

(Updated Nov. 16, 2016, 6:37 p.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Added `system` environement variables in ` execvpe.cpp`.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 
320e42748adbabf09f77cb4f5951e2a7ea58fe64 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52625: Replaced POSIX `int` with `int_fd` abstraction in `libprocess` folder.

2016-11-16 Thread Daniel Pravat


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

(Updated Nov. 16, 2016, 6:37 p.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
Park.


Repository: mesos


Description
---

On POSIX this should have no effect.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
eec5efd7e6b71a783f2bb40826054d0488cee71f 
  3rdparty/libprocess/include/process/network.hpp 
52110667185370a4c92e2fa524819ab1f34bdec9 
  3rdparty/libprocess/include/process/socket.hpp 
f798af7879546d71e8ef4a295c9cf489a70cb61f 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
1d02454d5541f96cb4928bf027fcae3764989d67 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 
f452f6743d01f0b99010fa5e5bcbaae1358c8241 
  3rdparty/libprocess/src/io.cpp d930ceebc90468e082b984e41385549f906dc6ae 
  3rdparty/libprocess/src/libevent_poll.cpp 
0803ba33622c86df38b3efd4f1e3197edf93a0af 
  3rdparty/libprocess/src/poll_socket.hpp 
d04f3f2d1bcf70464ac659b29f96574bbd233414 
  3rdparty/libprocess/src/poll_socket.cpp 
f0ee1490e6fccb038f64a27b2c71458ad5b5e5a1 
  3rdparty/libprocess/src/process.cpp ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
  3rdparty/libprocess/src/socket.cpp 7f93168e1572f8669f67a4c5e6e5467259b7a407 
  3rdparty/libprocess/src/subprocess_windows.cpp 
20cad52d4a4d7fc51487e150a849972eb19ed08e 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
0dc1c62b0e708125392ffa798a52b59ea6e55abe 
  3rdparty/stout/include/stout/os/open.hpp 
2a357926860b1523c51f12c7edee2babe6afbfa3 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 53473: Add new param user to logrotate's prepare function.

2016-11-16 Thread Sivaram Kannan



> On Nov. 15, 2016, 2:42 p.m., Sivaram Kannan wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1388
> > 
> >
> > Passing None() has the agent crashing when we have switch_user=false.
> 
> Sivaram Kannan wrote:
> That is the reason I thought I would have the current user of the agent 
> set to logger.
> 
> Joseph Wu wrote:
> The reason the agent/test would fail is because of this line in your 
> fourth diff:
> https://reviews.apache.org/r/53473/diff/4#7
> 
> `container->config.user()` has a return type of `string`.  When the value 
> is not set, calling `container->config.user()` returns the empty string.  You 
> need to manually convert it to an `Option`.
> 
> `container->config` is a protobuf defined here: 
> 
> https://github.com/apache/mesos/blob/1.1.x/include/mesos/slave/containerizer.proto#L112
> 
> Sivaram Kannan wrote:
> https://reviews.apache.org/r/53473/diff/6/ has an explicit conversion to 
> Option. When container->config.user() is empty, I tried to set 
> 
> Option user = None()
> 
> But the call logger->prepare(..., user), still crashes the agent. Doesn't 
> the below code more logical than setting the value to None()?
> 
> Result result = os::user();
> user = result.get();
> 
> Joseph Wu wrote:
> What's the stack trace for the crash?  (And what are the steps taken to 
> repro?)

Sorry - should have added when I created the RBR. 

Steps to reproduce: 
1. In the containerizer.cpp, change the below code in the rbr 
2.   Option user = None();
  if (container->config.has_user()) {
user = container->config.user();
  } else {
Result result = os::user();
//user = result.get(); <= Current code
user = None(); <=== Change it to this
  }
 3. make and make check. 
 4. Run the below test
 5. sudo bin/mesos-tests.sh --verbose 
--gtest_filter="UserContainerLoggerTest.ROOT_LOGROTATE_RotateAsSwitchUserFalse"
 6. The above test would end up in a crash like below.
 7. Stack Trace:
 8. I1116 15:58:16.660096 15282 slave.cpp:2031] Queued task 
'bffed9be-9830-4840-a313-e2ff3741b2fc' for executor 
'bffed9be-9830-4840-a313-e2ff3741b2fc' of framework 
16b6e12e-3344-4aad-adf1-a3460b3ad2a1-
lt-mesos-tests: ../../3rdparty/stout/include/stout/option.hpp:111: const T& 
Option::get() const & [with T = std::basic_string]: Assertion 
`isSome()' failed.
*** Aborted at 1479311896 (unix time) try "date -d @1479311896" if you are 
using GNU date ***
PC: @ 0x7f89e65b0c37 (unknown)
*** SIGABRT (@0x3ba2) received by PID 15266 (TID 0x7f89dd0db700) from PID 
15266; stack trace: ***
@ 0x7f89e694f330 (unknown)
@ 0x7f89e65b0c37 (unknown)
@ 0x7f89e65b4028 (unknown)
@ 0x7f89e65a9bf6 (unknown)
@ 0x7f89e65a9ca2 (unknown)
@   0xa8542f _ZNKR6OptionISsE3getEv
@ 0x7f89dfba6a9b 
mesos::internal::logger::LogrotateContainerLoggerProcess::prepare()
@ 0x7f89dfbbcb5e 
_ZZN7process8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS1_8internal6logger31LogrotateContainerLoggerProcessERKNS1_12ExecutorInfoERKSsRK6OptionISsES8_SsSE_EENS_6FutureIT_EERKNS_3PIDIT0_EEMSL_FSJ_T1_T2_T3_ET4_T5_T6_ENKUlPNS_11ProcessBaseEE_clESY_
@ 0x7f89dfbcb54d 
_ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8dispatchIN5mesos5slave15ContainerLogger14SubprocessInfoENS5_8internal6logger31LogrotateContainerLoggerProcessERKNS5_12ExecutorInfoERKSsRK6OptionISsESC_SsSI_EENS0_6FutureIT_EERKNS0_3PIDIT0_EEMSP_FSN_T1_T2_T3_ET4_T5_T6_EUlS2_E_E9_M_invokeERKSt9_Any_dataS2_
@ 0x7f89eca9168d std::function<>::operator()()
@ 0x7f89eca73e0d process::ProcessBase::visit()
@ 0x7f89eca7c2ec process::DispatchEvent::visit()
@   0xa674b2 process::ProcessBase::serve()
@ 0x7f89eca700fa process::ProcessManager::resume()
@ 0x7f89eca6cc39 _ZZN7process14ProcessManager12init_threadsEvENKUt_clEv
@ 0x7f89eca7ba94 
_ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEE9_M_invokeIIEEEvSt12_Index_tupleIIXspT_EEE
@ 0x7f89eca7b9eb 
_ZNSt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEclEv
@ 0x7f89eca7b984 
_ZNSt6thread5_ImplISt12_Bind_simpleIFZN7process14ProcessManager12init_threadsEvEUt_vEEE6_M_runEv
@ 0x7f89e712aa60 (unknown)
@ 0x7f89e6947184 start_thread
@ 0x7f89e667437d (unknown)


- Sivaram


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


On Nov. 14, 2016, 9:52 p.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53473/
>

Re: Review Request 53541: WIP: Added authorization actions for debug API.

2016-11-16 Thread Alexander Rojas


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

(Updated Nov. 16, 2016, 4:32 p.m.)


Review request for mesos, Adam B, Kapil Arya, Kevin Klues, and Till Toenshoff.


Changes
---

- Updated to latest review design.
- Disabled tests for the new actions, they need to be rewritten.
- `LAUNCH_NESTED_CONTAINER`  and `LAUNCH_NESTER_CONTAINER_SESSION` verify two 
different sets of ACL's each.


Summary (updated)
-

WIP: Added authorization actions for debug API.


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


Repository: mesos


Description (updated)
---

**WIP: Do not commit**

Added authorization actions for debug API.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto e3fd6a4a1b617a75714ebd6e08ab10cffa1a7d1b 
  include/mesos/authorizer/authorizer.hpp 
7217600351bb089dbd5728ce015d962be7498665 
  include/mesos/authorizer/authorizer.proto 
0696a629ac2d2b9950e20708f0c3666b58ff7ca0 
  src/authorizer/local/authorizer.cpp 77e05dd2475d6e7511e7c7eeea578ec31ff3d198 
  src/tests/authorization_tests.cpp d23f551c2caa454da0c0f6cb7d77a8c2bd75a474 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 53534: Added test `ReservationTest.PreventUnreservingAlienResources`.

2016-11-16 Thread Alexander Rukletsov


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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 16, 2016, 2:01 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53534/
> ---
> 
> (Updated Nov. 16, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that a framework can't use an `UNRESERVE` operation to
> steal resources reserved by a framework with another role.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53534/diff/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter='ReservationTest.PreventUnreservingAlienResources' 
> --break-on-error --gtest_repeat=1000` with the allocation interval set to 5ns 
> and to 1ns.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53534: Added test `ReservationTest.PreventUnreservingAlienResources`.

2016-11-16 Thread Alexander Rukletsov



> On Nov. 16, 2016, 12:56 p.m., Alexander Rukletsov wrote:
> > src/tests/reservation_tests.cpp, lines 2399-2400
> > 
> >
> > Probably `WillRepeatedly(Return()); // Ignore subsequent offers`
> 
> Gastón Kleiman wrote:
> This is supposed to be the last offer.

But if the allocator decides to make one more offer (you have 5s allocation 
interval), there will be a warning, right?


- Alexander


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


On Nov. 16, 2016, 2:01 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53534/
> ---
> 
> (Updated Nov. 16, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that a framework can't use an `UNRESERVE` operation to
> steal resources reserved by a framework with another role.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53534/diff/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter='ReservationTest.PreventUnreservingAlienResources' 
> --break-on-error --gtest_repeat=1000` with the allocation interval set to 5ns 
> and to 1ns.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53470: Added a test to ensure MESOS-6142 is fixed.

2016-11-16 Thread Alexander Rukletsov


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




src/tests/reservation_tests.cpp (line 2145)


You probably want `.WillRepeatedly(Return());` here


- Alexander Rukletsov


On Nov. 16, 2016, 1:57 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53470/
> ---
> 
> (Updated Nov. 16, 2016, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6142
> https://issues.apache.org/jira/browse/MESOS-6142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that frameworks can't reserve resources using a role
> different from the one they registered with.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53470/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53528: Added test `ReservationTest.DropUnreserveWithInvalidRole`.

2016-11-16 Thread Gastón Kleiman


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

(Updated Nov. 16, 2016, 3:09 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
Park.


Repository: mesos


Description
---

See the summary.


Diffs
-

  src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 53741: Display maintainance info in the webui.

2016-11-16 Thread Tomasz Janiszewski



> On Nov. 16, 2016, 8:18 a.m., haosdent huang wrote:
> > src/webui/master/static/js/controllers.js, line 469
> > 
> >
> > Any reason that remove `pollState();` here?

This function was redundant since this page is not live updated.


- Tomasz


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


On Nov. 15, 2016, 9:03 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Nov. 15, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 6211892e7d689df9bf3b2a9071a76ad4c60d0485 
>   src/webui/master/static/js/app.js c32177aa23c962f2bdf03d98272fb6d21a565382 
>   src/webui/master/static/js/controllers.js 
> dd0cc810748a9bd378d9c6b15e9fe89b7c0f11d9 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 53534: Added test `ReservationTest.PreventStealingFromAnotherRole`.

2016-11-16 Thread Gastón Kleiman



> On Nov. 16, 2016, 12:56 p.m., Alexander Rukletsov wrote:
> > src/tests/reservation_tests.cpp, lines 2399-2400
> > 
> >
> > Probably `WillRepeatedly(Return()); // Ignore subsequent offers`

This is supposed to be the last offer.


- Gastón


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


On Nov. 7, 2016, 2:26 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53534/
> ---
> 
> (Updated Nov. 7, 2016, 2:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that a framework can't use an `UNRESERVE` operation to
> steal resources reserved by a framework with another role.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53534/diff/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter='ReservationTest.PreventStealingFromAnotherRole' 
> --break-on-error --gtest_repeat=1000`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53534: Added test `ReservationTest.PreventUnreservingAlienResources`.

2016-11-16 Thread Gastón Kleiman


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

(Updated Nov. 16, 2016, 2:01 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
Park.


Summary (updated)
-

Added test `ReservationTest.PreventUnreservingAlienResources`.


Repository: mesos


Description
---

This test ensures that a framework can't use an `UNRESERVE` operation to
steal resources reserved by a framework with another role.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 

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


Testing (updated)
---

`bin/mesos-tests.sh 
--gtest_filter='ReservationTest.PreventUnreservingAlienResources' 
--break-on-error --gtest_repeat=1000` with the allocation interval set to 5ns 
and to 1ns.


Thanks,

Gastón Kleiman

Re: Review Request 53470: Added a test to ensure MESOS-6142 is fixed.

2016-11-16 Thread Gastón Kleiman


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

(Updated Nov. 16, 2016, 1:57 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
Park.


Changes
---

Address feedback + minor cleanup.


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


Repository: mesos


Description
---

This test ensures that frameworks can't reserve resources using a role
different from the one they registered with.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 53534: Added test `ReservationTest.PreventStealingFromAnotherRole`.

2016-11-16 Thread Alexander Rukletsov


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




src/tests/reservation_tests.cpp (line 2271)


How about `PreventUnreservingAlienResources`?



src/tests/reservation_tests.cpp (line 2291)


No to slavery : )



src/tests/reservation_tests.cpp (line 2336)


Please extract these into constants.



src/tests/reservation_tests.cpp (line 2395)


Probably pull it down closer to the code to avoid stranded comment.



src/tests/reservation_tests.cpp (line 2397)


first?



src/tests/reservation_tests.cpp (lines 2399 - 2400)


Probably `WillRepeatedly(Return()); // Ignore subsequent offers`


- Alexander Rukletsov


On Nov. 7, 2016, 2:26 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53534/
> ---
> 
> (Updated Nov. 7, 2016, 2:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that a framework can't use an `UNRESERVE` operation to
> steal resources reserved by a framework with another role.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53534/diff/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter='ReservationTest.PreventStealingFromAnotherRole' 
> --break-on-error --gtest_repeat=1000`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53528: Added test `ReservationTest.DropUnreserveWithInvalidRole`.

2016-11-16 Thread Alexander Rukletsov


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



After a second though, why not just keeping https://reviews.apache.org/r/53534/ 
? Seems sufficient.

- Alexander Rukletsov


On Nov. 7, 2016, 2:21 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53528/
> ---
> 
> (Updated Nov. 7, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See the summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53528/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53571: Improved some operation validation tests.

2016-11-16 Thread Alexander Rukletsov


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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 8, 2016, 10:23 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53571/
> ---
> 
> (Updated Nov. 8, 2016, 10:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Removed some tests that don't make sense.
> * Renamed some tests to be consistent with what they check.
> * Improved/fixed some test descriptions.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 8697bb2dc05c54553066ace58553f0e40dfcb5f5 
> 
> Diff: https://reviews.apache.org/r/53571/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j 4 check V=0`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53528: Added test `ReservationTest.DropUnreserveWithInvalidRole`.

2016-11-16 Thread Alexander Rukletsov


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


Fix it, then Ship it!





src/tests/reservation_tests.cpp (line 2203)


Let's abolish slavery : )



src/tests/reservation_tests.cpp (line 2248)


Let's pull both roles as constants to the beginning of the test. And 
probably rename "yoyo" (even though I like it personally) : ).



src/tests/reservation_tests.cpp (lines 2255 - 2256)


Fits one line.


- Alexander Rukletsov


On Nov. 7, 2016, 2:21 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53528/
> ---
> 
> (Updated Nov. 7, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See the summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53528/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53470: Added a test to ensure MESOS-6142 is fixed.

2016-11-16 Thread Alexander Rukletsov


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


Fix it, then Ship it!





src/tests/reservation_tests.cpp (line 2114)


Let's abolish slavery at least in variable names.



src/tests/reservation_tests.cpp (line 2159)


Let's pull both roles as constants to the beginning of the test. And 
probably rename "yoyo" (even though I like it personally) : ).



src/tests/reservation_tests.cpp (lines 2160 - 2161)


Blank line.



src/tests/reservation_tests.cpp (lines 2166 - 2167)


fits one line.


- Alexander Rukletsov


On Nov. 4, 2016, 12:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53470/
> ---
> 
> (Updated Nov. 4, 2016, 12:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6142
> https://issues.apache.org/jira/browse/MESOS-6142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that frameworks can't reserve resources using a role
> different from the one they registered with.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
> 
> Diff: https://reviews.apache.org/r/53470/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

2016-11-16 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [52735, 50128, 50947, 53532, 50599, 50125, 53533, 50127]

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

- Mesos ReviewBot


On Nov. 16, 2016, 3:30 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated Nov. 16, 2016, 3:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53541: Added authorization actions for debug API.

2016-11-16 Thread Alexander Rojas



> On Nov. 16, 2016, 7:14 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, lines 142-143
> > 
> >
> > What are these exceptional cases? What is an authorizer supposed to do 
> > with no object metadata at all?

I could remove that, however, there could be cases where `user` is not set in 
any of the fields.


- Alexander


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


On Nov. 9, 2016, 1:43 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53541/
> ---
> 
> (Updated Nov. 9, 2016, 1:43 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Kevin Klues, and Till Toenshoff.
> 
> 
> Bugs: MESOS-6474
> https://issues.apache.org/jira/browse/MESOS-6474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization actions for debug API.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> e3fd6a4a1b617a75714ebd6e08ab10cffa1a7d1b 
>   include/mesos/authorizer/authorizer.proto 
> b6a9f142eecbdfd59210872a92e3126f04de334c 
>   src/authorizer/local/authorizer.cpp 
> f1dff65d973fc84f4171f68fd0391a2343a96965 
>   src/tests/authorization_tests.cpp 5d7e17b67821357b8cb538798acc883945c8f8fd 
> 
> Diff: https://reviews.apache.org/r/53541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 53741: Display maintainance info in the webui.

2016-11-16 Thread haosdent huang


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


Fix it, then Ship it!




Ship It!


src/webui/master/static/js/app.js (lines 291 - 293)


Got it, thank you! Let's use `if (!key)` to check if it is `undefined` or 
`null` here. `key` may equal to `undefined`?



src/webui/master/static/js/controllers.js (line 469)


Any reason that remove `pollState();` here?



src/webui/master/static/maintenance.html (line 6)


`Maintenance Schedule`



src/webui/master/static/maintenance.html (line 23)


`{{(window.unavailability.start.nanoseconds/100) | isoDate}}`



src/webui/master/static/maintenance.html (line 30)


`{{machine_ids.hostname ? machine_ids.hostname : machine_ids.ip}}`


- haosdent huang


On Nov. 15, 2016, 9:03 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Nov. 15, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 6211892e7d689df9bf3b2a9071a76ad4c60d0485 
>   src/webui/master/static/js/app.js c32177aa23c962f2bdf03d98272fb6d21a565382 
>   src/webui/master/static/js/controllers.js 
> dd0cc810748a9bd378d9c6b15e9fe89b7c0f11d9 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

63 matches

Mail list logo