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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53704, 53837]

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. 19, 2016, 1:27 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53837/
> ---
> 
> (Updated Nov. 19, 2016, 1:27 a.m.)
> 
> 
> 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
> 
>

Review Request 53927: Check isJailed in tests that call mknod.

[David Forsythe]

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

Review request for mesos and Ian Downes.


Bugs: MESOS-6612 and MESOS-6613
https://issues.apache.org/jira/browse/MESOS-6612
https://issues.apache.org/jira/browse/MESOS-6613


Repository: mesos


Description
---

Check isJailed in tests that call mknod.


Diffs
-

  3rdparty/stout/tests/os/rmdir_tests.cpp 
9aa4059d589e84f3c377163b1d6d2a278d4130b6 
  3rdparty/stout/tests/os_tests.cpp f4b9ad71b22b5cc70a412c9a6a3e21da67121e17 

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


Testing
---

gmake check


Thanks,

David Forsythe

Review Request 53926: Move isJailed for FreeBSD into utils.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Move isJailed for FreeBSD into utils.


Diffs
-

  3rdparty/stout/include/stout/tests/utils.hpp 
4e5359c82da4a39d4a7093bbfe95afa14e61f69d 
  3rdparty/stout/tests/os_tests.cpp f4b9ad71b22b5cc70a412c9a6a3e21da67121e17 

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


Testing
---

gmake check on FreeBSD


Thanks,

David Forsythe

Review Request 53925: Fix wait macros on FreeBSD.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Fix wait macros on FreeBSD.


Diffs
-

  3rdparty/stout/include/stout/gtest.hpp 
b2f75b6c706df9de68edbac86a1e2dec32a574ed 

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


Testing
---

gmake check gets closer to build completion.


Thanks,

David Forsythe

Re: Review Request 53879: Removed duplicate note.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Nov. 18, 2016, 4:23 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53879/
> ---
> 
> (Updated Nov. 18, 2016, 4:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed duplicate note.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp c7ac957a9c01f93a4ed852cd26d7f2cc8e70c78e 
> 
> Diff: https://reviews.apache.org/r/53879/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 53574: Made `CommandExecutorTest.NoTransitionFromKillingToRunning` more robust.

[Alexander Rukletsov]

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




src/tests/command_executor_tests.cpp (lines 266 - 268)


Fits one line?


- Alexander Rukletsov


On Nov. 8, 2016, 4 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53574/
> ---
> 
> (Updated Nov. 8, 2016, 4 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the test use the newly introduced test helper, instead
> of the more fragile `trap` shell built-in.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> 62a09bd17152540bad658c7c3583ee7239f435f5 
> 
> Diff: https://reviews.apache.org/r/53574/diff/
> 
> 
> Testing
> ---
> 
> `make check` (macOS and Debian)
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53573: Added the kill policy test helper.

[Alexander Rukletsov]

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




src/tests/kill_policy_test_helper.hpp (line 36)


`uint16_t` requires `#include `



src/tests/kill_policy_test_helper.cpp (line 29)


One more blank line?



src/tests/kill_policy_test_helper.cpp (lines 30 - 35)


Either rename the fucntion to reflect it is meant to be used for `SIGTERM` 
only, or add an `if` inside.



src/tests/kill_policy_test_helper.cpp (line 60)


You don't use `os::sleep` on purpose?



src/tests/kill_policy_test_helper.cpp (line 62)


`#include `?


- Alexander Rukletsov


On Nov. 8, 2016, 3:59 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53573/
> ---
> 
> (Updated Nov. 8, 2016, 3:59 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test helper to facilitate the testing of `KillPolicy`.
> 
> The helper blocks until it receives a SIGTERM, then sleeps for a
> configurable amount of time before finally returning EXIT_SUCCESS.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/tests/CMakeLists.txt cf583ea3f10482b510459fb11a7ecaf76e498391 
>   src/tests/kill_policy_test_helper.hpp PRE-CREATION 
>   src/tests/kill_policy_test_helper.cpp PRE-CREATION 
>   src/tests/test_helper_main.cpp 42a8ec675500cb7b155823a4ea64d70a3481eaf4 
> 
> Diff: https://reviews.apache.org/r/53573/diff/
> 
> 
> Testing
> ---
> 
> `make check` (macOS and Debian)
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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

[Michael Park]

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




3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 34 - 39)


I think we would be better off if we represented the states more accurately 
according to the invariants of this class.

In terms of members: I think we should represent them as:

```
union {
  // We keep both a CRT FD as well as a `HANDLE`
  // regardless of whether we were constructed
  // from a file or a handle.
  //
  // This is because once we request for a CRT FD
  // from a `HANDLE`, we're required to close it
  // via `_close`. If we were to do the conversion
  // lazily upon request, the resulting CRT FD
  // would be dangling.
  struct {
int file;
HANDLE handle;
  };
  SOCKET socket;
};
```

I think we should keep separate track of which one is really active.

```cpp
enum { FD_FILE, FD_HANDLE, FD_SOCKET } type;
```



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 42 - 44)


As suggested above, I think we should keep track of the real active member 
via the `enum`.

While the `isSocket` is fine, due to the invariant that we have both the 
CRT FD and `HANDLE` set at all times, `isHandle` would inaccurately return 
`true` for file, and `isFile` would inaccurately return `true` for a handle.



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 46 - 51)


We should just `= default;` this.



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 53 - 54)


Also just `= default;`



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 56 - 65)


With the suggestion above, we get:

```
  WindowsFD(HANDLE handle)
: type(FD_HANDLE),
  file(
  handle == INVALID_HANDLE_VALUE
? -1
: _open_osfhandle(reinterpret_cast(handle), O_RDWR)),
  handle(handle) {}
```



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 67 - 72)


```
  WindowsFD(SOCKET socket) : type(FD_SOCKET), socket(socket) {}
```



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 74 - 83)


```
  WindowsFD(int file)
: type(FD_FILE),
  file(file),
  handle(
  file < 0 ? INVALID_HANDLE_VALUE
   : reinterpret_cast(_get_osfhandle(file))) {}
```



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 91 - 113)


I don't think we need these.

```
  WindowsFileDescriptor& operator=(const WindowsFileDescriptor&) = default;
```
should do this for us. That is, `int`, `HANDLE` or `SOCKET` will implicitly 
converted to `WindowsFileDescriptor` using the implicit constructors above and 
be assigned.



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 115 - 129)


My preference would be to remove this member function and define it in 
`windows/close.hpp` as:

```
Try close(WindowsFD fd)
{
  switch (fd.type) {
case WindowsFD::FD_FILE:
case WindowsFD::FD_HANDLE: {
  // The invocation to `_close` also implicitly
  // closes the underlying `HANDLE`,
  // therefore we don't need to call `CloseHandle`.
  ::_close(fd.file);
  break;
}
case WindowsFD::FD_SOCKET: {
  ::shutdown(fd.socket, SD_BOTH);
  ::closesocket(fd.socket);
  break;
}
  }
  return Nothing();
}
```

This would need to be `friend`ed by `WindowsFD`.
This also means that all non-member functions also need to be friended, and 
I think that's fine.

```
  friend Try close(WindowsFD fd);
```



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 318 - 334)


I feel like we should be able to answer this question (`is_socket`) for 
`WindowsFD` since we answer it for `int` in POSIX. (Beyond the fact that it 
seems bizzare for `SOCKET` to somehow __not__ be a socket...)



3rdparty/stout/include/stout/os/windows/filedescriptor.hpp (lines 381 - 384)


Do we actually need this? I think the `int` on the rhs should 

Re: Review Request 52868: Added pause/resume functionality to HealthChecker.

[Benjamin Mahler]

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



After discussion offline, there seems to be some interesting questions around 
pause/resume functionality:

(1) What are the implications for timeouts, delays, etc? (e.g. do we reset the 
consecutive failure counters? do we schedule a new health check immediately or 
wait for another delay?)

(2) Since we only need to support the ability to stop health checking for now, 
maybe we can just avoid having to consider these questions and only supporting 
stop for now.

- Benjamin Mahler


On Nov. 14, 2016, 10:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52868/
> ---
> 
> (Updated Nov. 14, 2016, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> a1dc72493ff31b87068d5691f4d5b794392caf76 
>   src/health-check/health_checker.cpp 
> e2b32e2d57515202f547d12ba492ad8eb633641b 
> 
> Diff: https://reviews.apache.org/r/52868/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Mesos ReviewBot]

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



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. 18, 2016, 10:53 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53791/
> ---
> 
> (Updated Nov. 18, 2016, 10:53 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/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/linux/ldd.hpp PRE-CREATION 
>   src/linux/ldd.cpp PRE-CREATION 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   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 53837: Added a per container mesos-io-switchboard process.

[Kevin Klues]

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

(Updated Nov. 19, 2016, 1:27 a.m.)


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


Changes
---

Updated the termination semantics of the switchboard process.


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 (updated)
-

  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 52295: Added additional unit tests for shared resources.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52295]

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. 18, 2016, 7:49 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52295/
> ---
> 
> (Updated Nov. 18, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6185
> https://issues.apache.org/jira/browse/MESOS-6185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests include the following:
> - Allocator tests when dealing with multiple frameworks of the same
>   role using the same shared volume at the same time.
> - Validity of DESTROY shared volume when there are pending tasks
>   that have been assigned that shared volume.
> - Handlng of master failover.
> - Sorter tests covering shared resources.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> edd5cea8996d7c616cf9428f0f1c05d70c37c307 
>   src/tests/master_validation_tests.cpp 
> f893067859425967654401f3226149268b51cf57 
>   src/tests/persistent_volume_tests.cpp 
> 8651b2c5455089041f16d091c90a7e0d948191b8 
>   src/tests/sorter_tests.cpp 9f48abe65c011acfaf79f3d0d79f1d032fd6a4af 
> 
> Diff: https://reviews.apache.org/r/52295/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52645: Harden Mesos

[Michael Park]

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




m4/ax_check_compile_flag.m4 (line 61)


This seems to introduce a new requirement of `autoconf` version 2.64 or 
higher. Ran into this no CentOS 6.


- Michael Park


On Nov. 9, 2016, 11:37 a.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Nov. 9, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for Mesos.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   configure.ac 5380cbc 
>   m4/ax_check_compile_flag.m4 PRE-CREATION 
>   src/Makefile.am 5a47c93 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52002: Added helper methods to determine types of disk resources.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/common/resources.cpp (lines 792 - 793)


This indentation looks odd.

Let's do:

```
  return resource.name() == "disk" &&
 resource.has_disk() && 
 resource.disk().has_persistence();
```



src/common/resources.cpp (lines 835 - 836)


Style suggestion:

```
  return resource.name() == "disk" &&
 (!resource.has_disk() || !resource.disk().has_source());
```


- Jiang Yan Xu


On Oct. 25, 2016, 11:19 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52002/
> ---
> 
> (Updated Oct. 25, 2016, 11:19 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
> https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper methods to determine types of disk resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp f569c931ff7db8d51dfd7c96f4f2addab05df85d 
>   include/mesos/v1/resources.hpp f60ab794a0c7c24885c49cc47b798c363e3279e7 
>   src/common/resources.cpp 4bb9beffcb3509f4226b4985e05eccec01412d0f 
>   src/tests/resources_tests.cpp d6eb7787bac58c1133a4bab0fc17df49117fed87 
>   src/v1/resources.cpp 46cc00f2f453f5eb4ddc4b0b9b89be2bd89f05d9 
> 
> Diff: https://reviews.apache.org/r/52002/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53096: Fix handling in shared count in total resources in the sorter.

[Jiang Yan Xu]

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




src/master/allocator/mesos/hierarchical.cpp (lines 752 - 755)


This seems the wrong place to do it, multiple copies of the same shared 
resources are added to the allocation of the role (in roleSorter and 
quotaRoleSorter) from multiple places:

- updateAllocation
- addFramework
- addSlave
- allocate

If the rule is to not have more than one copy the shared resources to 
roleSorter and quotaRoleSorter, they should be invariants, i.e., we should 
prevent them from being added instead of deduping them here.

Let's chat about the design.



src/master/allocator/mesos/hierarchical.cpp (line 757)


So if `operation` contians multiple copies, the result will end up having 
multiple copies right?


- Jiang Yan Xu


On Nov. 17, 2016, 9:14 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53096/
> ---
> 
> (Updated Nov. 17, 2016, 9:14 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6444
> https://issues.apache.org/jira/browse/MESOS-6444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We maintain a single copy of shared resource in the role and quota
> sorter's total resources. So, when we update these resources, we ensure
> that we only count a single copy even though the framework sorter
> may be returned multiple copies of a shared resource.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> c8f9492ee1b69e125a1e841116d22a578a9b524e 
>   src/tests/persistent_volume_tests.cpp 
> 8651b2c5455089041f16d091c90a7e0d948191b8 
> 
> Diff: https://reviews.apache.org/r/53096/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53882: Fix configure on FreeBSD.

[David Forsythe]

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

(Updated Nov. 19, 2016, 12:11 a.m.)


Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Fix configure on FreeBSD.


Diffs (updated)
-

  configure.ac 5380cbc6a7951ede2f883f7045952a3f3434479e 

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


Testing
---

../configure


Thanks,

David Forsythe

Review Request 53913: Disable sentinel checks for clang on FreeBSD.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Disable sentinel checks for clang on FreeBSD.


Diffs
-

  configure.ac 5380cbc6a7951ede2f883f7045952a3f3434479e 

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


Testing
---

gmake on FreeBSD.


Thanks,

David Forsythe

Review Request 53912: Fix xattr for FreeBSD.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Fix xattr for FreeBSD.


Diffs
-

  3rdparty/stout/include/stout/os/posix/xattr.hpp 
518940fdffab38ad97cf229078c4494fa944e1d8 

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


Testing
---

gmake on FreeBSD, no check run.


Thanks,

David Forsythe

Re: Review Request 53897: Changed how master represents "recovered" frameworks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53885, 53886, 53887, 53888, 53889, 53890, 53891, 53892, 
53893, 53894, 53895, 53896, 53897]

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. 18, 2016, 7:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53897/
> ---
> 
> (Updated Nov. 18, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6419
> https://issues.apache.org/jira/browse/MESOS-6419
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After master failover, the new master doesn't know which frameworks were
> registered with the previous master (because this information is not
> currently stored in the registry). In the period after the master fails
> over but before the framework scheduler has re-registered, the master
> learns about the frameworks in the cluster when agents re-register (an
> agent reports the FrameworkInfo for all of the frameworks it is running
> when it re-registers).
> 
> Such frameworks were previously represented separately from the normal
> list of frameworks in the master: the master kept a collection of
> `FrameworkInfo` for these "recovered" frameworks.
> 
> This commit removes this separate collection of "recovered" frameworks.
> Instead, the master now treats recovered frameworks very similarly to
> frameworks that are registered but currently disconnected. For example,
> recovered frameworks will now have a `Framework` object which tracks the
> tasks/executors running under that framework; recovered frameworks will
> be reported via the normal "frameworks" key when querying HTTP
> endpoints. Similarly, "teardown" operations on recovered frameworks will
> now work correctly (MESOS-6419).
> 
> This means there is no longer a concept of "orphan tasks" [1]: if the
> master knows about a task, the task will be running under a framework
> (albeit the framework might be recovered or disconnected). A new
> "recovered" key has been added to various HTTP endpoints/APIs to
> determine if a framework hasn't yet re-registered after master failover.
> 
> [1] The exception here is if the cluster contains Mesos agents older
> than 1.0, because old Mesos agents don't report `FrameworkInfo`s when
> they re-register.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
>   include/mesos/v1/master/master.proto 
> 022b491b7d5c49c5aeddf4ffc97c148f55629c95 
>   src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
>   src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
>   src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
>   src/tests/api_tests.cpp 8889e7807ecead736eaac3910332e86d594e8cec 
>   src/tests/fault_tolerance_tests.cpp 
> 1ade7faee56d394de30b798982dbb6e32f81 
>   src/tests/master_allocator_tests.cpp 
> bb94e38d5bb472801366c172cfc036f2eecdcbcb 
>   src/tests/master_authorization_tests.cpp 
> 4712361021708fff1ebdb5fa34386196c10c838e 
>   src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 
>   src/tests/partition_tests.cpp 5a0d4bd2de6a5aa0e9fdf0d34cd10d16fd4e34a1 
>   src/tests/teardown_tests.cpp 0babf8c99f133c3f0dada772bd5cd2601c47a080 
> 
> Diff: https://reviews.apache.org/r/53897/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Michael Park]

> On Nov. 16, 2016, 4:57 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 36-38
> > 
> >
> > What are these for?
> 
> Daniel Pravat wrote:
> 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.

What about the others `handletype`, and `nonblock`? I'm mostly curious if these 
are here for some layout reason? In specific, why the bitfields?


> On Nov. 16, 2016, 4:57 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 356-379
> > 
> >
> > Do we actually need this for something..?
> 
> Daniel Pravat wrote:
> 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.

So we do actually use both. Okay. Dropping.


- Michael


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


On Nov. 16, 2016, 11:09 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, 11:09 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
> 
>

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

[James Peach]

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

(Updated Nov. 18, 2016, 10:54 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
---

Move containerizer Rootfs support to a cpp file.


Diffs (updated)
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
  src/tests/containerizer/rootfs.cpp PRE-CREATION 

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


Testing
---

sudo make check (Fedora 24)


Thanks,

James Peach

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Michael Park]

> On Nov. 17, 2016, 1:01 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/decoder.hpp, lines 253-254
> > 
> >
> > Do you happen to know what the request body length has to do with 
> > `CHAR_MAX` in the first place...?
> 
> Aaron Wood wrote:
> I'm not 100% clear on this but my guess is that it's from a negotiated 
> max body size between the server and clients within Mesos...?
> 
> James Peach wrote:
> AFAICT this is assigning the `Content-Length` using `basic_string& 
> operator=( CharT ch );`, which is entirely broken.
> 
> Aaron Wood wrote:
> Ignore my comment. This looks like a bug since it'll cause requests 
> coming in from clients using gzip to be greatly truncated.

We'll let Anand and Joseph take over this issue as a separate task.


- Michael


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


On Nov. 18, 2016, 8:24 a.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Nov. 18, 2016, 8:24 a.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 76dca0b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

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

[Kevin Klues]

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

(Updated Nov. 18, 2016, 9:57 p.m.)


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


Changes
---

Fixed a typo in the name of the class instantiated for the `StderrWriter`.


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 (updated)
-

  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 53877: Added temporary file environment passing towards docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53877]

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. 18, 2016, 6:18 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 18, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7f 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

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

[Kevin Klues]

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

(Updated Nov. 18, 2016, 9:08 p.m.)


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


Changes
---

Added missing header since `SwitchboardServer` inherits from `ProtobufProcess`


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 (updated)
-

  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 53837: Added a per container mesos-io-switchboard process.

[Kevin Klues]

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

(Updated Nov. 18, 2016, 8:53 p.m.)


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


Changes
---

Completely reworked the abstractions in the mesos-io-switchboard binary and 
added a new `SwitchboardServer` process as a stub for implementing the core 
logic of the io switchboard in terms of handling incoming 
`ATTACH_CONTAINER_INPUT` and `ATTACH_CONTAINER_OUTPUT` messages.


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 (updated)
-

  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 53837: Added a per container mesos-io-switchboard process.

[Kevin Klues]

> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, line 38
> > 
> >
> > shouldn't this include be the first one in the file?
> 
> Kevin Klues wrote:
> We typically don't do that.
> We always put headers in the order:
> 
> 1) C  headers like , , etc.
> 2) C++ library headers like , , , etc.
> 3) Mesos external headers like , , 
> etc.
> 4) Mesos internal headers like the file above.
> 
> Within each category we group files at the same path depth 
> alphabetically, and separate each group by a blank line.
> 
> Gastón Kleiman wrote:
> The Mesos and the Google C++ style guide say that the "related header" 
> goes first:
> 
> > Use standard order for readability and to avoid hidden dependencies: 
> Related header, C library, C++ library, other libraries' .h, your project's 
> .h. 
> (https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes)
> 
> And in the "Order of includes" example in 
> http://mesos.apache.org/documentation/latest/c++-style-guide/

I'm willing to change this if @jie thinks we should, but doing a scan of all 
executables we have, we don't seem to follow this convention.


- Kevin


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


On Nov. 18, 2016, 5:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53837/
> ---
> 
> (Updated Nov. 18, 2016, 5:07 a.m.)
> 
> 
> 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 53882: Fix configure on FreeBSD.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53882]

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. 18, 2016, 6:08 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53882/
> ---
> 
> (Updated Nov. 18, 2016, 6:08 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: MESOS-6607
> https://issues.apache.org/jira/browse/MESOS-6607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix configure on FreeBSD.
> 
> 
> Diffs
> -
> 
>   configure.ac 5380cbc6a7951ede2f883f7045952a3f3434479e 
> 
> Diff: https://reviews.apache.org/r/53882/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

> On Nov. 17, 2016, 9:01 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/decoder.hpp, lines 253-254
> > 
> >
> > Do you happen to know what the request body length has to do with 
> > `CHAR_MAX` in the first place...?
> 
> Aaron Wood wrote:
> I'm not 100% clear on this but my guess is that it's from a negotiated 
> max body size between the server and clients within Mesos...?
> 
> James Peach wrote:
> AFAICT this is assigning the `Content-Length` using `basic_string& 
> operator=( CharT ch );`, which is entirely broken.

Ignore my comment. This looks like a bug since it'll cause requests coming in 
from clients using gzip to be greatly truncated.


- Aaron


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


On Nov. 18, 2016, 4:24 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Nov. 18, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 76dca0b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

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

[Vijay Srinivasaraghavan]

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



Anand Mazumdar - Could you please take a look at the patch?

- Vijay Srinivasaraghavan


On Nov. 18, 2016, 8:04 a.m., Vijay Srinivasaraghavan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53825/
> ---
> 
> (Updated Nov. 18, 2016, 8:04 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zameer Manji.
> 
> 
> 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
> -
> 
>   include/mesos/v1/allocator/allocator.proto 
> 73d45b37a7afc47366a4a01a36912f30b47c30b1 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
> 
> Diff: https://reviews.apache.org/r/53825/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vijay Srinivasaraghavan
> 
>

Re: Review Request 52295: Added additional unit tests for shared resources.

[Anindya Sinha]

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

(Updated Nov. 18, 2016, 7:49 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

A minor cosmetic change.


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


Repository: mesos


Description
---

Tests include the following:
- Allocator tests when dealing with multiple frameworks of the same
  role using the same shared volume at the same time.
- Validity of DESTROY shared volume when there are pending tasks
  that have been assigned that shared volume.
- Handlng of master failover.
- Sorter tests covering shared resources.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 
  src/tests/master_validation_tests.cpp 
f893067859425967654401f3226149268b51cf57 
  src/tests/persistent_volume_tests.cpp 
8651b2c5455089041f16d091c90a7e0d948191b8 
  src/tests/sorter_tests.cpp 9f48abe65c011acfaf79f3d0d79f1d032fd6a4af 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52295: Added additional unit tests for shared resources.

[Anindya Sinha]

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

(Updated Nov. 18, 2016, 7:48 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Tests include the following:
- Allocator tests when dealing with multiple frameworks of the same
  role using the same shared volume at the same time.
- Validity of DESTROY shared volume when there are pending tasks
  that have been assigned that shared volume.
- Handlng of master failover.
- Sorter tests covering shared resources.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 
  src/tests/master_validation_tests.cpp 
f893067859425967654401f3226149268b51cf57 
  src/tests/persistent_volume_tests.cpp 
8651b2c5455089041f16d091c90a7e0d948191b8 
  src/tests/sorter_tests.cpp 9f48abe65c011acfaf79f3d0d79f1d032fd6a4af 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

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

[James Peach]

> On Nov. 18, 2016, 1:24 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/rootfs.cpp, lines 81-84
> > 
> >
> > If we have to do this, format it this way
> > 
> > ```
> > auto entry = std::find_if(
> > cache.begin(),
> > cache.end(),
> > [dependency](const ldcache::Entry& e) {
> >   return strings::startsWith(e.name, dependency);
> > });
> > ```
> > 
> > But is prefix matching necessary? Can't we do extract matching 
> > (i.e.,`hashset.contains()`)?
> > 
> > Note that our seed list are just programs without versions.

I checked that you don't need the `startsWith`. The ld cache is a vector so I 
still need to do the linear search.


- James


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


On Nov. 18, 2016, 7:23 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53791/
> ---
> 
> (Updated Nov. 18, 2016, 7:23 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 53791: Use the stout ELF parser to collect Linux rootfs files.

[James Peach]

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

(Updated Nov. 18, 2016, 7:23 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 (updated)
-

  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 53791: Use the stout ELF parser to collect Linux rootfs files.

[James Peach]

> On Nov. 18, 2016, 1:24 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/rootfs.cpp, line 234
> > 
> >
> > Would the following look better?
> > 
> > ```
> > hashset needed(programs.begin(), programs.end());
> > 
> > foreach (const string& program, programs) {
> >   Try dependencies = ldd(file);
> > 
> >   // error handling.
> > 
> >   needed = needed | dependencies;
> > }
> > ```

We add the dependencies depth first, so initializing the needed set with the 
programs will make us think that we have already collected all the dependencies.


> On Nov. 18, 2016, 1:24 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/rootfs.cpp, line 107
> > 
> >
> > Not convinced that we need to extract it out since it's used only in a 
> > single place?

It's used in 2 places.


> On Nov. 18, 2016, 1:24 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/rootfs.cpp, lines 69-71
> > 
> >
> > It feels like it's not worth extracting out the logic
> > 
> > (similar code from volume.cpp)
> > ```
> > Try load = elf::File::load(realpath.get());
> > if (load.isError()) {
> >   return Error("Failed to elf::File::load '" + realpath.get() + 
> > "':"
> >" " + load.error());
> > }
> > 
> > Owned file(load.get());
> > ```
> > 
> > into a 10-line helper method and spend 4 lines here?

Yes I saw the usage in `volume.cpp` and that seemed awkward to have additional 
temporaries just to get the same pointer as an `Owned` pointer. I prefered this 
more verbose approach, though I don't mind switching back if you prefer the 
extra temporaries.


- James


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


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
> 
>

Review Request 53897: Changed how master represents "recovered" frameworks.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

After master failover, the new master doesn't know which frameworks were
registered with the previous master (because this information is not
currently stored in the registry). In the period after the master fails
over but before the framework scheduler has re-registered, the master
learns about the frameworks in the cluster when agents re-register (an
agent reports the FrameworkInfo for all of the frameworks it is running
when it re-registers).

Such frameworks were previously represented separately from the normal
list of frameworks in the master: the master kept a collection of
`FrameworkInfo` for these "recovered" frameworks.

This commit removes this separate collection of "recovered" frameworks.
Instead, the master now treats recovered frameworks very similarly to
frameworks that are registered but currently disconnected. For example,
recovered frameworks will now have a `Framework` object which tracks the
tasks/executors running under that framework; recovered frameworks will
be reported via the normal "frameworks" key when querying HTTP
endpoints. Similarly, "teardown" operations on recovered frameworks will
now work correctly (MESOS-6419).

This means there is no longer a concept of "orphan tasks" [1]: if the
master knows about a task, the task will be running under a framework
(albeit the framework might be recovered or disconnected). A new
"recovered" key has been added to various HTTP endpoints/APIs to
determine if a framework hasn't yet re-registered after master failover.

[1] The exception here is if the cluster contains Mesos agents older
than 1.0, because old Mesos agents don't report `FrameworkInfo`s when
they re-register.


Diffs
-

  include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
  include/mesos/v1/master/master.proto 022b491b7d5c49c5aeddf4ffc97c148f55629c95 
  src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
  src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
  src/tests/api_tests.cpp 8889e7807ecead736eaac3910332e86d594e8cec 
  src/tests/fault_tolerance_tests.cpp 1ade7faee56d394de30b798982dbb6e32f81 
  src/tests/master_allocator_tests.cpp bb94e38d5bb472801366c172cfc036f2eecdcbcb 
  src/tests/master_authorization_tests.cpp 
4712361021708fff1ebdb5fa34386196c10c838e 
  src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 
  src/tests/partition_tests.cpp 5a0d4bd2de6a5aa0e9fdf0d34cd10d16fd4e34a1 
  src/tests/teardown_tests.cpp 0babf8c99f133c3f0dada772bd5cd2601c47a080 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

[James Peach]

> On Nov. 17, 2016, 9:47 p.m., Jiang Yan Xu wrote:
> > src/Makefile.am, line 1019
> > 
> >
> > Keep the header file here?
> 
> James Peach wrote:
> I moved it because it makes no sense to specify files that are part of 
> `mesos-tests` in the `libmesos_no_3rdparty_la_SOURCES` variable. automake 
> doesn't actually build header files, but you still need to list them so they 
> are included in the source distribution.

@xujyan and I agreed that consistency trumps all ;)


- James


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


On Nov. 18, 2016, 6:08 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53790/
> ---
> 
> (Updated Nov. 18, 2016, 6:08 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
> ---
> 
> Move containerizer Rootfs support to a cpp file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   src/tests/containerizer/rootfs.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53790/diff/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 24)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53896: Refactored representation of framework connectedness.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Previously, the master used a bool to track whether a given framework is
connected. This commit adjusts the master to use an enum instead. The
enum currently only has two values, CONNECTED and DISCONNECTED, but an
additional value (RECOVERED) will be introduced shortly.


Diffs
-

  src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
  src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
  src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

[James Peach]

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

(Updated Nov. 18, 2016, 7:21 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
---

Move containerizer Rootfs support to a cpp file.


Diffs (updated)
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
  src/tests/containerizer/rootfs.cpp PRE-CREATION 

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


Testing
---

sudo make check (Fedora 24)


Thanks,

James Peach

Review Request 53895: Changed the allocator API to allow adding inactive frameworks.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Previously, a newly added framework was assumed to always be
active. Adding inactive frameworks will be used to support recovered
frameworks that haven't yet re-registered.


Diffs
-

  include/mesos/allocator/allocator.hpp 
3fe04f8a4fb331defbe672c739df4e137bdb3627 
  src/master/allocator/mesos/allocator.hpp 
61159a91700270b1aa6fb335b8a41592e03ef8a9 
  src/master/allocator/mesos/hierarchical.hpp 
2c31471ee0f5d6836393bf87ff9ecfd8df835013 
  src/master/allocator/mesos/hierarchical.cpp 
c8f9492ee1b69e125a1e841116d22a578a9b524e 
  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
  src/tests/allocator.hpp bce11904f0b8de66a6620b636321568fdc0d113f 
  src/tests/hierarchical_allocator_tests.cpp 
edd5cea8996d7c616cf9428f0f1c05d70c37c307 
  src/tests/master_allocator_tests.cpp bb94e38d5bb472801366c172cfc036f2eecdcbcb 
  src/tests/persistent_volume_endpoints_tests.cpp 
f35592ae311f49d3e331fa78c8e427f34bf5 
  src/tests/reservation_tests.cpp dddfbbe0fc3d0940a65a809ab8340d57dfc685c8 
  src/tests/resource_offers_tests.cpp 3dde7bf236cb88d72dce73da18d3bc55f0f9402f 
  src/tests/slave_recovery_tests.cpp 99c27399297e3b04af167029d200d8ac418af2a6 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53894: Added helper function, `Master::isCompletedFramework`.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added helper function, `Master::isCompletedFramework`.


Diffs
-

  src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53893: Avoided sending `LostSlaveMessage` to disconnected frameworks.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Since we don't attempt to send messages to disconnected frameworks, this
would only have resulted in a warning message in the master logs anyway.


Diffs
-

  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53892: Fixed typo, removed hard tab character.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Fixed typo, removed hard tab character.


Diffs
-

  include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
  include/mesos/v1/master/master.proto 022b491b7d5c49c5aeddf4ffc97c148f55629c95 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53891: Replaced "slave" with "agent" in status update messages.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Replaced "slave" with "agent" in status update messages.


Diffs
-

  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53890: Tweaked slave removal logic.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

A call to `getFramework` can be hoisted out of an inner loop.


Diffs
-

  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53889: Improved comments in the master.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Improved comments in the master.


Diffs
-

  src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53888: Improved SlaveRecoveryTest.ReconcileShutdownFramework.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Check the output of the "/state" endpoint to confirm that the framework
has been shutdown properly and the task has been marked as killed.


Diffs
-

  src/tests/slave_recovery_tests.cpp 99c27399297e3b04af167029d200d8ac418af2a6 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53887: Improved FaultToleranceTest.FrameworkReregister.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Code cleanup; also check output of the "/state" endpoint.


Diffs
-

  src/tests/fault_tolerance_tests.cpp 1ade7faee56d394de30b798982dbb6e32f81 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53886: Improved TeardownTest.Success.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Check that when a framework is shutdown, this is appropriately reflected
in the content of the master's "/state" endpoint.


Diffs
-

  src/tests/teardown_tests.cpp 0babf8c99f133c3f0dada772bd5cd2601c47a080 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53885: Removed stuttering from teardown test names.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Removed stuttering from teardown test names.


Diffs
-

  src/tests/teardown_tests.cpp 0babf8c99f133c3f0dada772bd5cd2601c47a080 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53515: Fixed a typo in slave_recovery_tests.cpp.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Nov. 6, 2016, 4:47 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53515/
> ---
> 
> (Updated Nov. 6, 2016, 4:47 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in slave_recovery_tests.cpp.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp cc50498040986e9aca1031df3622b13e7a44218a 
> 
> Diff: https://reviews.apache.org/r/53515/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

(Updated Nov. 18, 2016, 6:18 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  src/docker/docker.cpp f03ea7f 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

[James Peach]

> On Nov. 17, 2016, 9:47 p.m., Jiang Yan Xu wrote:
> > src/Makefile.am, line 1019
> > 
> >
> > Keep the header file here?

I moved it because it makes no sense to specify files that are part of 
`mesos-tests` in the `libmesos_no_3rdparty_la_SOURCES` variable. automake 
doesn't actually build header files, but you still need to list them so they 
are included in the source distribution.


- James


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


On Nov. 18, 2016, 6:08 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53790/
> ---
> 
> (Updated Nov. 18, 2016, 6:08 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
> ---
> 
> Move containerizer Rootfs support to a cpp file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   src/tests/containerizer/rootfs.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53790/diff/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 24)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53882: Fix configure on FreeBSD.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

Fix configure on FreeBSD.


Diffs
-

  configure.ac 5380cbc6a7951ede2f883f7045952a3f3434479e 

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


Testing
---

../configure


Thanks,

David Forsythe

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

[James Peach]

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

(Updated Nov. 18, 2016, 6:08 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
---

Move containerizer Rootfs support to a cpp file.


Diffs (updated)
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
  src/tests/containerizer/rootfs.cpp PRE-CREATION 

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


Testing
---

sudo make check (Fedora 24)


Thanks,

James Peach

Re: Review Request 53879: Removed duplicate note.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53879]

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. 18, 2016, 4:23 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53879/
> ---
> 
> (Updated Nov. 18, 2016, 4:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed duplicate note.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp c7ac957a9c01f93a4ed852cd26d7f2cc8e70c78e 
> 
> Diff: https://reviews.apache.org/r/53879/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

> On Nov. 18, 2016, 4:52 p.m., Gastón Kleiman wrote:
> > src/docker/docker.cpp, lines 557-561
> > 
> >
> > Do we need the tmp dir?
> > 
> > The CNI port mapper plugin doesn't create one for its tmp file:
> > 
> > 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp#L518

You are right, we don't - I was orienting this patch too close to the 
docker-home configuration temp folder.


- Till


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


On Nov. 18, 2016, 4:36 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 18, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7f 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

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

[Gastón Kleiman]

> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, line 38
> > 
> >
> > shouldn't this include be the first one in the file?
> 
> Kevin Klues wrote:
> We typically don't do that.
> We always put headers in the order:
> 
> 1) C  headers like , , etc.
> 2) C++ library headers like , , , etc.
> 3) Mesos external headers like , , 
> etc.
> 4) Mesos internal headers like the file above.
> 
> Within each category we group files at the same path depth 
> alphabetically, and separate each group by a blank line.

The Mesos and the Google C++ style guide say that the "related header" goes 
first:

> Use standard order for readability and to avoid hidden dependencies: Related 
> header, C library, C++ library, other libraries' .h, your project's .h. 
> (https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes)

And in the "Order of includes" example in 
http://mesos.apache.org/documentation/latest/c++-style-guide/


- Gastón


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


On Nov. 18, 2016, 5:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53837/
> ---
> 
> (Updated Nov. 18, 2016, 5:07 a.m.)
> 
> 
> 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 53289: Updated favicon.ico.

[Tomasz Janiszewski]

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


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 30, 2016, 1:40 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53289/
> ---
> 
> (Updated Oct. 30, 2016, 1:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-6379
> https://issues.apache.org/jira/browse/MESOS-6379
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated favicon.ico.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/ico/favicon.ico 
> 737c102678f040b8281eeb83302734c03bc3a485 
> 
> Diff: https://reviews.apache.org/r/53289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53288: Updated the brand in navbar to Mesos logo.

[Tomasz Janiszewski]

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


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 29, 2016, 4:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53288/
> ---
> 
> (Updated Oct. 29, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-6379
> https://issues.apache.org/jira/browse/MESOS-6379
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the brand in navbar to Mesos logo.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/css/mesos.css 
> 5b1227e9d64757f9fc106e497f7fa3ed72112c10 
>   src/webui/master/static/img/mesos_logo.png PRE-CREATION 
>   src/webui/master/static/index.html 6211892e7d689df9bf3b2a9071a76ad4c60d0485 
> 
> Diff: https://reviews.apache.org/r/53288/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53287: Overrided the default navbar color to match project website.

[Tomasz Janiszewski]

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


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 29, 2016, 4:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53287/
> ---
> 
> (Updated Oct. 29, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-6379
> https://issues.apache.org/jira/browse/MESOS-6379
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Overrided the default navbar color to match project website.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/css/mesos.css 
> 5b1227e9d64757f9fc106e497f7fa3ed72112c10 
> 
> Diff: https://reviews.apache.org/r/53287/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53286: Applied material style in WebUI.

[Tomasz Janiszewski]

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


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 29, 2016, 4:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53286/
> ---
> 
> (Updated Oct. 29, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-6379
> https://issues.apache.org/jira/browse/MESOS-6379
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Applied material style in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/css/bootstrap-material-design.css PRE-CREATION 
>   src/webui/master/static/css/bootstrap-material-design.min.css PRE-CREATION 
>   src/webui/master/static/css/ripples.css PRE-CREATION 
>   src/webui/master/static/css/ripples.min.css PRE-CREATION 
>   src/webui/master/static/index.html 6211892e7d689df9bf3b2a9071a76ad4c60d0485 
>   src/webui/master/static/js/app.js c32177aa23c962f2bdf03d98272fb6d21a565382 
>   src/webui/master/static/js/material.js PRE-CREATION 
>   src/webui/master/static/js/material.min.js PRE-CREATION 
>   src/webui/master/static/js/ripples.js PRE-CREATION 
>   src/webui/master/static/js/ripples.min.js PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53286/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Kevin Klues]

> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, line 38
> > 
> >
> > shouldn't this include be the first one in the file?

We typically don't do that.
We always put headers in the order:

1) C  headers like , , etc.
2) C++ library headers like , , , etc.
3) Mesos external headers like , , etc.
4) Mesos internal headers like the file above.

Within each category we group files at the same path depth alphabetically, and 
separate each group by a blank line.


> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/main.cpp, line 31
> > 
> >
> > Move this include to the top.

See above.


> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/main.cpp, line 66
> > 
> >
> > This struct is instantiated in line 80. Do we consider such a struct an 
> > `abstract class`?

Yeah the wording is a little weird, I agree. What I meant to say was that this 
class represents an "abstraction" of an FD that holds both an int for the fd 
itself as well as a promise to signal when we have detected it has become 
invalid.

That said, I am actually planning on doing a complete revamp of this file 
today, so this class will likely disappear :)


> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp, line 149
> > 
> >
> > s/spawened/spawned/

I am reworking this file today and if this comment stays around, I will make 
sure and fix it. Thanks.


> On Nov. 18, 2016, 3:28 p.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/io_switchboard/main.cpp, line 243
> > 
> >
> > I know that this is an internal binary, but we might want to print the 
> > usage and exit with a failure if `--help` is passed:
> > 
> > ```
> >   if (flags.help) {
> > EXIT(EXIT_FAILURE) << flags.usage();
> >   }
> > ```

Maybe. The only usage of this file right has the stdout/stderr of the log files 
dup'd over the programs stdou/stderr, so writing to these would write this 
information directly to the logs. I agree though, as a general mechanism, I 
should probably include printing the usage semantics indepdendent of what 
happens to be dup'd over stdout.

I'll fix this in my next iteration.


- Kevin


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


On Nov. 18, 2016, 5:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53837/
> ---
> 
> (Updated Nov. 18, 2016, 5:07 a.m.)
> 
> 
> 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 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[James Peach]

> On Nov. 17, 2016, 9:01 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/decoder.hpp, lines 253-254
> > 
> >
> > Do you happen to know what the request body length has to do with 
> > `CHAR_MAX` in the first place...?
> 
> Aaron Wood wrote:
> I'm not 100% clear on this but my guess is that it's from a negotiated 
> max body size between the server and clients within Mesos...?

AFAICT this is assigning the `Content-Length` using `basic_string& operator=( 
CharT ch );`, which is entirely broken.


- James


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


On Nov. 18, 2016, 4:24 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Nov. 18, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 76dca0b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Gastón Kleiman]

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




src/docker/docker.cpp (lines 556 - 560)


Do we need the tmp dir?

The CNI port mapper plugin doesn't create one for its tmp file:


https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp#L518


- Gastón Kleiman


On Nov. 18, 2016, 4:36 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 18, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7f 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52295: Added additional unit tests for shared resources.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/tests/hierarchical_allocator_tests.cpp (line 3532)



s/HierarchicalAllocatorCommonRoleQuotaTest/HierarchicalAllocatorTesWithParam/

Yeah it's hard to pick a concise yet descriptive name about the role and 
the quota, etc. so just leave it to the comments to explain it?



src/tests/hierarchical_allocator_tests.cpp (lines 3537 - 3538)


Add a TODO above:

```
Move over more allocator tests that make sense to run both when the role is 
quota'ed and not. 
```



src/tests/hierarchical_allocator_tests.cpp (line 3540)


s/MultiFrameworks/QuotaSwitch/

as in, the Bool() is a switch to turn quota on and off?



src/tests/hierarchical_allocator_tests.cpp (line 3542)


s/Values(false, true)/Bool()/



src/tests/hierarchical_allocator_tests.cpp (line 3546)


s/it is/they are/



src/tests/hierarchical_allocator_tests.cpp (lines 3547 - 3548)


The correct indentation would be:

```
TEST_P(HierarchicalAllocatorTesWithParam,
   AllocateSharedResources)
{
}
```

"MultiFrameworks" doesn't feel like it's serving a qualifying purpose here 
(we don't have another test for a single framework). I'd say we just use the 
shorter test name "AllocateSharedResources".



src/tests/hierarchical_allocator_tests.cpp (line 3595)


s/update/updated/ a better name?



src/tests/hierarchical_allocator_tests.cpp (lines 3609 - 3611)


Not clear to me what this means...

How about 

```
Note that the volume is not recovered as it's used by the task (but it's 
still offerable because it's shared).
```



src/tests/master_validation_tests.cpp (line 696)


No need for this variable, just use `sharedDisk` below?



src/tests/master_validation_tests.cpp (line 702)


s/""/"sleep 1000"/



src/tests/master_validation_tests.cpp (line 710)


You should be able to do:

```
pendingTasks[frameworkId1] = {{task.task_id(), task}};
```

and thus don't need the single use variable `taskMap`.



src/tests/master_validation_tests.cpp (line 718)


Remove a redundant space.



src/tests/persistent_volume_tests.cpp (line 1199)


One space before `//`.



src/tests/persistent_volume_tests.cpp (line 1203)


This line no longer relevant?



src/tests/sorter_tests.cpp (line 580)


s/likely hood/likelyhood/



src/tests/sorter_tests.cpp (line 706)


No need to wrap `sharedDisk` with `Resources()`?



src/tests/sorter_tests.cpp (line 732)


The meaning of `totalResources` here is not very clear to me: if should 
have disk of 1000 but here it's 900. Later the `shareDisk` is added to sorter 
and not to this variable.

Just simply:

```
  sorter.add(
  slaveId,
  Resources::parse("cpus:100;mem:100;disk(role1):900").get());
```
?



src/tests/sorter_tests.cpp (line 736)


s/origTotalScalarQuantity/quantity1/ ?

We are applying a sequence of operatoins to the sorter. Names like 
quantity1, quantity2 and quantity3 are as clear as these long names?



src/tests/sorter_tests.cpp (line 738)


No need to wrap it with `Resources()`?

Here and elsewhere.



src/tests/sorter_tests.cpp (line 741)


Not just "NE", we expect that

```
EXPECT_EQ(Resources::parse("disk(role1):100").get(), quantity2 - quantity1);
```

right?



src/tests/sorter_tests.cpp (line 749)


"it doesn't get removed from the sorter"

I am being pedantic but the copy does get removed, just not the quantity. 
How about

```
// The quantity of the shared disk is removed only when the last copy is 

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

(Updated Nov. 18, 2016, 4:36 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  src/docker/docker.cpp f03ea7f 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

Re: Review Request 52886: Fix new sign comparison errors in stout produced by hardened flags

[Aaron Wood]

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

(Updated Nov. 18, 2016, 4:27 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comment about not converting to an unsigned value.


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


Repository: mesos


Description
---

The hardening flags produced many new sign comparison errors in stout that need 
to be fixed for Mesos to compile/run.


Diffs (updated)
-

  3rdparty/stout/tests/cache_tests.cpp 0950c85 
  3rdparty/stout/tests/flags_tests.cpp da4deb9 
  3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
  3rdparty/stout/tests/hashset_tests.cpp 66e59db 
  3rdparty/stout/tests/ip_tests.cpp b5a206f 
  3rdparty/stout/tests/json_tests.cpp 2bc4c88 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
  3rdparty/stout/tests/multimap_tests.cpp 488991b 
  3rdparty/stout/tests/os/process_tests.cpp 4cb3b5f 
  3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
  3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
  3rdparty/stout/tests/os_tests.cpp 0b7ee07 
  3rdparty/stout/tests/strings_tests.cpp 7dd3301 

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


Testing
---

Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
Ran make && make check && make bench.


Thanks,

Aaron Wood

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

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

(Updated Nov. 18, 2016, 4:24 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comments and made a few small fixes.


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


Repository: mesos


Description
---

The hardening flags produced many new sign comparison errors in libprocess that 
need to be fixed for Mesos to compile/run.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp 76dca0b 
  3rdparty/libprocess/src/encoder.hpp 005d1cc 
  3rdparty/libprocess/src/process.cpp ab2b5a9 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
  3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
  3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
  3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 

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


Testing
---

Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
Ran `make && make check && make bench`.


Thanks,

Aaron Wood

Review Request 53879: Removed duplicate note.

[Ilya Pronin]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Removed duplicate note.


Diffs
-

  src/logging/logging.cpp c7ac957a9c01f93a4ed852cd26d7f2cc8e70c78e 

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


Testing
---


Thanks,

Ilya Pronin

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

> On Nov. 17, 2016, 9:01 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/decoder.hpp, line 21
> > 
> >
> > `#include `

My fault, I have no idea why I decided to add a .h at the time :)


- Aaron


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


On Nov. 7, 2016, 4:45 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Nov. 7, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 76dca0b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [53877]

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

Error:
2016-11-18 16:04:49 URL:https://reviews.apache.org/r/53877/diff/raw/ 
[3053/3053] -> "53877.patch" [1]
error: patch failed: src/docker/docker.cpp:532
error: src/docker/docker.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/16117/console

- Mesos ReviewBot


On Nov. 18, 2016, 3:39 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 18, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 50fda39 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

> On Nov. 17, 2016, 9:01 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/decoder.hpp, lines 253-254
> > 
> >
> > Do you happen to know what the request body length has to do with 
> > `CHAR_MAX` in the first place...?

I'm not 100% clear on this but my guess is that it's from a negotiated max body 
size between the server and clients within Mesos...?


- Aaron


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


On Nov. 7, 2016, 4:45 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Nov. 7, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 76dca0b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

(Updated Nov. 18, 2016, 3:39 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  src/docker/docker.cpp 50fda39 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

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

[Gastón Kleiman]

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




src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 38)


shouldn't this include be the first one in the file?



src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 149)


s/spawened/spawned/



src/slave/containerizer/mesos/io_switchboard/main.cpp (line 31)


Move this include to the top.



src/slave/containerizer/mesos/io_switchboard/main.cpp (line 66)


This struct is instantiated in line 80. Do we consider such a struct an 
`abstract class`?



src/slave/containerizer/mesos/io_switchboard/main.cpp (line 243)


I know that this is an internal binary, but we might want to print the 
usage and exit with a failure if `--help` is passed:

```
  if (flags.help) {
EXIT(EXIT_FAILURE) << flags.usage();
  }
```


- Gastón Kleiman


On Nov. 18, 2016, 5:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53837/
> ---
> 
> (Updated Nov. 18, 2016, 5:07 a.m.)
> 
> 
> 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 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

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


Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

[Gastón Kleiman]

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




src/examples/persistent_volume_framework.cpp (line 150)


Would it make sense to add the `shared-vol` prefix to the shard name in 
order to make debugging/troubleshooting easier?


- Gastón Kleiman


On Nov. 18, 2016, 5:13 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45962/
> ---
> 
> (Updated Nov. 18, 2016, 5:13 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated a persistent volume test framework to include shared volumes.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> 9d45bb496c4cf378af429b5aa970bf81450e482a 
> 
> Diff: https://reviews.apache.org/r/45962/diff/
> 
> 
> Testing
> ---
> 
> New test framework for shared resources added.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

[Gastón Kleiman]

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




src/examples/persistent_volume_framework.cpp (line 265)


s/upto/up to/



src/examples/persistent_volume_framework.cpp (line 280)


s/create/created/



src/examples/persistent_volume_framework.cpp (line 289)


Missing space before the opening curly brace.


- Gastón Kleiman


On Nov. 18, 2016, 5:13 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45962/
> ---
> 
> (Updated Nov. 18, 2016, 5:13 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated a persistent volume test framework to include shared volumes.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> 9d45bb496c4cf378af429b5aa970bf81450e482a 
> 
> Diff: https://reviews.apache.org/r/45962/diff/
> 
> 
> Testing
> ---
> 
> New test framework for shared resources added.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53825]

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. 18, 2016, 8:04 a.m., Vijay Srinivasaraghavan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53825/
> ---
> 
> (Updated Nov. 18, 2016, 8:04 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zameer Manji.
> 
> 
> 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
> -
> 
>   include/mesos/v1/allocator/allocator.proto 
> 73d45b37a7afc47366a4a01a36912f30b47c30b1 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
> 
> Diff: https://reviews.apache.org/r/53825/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vijay Srinivasaraghavan
> 
>

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Benjamin Mahler]

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


Ship it!





src/health-check/health_checker.cpp (line 369)


Still want these timeout variables?



src/health-check/health_checker.cpp (line 445)


Still want these timeout variables?



src/health-check/health_checker.cpp (line 569)


Still want these timeout variables?


- Benjamin Mahler


On Nov. 14, 2016, 10:28 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52867/
> ---
> 
> (Updated Nov. 14, 2016, 10:28 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
> Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally persist health check parameters from the `HealthCheck`
> protobuf as class members to avoid code duplication.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> a1dc72493ff31b87068d5691f4d5b794392caf76 
>   src/health-check/health_checker.cpp 
> e2b32e2d57515202f547d12ba492ad8eb633641b 
> 
> Diff: https://reviews.apache.org/r/52867/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Vijay Srinivasaraghavan]

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

(Updated Nov. 18, 2016, 8:04 a.m.)


Review request for mesos, Anand Mazumdar and Zameer Manji.


Changes
---

Fixed allocator proto header to include java package name. Rebased code against 
master and squashed the commits


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


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  include/mesos/v1/allocator/allocator.proto 
73d45b37a7afc47366a4a01a36912f30b47c30b1 
  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 

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


Testing
---


Thanks,

Vijay Srinivasaraghavan