Re: Review Request 42689: Clarified a comment that occurs in several test cases.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42689]

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

- Mesos ReviewBot


On Feb. 9, 2016, 5:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42689/
> ---
> 
> (Updated Feb. 9, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified a comment that occurs in several test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> bec4966daeb756c3217f52907d36991889b923ce 
>   src/tests/persistent_volume_tests.cpp 
> cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
> 
> Diff: https://reviews.apache.org/r/42689/diff/
> 
> 
> Testing
> ---
> 
> Seems like these snuck in due to copy-and-pastes that were concurrent with 
> the patch to fixup the rest of the instances of this comment.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43198: Added common appc spec utilities.

[Jojy Varghese]

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

(Updated Feb. 9, 2016, 7:40 p.m.)


Review request for mesos and Jie Yu.


Changes
---

removed getDependencies.


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


Repository: mesos


Description
---

This change adds common utility functions such as :
  - validating image information against actual data in the image directory.
  - getting list of dependencies at depth 1 for an image.
  - getting image path simple image discovery.


Diffs (updated)
-

  include/mesos/appc/spec.hpp 462159daeb5c5a75dd5c2c340d797d3cfd4d7e11 
  src/appc/spec.cpp fc36a0b5bc36c236cc4f49210d0058b34d0ffc40 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43367: Speed up FetcherCache test cases by advance allocation_interval.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [42696]

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

Error:
2016-02-09 19:18:10 URL:https://reviews.apache.org/r/42696/diff/raw/ 
[144206/144206] -> "42696.patch" [1]
Total errors found: 0
Checking 38 files
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  CHANGELOG
  Makefile.am
  configure.ac
  docs/endpoints/files/browse.json.md
  docs/endpoints/files/browse.md
  docs/endpoints/files/debug.json.md
  docs/endpoints/files/debug.md
  docs/endpoints/files/download.json.md
  docs/endpoints/files/download.md
  docs/endpoints/files/read.json.md
  docs/endpoints/files/read.md
  docs/endpoints/index.md
  docs/endpoints/logging/toggle.md
  docs/endpoints/master/api/v1/scheduler.md
  docs/endpoints/master/create-volumes.md
  docs/endpoints/master/destroy-volumes.md
  docs/endpoints/master/flags.md
  docs/endpoints/master/frameworks.md
  docs/endpoints/master/health.md
  docs/endpoints/master/machine/down.md
  docs/endpoints/master/machine/up.md
  docs/endpoints/master/maintenance/schedule.md
  docs/endpoints/master/maintenance/status.md
  docs/endpoints/master/observe.md
  docs/endpoints/master/quota.md
  docs/endpoints/master/redirect.md
  docs/endpoints/master/reserve.md
  docs/endpoints/master/roles.json.md
  docs/endpoints/master/roles.md
  docs/endpoints/master/slaves.md
  docs/endpoints/master/state-summary.md
  docs/endpoints/master/state.json.md
  docs/endpoints/master/state.md
  docs/endpoints/master/tasks.json.md
  docs/endpoints/master/tasks.md
  docs/endpoints/master/teardown.md
  docs/endpoints/master/unreserve.md
  docs/endpoints/metrics/snapshot.md
  docs/endpoints/monitor/statistics.json.md
  docs/endpoints/monitor/statistics.md
  docs/endpoints/profiler/start.md
  docs/endpoints/profiler/stop.md
  docs/endpoints/registrar/registry.md
  docs/endpoints/slave/api/v1/executor.md
  docs/endpoints/slave/flags.md
  docs/endpoints/slave/health.md
  docs/endpoints/slave/state.json.md
  docs/endpoints/slave/state.md
  docs/endpoints/system/stats.json.md
  docs/endpoints/version.md
  site/README.md
  src/Makefile.am
  src/examples/event_call_framework.cpp
  src/examples/test_framework.cpp
  src/examples/test_http_executor.cpp
  src/launcher/executor.cpp
  src/slave/containerizer/containerizer.hpp
  src/slave/containerizer/mesos/containerizer.cpp
  src/slave/containerizer/mesos/containerizer.hpp
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp
  src/slave/http.cpp
  src/slave/slave.cpp
  src/tests/balloon_framework_test.sh
  src/tests/containerizer/filesystem_isolator_tests.cpp
  src/tests/containerizer/isolator_tests.cpp
  src/tests/containerizer/launch_tests.cpp
  src/tests/containerizer/memory_test_helper.cpp
  src/tests/containerizer/mesos_containerizer_tests.cpp
  src/tests/containerizer/ns_tests.cpp
  src/tests/containerizer/port_mapping_tests.cpp
  src/tests/containerizer/provisioner_appc_tests.cpp
  src/tests/environment.cpp
  src/tests/event_call_framework_test.sh
  src/tests/examples_tests.cpp
  src/tests/fetcher_cache_tests.cpp
  src/tests/fetcher_tests.cpp
  src/tests/health_check_tests.cpp
  src/tests/mesos.cpp
  src/tests/module.cpp
  src/tests/module_tests.cpp
  src/tests/no_executor_framework_test.sh
  src/tests/oversubscription_tests.cpp
  src/tests/persistent_volume_framework_test.sh
  src/tests/script.cpp
  src/tests/slave_tests.cpp
  src/tests/test_framework_test.sh
  src/tests/uri_fetcher_tests.cpp
  src/tests/utils.cpp
  src/tests/utils.hpp
  support/atexit.sh
  support/coverage.sh
  support/docker_build.sh
  support/generate-endpoint-help.py
  support/hooks/commit-msg
  support/release.sh
  support/tag.sh
  support/vote.sh
stout:
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sunos.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp

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

- Mesos ReviewBot


On Feb. 9, 2016, 6:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43367/
> ---
> 
> (Updated Feb. 9, 2016, 6:30 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4618
> https://issues.apache.org/jira/browse/MESOS-4618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up FetcherCache 

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated Feb. 9, 2016, 7:25 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fixed error message


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 43309: Added systemd life-extension option to posix launcher.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43302, 43303, 43304, 43305, 43306, 43307, 43308, 43309]

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

- Mesos ReviewBot


On Feb. 9, 2016, 7:14 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43309/
> ---
> 
> (Updated Feb. 9, 2016, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added systemd life-extension option to posix launcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> d64832477313de2ef39aa43db67486cfa4023813 
> 
> Diff: https://reviews.apache.org/r/43309/diff/
> 
> 
> Testing
> ---
> 
> make check
> manual testing on systemd with agent-systemd-unit `KillMode=control-group`.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 43302: Libprocess: Introduced Hooks for Subprocess.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:12 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Summary (updated)
-

Libprocess: Introduced Hooks for Subprocess.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
bb50cc3070245a294fa16efe44f14ae893bc5518 
  3rdparty/libprocess/src/subprocess.cpp 
ff477e37a9619c780bddd5a8e629fa981b729715 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43201: Updated reservation documentation.

[Greg Mann]

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

(Updated Feb. 9, 2016, 7:32 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


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


Repository: mesos


Description
---

Updated reservation documentation.

Clarified differences between the RESERVE offer operation and the `/reserve` 
operator endpoint.


Diffs (updated)
-

  docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Joerg Schad]

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



Thanks for taking this on! Some small remarks below.
Overall would it make sense -given that the API is ...unstable- to add some 
tests here?


src/docker/docker.cpp (line 307)


Are these additional checks which should apply in both cases (i.e. 
deprecated and new `addressLocation`? I.e. prior to your change it did not 
return an error if `HostConfig.NetworkMode` was not set, or am I mistaken?
Feel free to drop if this is the intended behavior!



src/docker/docker.cpp (line 319)


Any reason you have a blank line here?



src/docker/docker.cpp (line 322)


We don't end log messages with a period.


hanks

- Joerg Schad


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43303: Libprocess: Added 'parent_hooks' as an argument to 'subprocess'.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:13 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Summary (updated)
-

Libprocess: Added 'parent_hooks' as an argument to 'subprocess'.


Repository: mesos


Description (updated)
---

In the future we will remove the default argument for `parent_hooks` to
force the caller to think about what kind of subprocess they want
to launch.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
bb50cc3070245a294fa16efe44f14ae893bc5518 
  3rdparty/libprocess/src/subprocess.cpp 
ff477e37a9619c780bddd5a8e629fa981b729715 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

> On Feb. 9, 2016, 1:33 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3080-3091
> > 
> >
> > This looks problematic to me now as we introduce 'status()' interface 
> > for isolators. What if there's a network isolator that wants to set the 
> > NetworkInfo in ContainerStatus using an IP that's different than the agent 
> > IP (e.g., think about the calico network isolator). In that case, we'll 
> > still set the ip in the NetworkInfo to be the agent IP?
> > 
> > My hunch is that we should combine containerizer->status() with this in 
> > the same function because logicially speaking, both of them are mutating 
> > TaskStatus with additional runtime information (ContainerStatus).
> > 
> > Maybe:
> > ```
> > Future getContainerStatus(const ContainerID& 
> > containerId)
> > {
> >   return containerizer->status()
> > .then([]() {
> >   // Set IP in NetworkInfo if not yet set.
> > });
> > }
> > ```

Modified so that we set the container IP as the IP address of the agent only if 
the NetworkInfo is not set by the isolators after `containerizer->status` has 
been invoked.


> On Feb. 9, 2016, 1:33 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3113-3114
> > 
> >
> > I am not sure if we want to set NetworkInfo in this case. Setting 
> > NetworkInfo.ip to be the agent IP is definitely problematic to me because 
> > it's unclear to me if a network isolator (e.g., calico) is used for the 
> > containr. Blindly returning the agent IP is definitely not correct.

Since, `ContainerStatus` is being updated after `containerizer->status` is 
called we shouldn't face this problem. Though if `NetworkInfo` is set by the 
`HookModules` we might end up hitting this particular case.


- Avinash


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


On Feb. 9, 2016, 8:32 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> ---
> 
> (Updated Feb. 9, 2016, 8:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4490
> https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
>   src/tests/status_update_manager_tests.cpp 
> 7bedd499a241a61938069381e0d4fafa4b8f96db 
> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Joerg Schad]

On Feb. 9, 2016, 7:13 p.m., Travis Hegner wrote:
> > hanks

Oh btw regarding the commit message:
We usually have commit messages stating what changed, so in your case it could 
be something along the lines of 'Added support for new docker network setting.'


- Joerg


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43308: Added systemd life-extension option to container logrotate module.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Added systemd life-extension option to container logrotate module.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
01a3ff031b70366443214e2895803cd4b2606ad8 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43304: Mesos: Disambiguated call to 'subprocess' in linux launcher.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:13 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Mesos: Disambiguated call to 'subprocess' in linux launcher.


Diffs (updated)
-

  src/slave/containerizer/mesos/linux_launcher.cpp 
c2e252ec6ed0d6d4c47e244f700315bd340cee5f 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43307: Added systemd life-extension option to docker process containerizer.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Added systemd life-extension option to docker process containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 19daf7d73cc8e0b0fd4afa58cc5ff7f6b7d9e345 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43306: Migrated linux launcher systemd executor logic into subprocess hook.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Migrated linux launcher systemd executor logic into subprocess hook.


Diffs (updated)
-

  src/linux/systemd.cpp 5034308cb4d1bb0b66c097daf5ec53a880cf510a 
  src/slave/containerizer/mesos/linux_launcher.cpp 
c2e252ec6ed0d6d4c47e244f700315bd340cee5f 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43309: Added systemd life-extension option to posix launcher.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Added systemd life-extension option to posix launcher.


Diffs (updated)
-

  src/slave/containerizer/mesos/launcher.cpp 
d64832477313de2ef39aa43db67486cfa4023813 

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


Testing
---

make check
manual testing on systemd with agent-systemd-unit `KillMode=control-group`.


Thanks,

Joris Van Remoortere

Re: Review Request 43305: Moved systemd executor slice initialization logic.

[Joris Van Remoortere]

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

(Updated Feb. 9, 2016, 7:13 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description (updated)
---

Moved systemd executor slice initialization logic.


Diffs (updated)
-

  src/linux/systemd.hpp dc8605b323cafdc0dde00d36a2cc5cf2c241e51c 
  src/linux/systemd.cpp 5034308cb4d1bb0b66c097daf5ec53a880cf510a 
  src/slave/containerizer/mesos/linux_launcher.hpp 
b061981b063a67d837fe8e291921ca9c44d3ac40 
  src/slave/containerizer/mesos/linux_launcher.cpp 
c2e252ec6ed0d6d4c47e244f700315bd340cee5f 
  src/slave/main.cpp 22b833044dc3f900e3b5ea509e6e545148649f48 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

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

(Updated Feb. 9, 2016, 7:35 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


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


Repository: mesos


Description
---

Updated authorization documentation.

Added information about the distinction between roles and principals, as well 
as a real-world authorization example.


Diffs (updated)
-

  docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43336: Introduced Appc image fetcher.

[Jie Yu]

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



This is a partial review. I scanned the entire patch and realized that the 
semantics of Fetcher is wiered to me.

IIUC, the Fetcher::fetch interface takes an 'Image::Appc' and a 'directory' and 
it'll download all needed layers into 'directory'.

Now, I am thinking about what will the appc::Store::get() be look like? It'll 
have to call 'Fetcher::fetcher' no matter if the name+labels is found or not in 
the cache because even if it's found in the cache, some layers might still be 
missing due to potential cache eviction of layers. Then, it sounds like 
Fetcher::fetch is doing what Store is supposed to be doing.

I am wondering if we should just do the actual fetching in Fetcher and put the 
rest of the logics in appc::Store. In other words, Fetcher::fetch will just 
download a single layer associated with 'Image::Appc', untar it, validate it 
(optional). The Store will be responsible for reading its dependencies and call 
Fetcher::fetch again for those dependencies. In that way, Cache will be 
internal to Store and you don't have to do explicit synchronization.

Based on that, appc::Fetcher can actually be a plugin under uri::Fetcher.

Happy to chat more if you have any question. Thanks Jojy!


src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (line 24)


This should be
```
#include "slave/flags.hpp"
```



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (line 48)


I think we should inject uri::Fetcher here.
```
static Try create(
const Flags& flags,
const Cache& cache,
const Shared& fetcher)
```



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (line 67)


const Owned& does not make sense. Please use:
```
Fetcher(Owned process)
```



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 57 - 59)


No need for the 'create' function here. validations should be done in 
Fetcher::create and Fetcher::create should be able to call the constructor 
directly (making the contructor public is OK since it's in a cpp file).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 59)


Why Option here? Please just use `Flags` here.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 65 - 66)


Can you swap the order of these two parameters?



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 67)


Owned does not sound right since there will be multiple 
entities in the agent sharing the same uri::Fetche in the future.

There're two options here:
1) make uri::Fetcher copyable
2) use Shared

I think 2) is easier currently (you need to mark uri::Fetcher::fetch as a 
const function).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 87 - 88)


Ditto on swapping the order of these two.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 127 - 129)


Why we need this? 'flags.appc_simple_discovery_uri_prefix' already has a 
default.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 127 - 141)


As I mentioned earlier, this should be moved to Fetcher::create (and kill 
FetcherProcess::create).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 137 - 141)


OK, this should be injected from top level.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 171 - 176)


I'd prefer wrapping comments in 70 char width (for better readability).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 181 - 183)


This looks wierd to me. do simple discovery on Error? I think cache.find 
should return an Option so that the logic here makes more sense (if not found 
in the cache, do simple discovery).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 187)


I don't see a reason why spec::getDependencies needs to return an Future. 
It's just reading a file, right? Returning a Try should be sufficient.

Also, I don't like 'getDependencies'. Let's just 

Re: Review Request 43127: Introduced Appc image cache.

[Jojy Varghese]

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

(Updated Feb. 9, 2016, 8:25 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Bugs: MESOS-4439 and MESOS-4575
https://issues.apache.org/jira/browse/MESOS-4439
https://issues.apache.org/jira/browse/MESOS-4575


Repository: mesos


Description
---

Image cache will cache metadata about all Appc images stores in the store's
image directory. This cache could be useful to:
  - Quickly query if an image is present in the store.
  - By sharing the cache with other components like fetcher, an image can be
  checked if its present before fetching it.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
4a63d479d3328605bac08fddffe190dbe21ad2b7 
  src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
af69db3cdfea05c72ecc6ed18adc9ce520ecdd7e 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
1f9b573f9388aafff3304358b8822a48075afb44 
  src/tests/containerizer/provisioner_appc_tests.cpp 
012dba4e24b9a94dc8da0d329baf4bec2d33ffca 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

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

(Updated Feb. 9, 2016, 8:32 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Modified agent to get container status from containerizer.


Diffs (updated)
-

  src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
  src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
  src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
  src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
  src/tests/status_update_manager_tests.cpp 
7bedd499a241a61938069381e0d4fafa4b8f96db 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43283: Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.

[haosdent huang]

> On Feb. 8, 2016, 10:52 p.m., Ian Downes wrote:
> > src/linux/perf.cpp, lines 423-426
> > 
> >
> > Hmmm, I'm not satisfied with this. I thought the new output format was 
> > introduced at a specific kernel version (3.13.0 might not be correct). 
> > Could you please confirm that the 3.10 on CentOS7.1 has the new unit field 
> > but other 3.10 kernels don't.

The interesting thing here is I saw `unit` appear since 3.14 
http://lxr.free-electrons.com/source/tools/perf/util/evsel.h?v=3.14#L72 I think 
I missing something, let me find out more details about this.


- haosdent


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


On Feb. 7, 2016, 8:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43283/
> ---
> 
> (Updated Feb. 7, 2016, 8:18 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jan Schlicht, and Paul Brett.
> 
> 
> Bugs: MESOS-4039
> https://issues.apache.org/jira/browse/MESOS-4039
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/43283/diff/
> 
> 
> Testing
> ---
> 
> This also fix similar error in 
> `CgroupsAnyHierarchyWithPerfEventTest.ROOT_CGROUPS_Perf`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43283: Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.

[haosdent huang]

> On Feb. 8, 2016, 10:52 p.m., Ian Downes wrote:
> > src/linux/perf.cpp, lines 423-426
> > 
> >
> > Hmmm, I'm not satisfied with this. I thought the new output format was 
> > introduced at a specific kernel version (3.13.0 might not be correct). 
> > Could you please confirm that the 3.10 on CentOS7.1 has the new unit field 
> > but other 3.10 kernels don't.
> 
> haosdent huang wrote:
> The interesting thing here is I saw `unit` appear since 3.14 
> http://lxr.free-electrons.com/source/tools/perf/util/evsel.h?v=3.14#L72 I 
> think I missing something, let me find out more details about this.

According 
https://github.com/torvalds/linux/blob/v3.13/tools/perf/builtin-stat.c#L1190-L1206
 , I think the `perf stat` format for 3.13.0 also is `value,event,cgroup`.
The `unit` added in this commit which contains since 3.14. 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=410136f5dd96b6013fe6d1011b523b1c247e1ccb
Let me try create a virtual machine to verify this.


- haosdent


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


On Feb. 7, 2016, 8:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43283/
> ---
> 
> (Updated Feb. 7, 2016, 8:18 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jan Schlicht, and Paul Brett.
> 
> 
> Bugs: MESOS-4039
> https://issues.apache.org/jira/browse/MESOS-4039
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/43283/diff/
> 
> 
> Testing
> ---
> 
> This also fix similar error in 
> `CgroupsAnyHierarchyWithPerfEventTest.ROOT_CGROUPS_Perf`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41760: Add initialization method to Anonymous module

[Marco Massenzio]

> On Feb. 8, 2016, 4:53 p.m., Kapil Arya wrote:
> > I am little concerned about `Flags` being passed to the module. Since there 
> > is no visibility about the allowed master/agent flags from the module's 
> > perspective, how does it cope with the changes to master/slave flags? Would 
> > we want to expose available master/slave flags as well in include/mesos? 
> > It's not quite clear what the correct representation would be.

Hey Kapil,

thanks for review!
I'm in the middle of something, but I'll get round to it this weekend - it all 
seems very agreeable at a quick glance.

I'll reply with more detail, but in the meantime, please note that teh idea was 
*not* expose the master/agent flags as types - the module simply iterates over 
the map (and getting the string values, not the actual typed objects); but I'm 
assuming that the flags' names will be from *fairly* to *very* stable, so the 
risk is rather low?

I think that exposing the flags in an include module would actually make 
matters worse, as it would cause backward compatibility issues and would impact 
the ability of Mesos to move forward wrt existing modules?

In any event, the way to cope with change would be as usual: for the *required* 
ones the module would have to fail (great suggestion to have a `Try` returned 
by the initialization) and for the *optional* ones, it will just have to make 
do without (possibly emitting warnings).


- Marco


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


On Jan. 4, 2016, 8:16 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41760/
> ---
> 
> (Updated Jan. 4, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Kapil Arya.
> 
> 
> Bugs: MESOS-4253
> https://issues.apache.org/jira/browse/MESOS-4253
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: [MESOS-4253](https://issues.apache.org/jira/browse/MESOS-4253)
> 
> To provide a basic "context" to Anonymous modules, we pass in the `BaseFlags`.
> 
> This tries to be backward compatible with existing modules, so a no-op 
> (`virtual`) implementation of the method is provided in the base class.
> Currently the code is only implemented in the Agent's `main()` method, but if 
> deemed correct, adding it to Master is trivial.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/anonymous.hpp 629eb123b9d5fa9e07ddb1c704ee72e96e8fec5b 
>   src/examples/test_anonymous_module.hpp 
> 3da33a42f04b7421cee8fbb85e29b66e352f5894 
>   src/examples/test_anonymous_module.cpp 
> dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/tests/anonymous_tests.cpp bc29ff8be94af82dd97f43db4f9594036705e835 
>   src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
>   src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 
> 
> Diff: https://reviews.apache.org/r/41760/diff/
> 
> 
> Testing
> ---
> 
> Unit tests pass.
> 
> Also implemented in the 
> [execute-module](http://github.com/massenz/execute-module) - and it works 
> there too:
> ```
> I0102 17:38:26.180788 19971 main.cpp:272] Initializing anonymous module 
> 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180800 19971 main.cpp:278] Sending flags to module 
> 'com_alertavert_mesos_RemoteExecution'
> I0102 17:38:26.180810 19971 execute_module.hpp:129] Executing initialize() 
> for module; parsing runtime flags
> I0102 17:38:26.181658 19971 execute_module.hpp:139] Configured work dir to 
> [/tmp/agent] and Sandbox dir to [/mnt/mesos/sandbox]
> ```
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 43283: Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.

[haosdent huang]

> On Feb. 8, 2016, 10:52 p.m., Ian Downes wrote:
> > src/linux/perf.cpp, lines 423-426
> > 
> >
> > Hmmm, I'm not satisfied with this. I thought the new output format was 
> > introduced at a specific kernel version (3.13.0 might not be correct). 
> > Could you please confirm that the 3.10 on CentOS7.1 has the new unit field 
> > but other 3.10 kernels don't.
> 
> haosdent huang wrote:
> The interesting thing here is I saw `unit` appear since 3.14 
> http://lxr.free-electrons.com/source/tools/perf/util/evsel.h?v=3.14#L72 I 
> think I missing something, let me find out more details about this.
> 
> haosdent huang wrote:
> According 
> https://github.com/torvalds/linux/blob/v3.13/tools/perf/builtin-stat.c#L1190-L1206
>  , I think the `perf stat` format for 3.13.0 also is `value,event,cgroup`.
> The `unit` added in this commit which contains since 3.14. 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=410136f5dd96b6013fe6d1011b523b1c247e1ccb
> Let me try create a virtual machine to verify this.

I try this in 

```
# cat /etc/issue
Ubuntu 14.04.3 LTS \n \l
```

Kernel is 

```
uname -a
Linux haosdent 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15 03:51:08 UTC 2014 
x86_64 x86_64 x86_64 GNU/Linux


The `perf stat` output don't contains unit:

```
perf stat --all-cpus --field-separator , --log-fd 1 --event cycles --cgroup 
mesos --event task-clock --cgroup mesos -- sleep 0.25
,cycles,mesos
,task-clock,mesos
```


- haosdent


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


On Feb. 7, 2016, 8:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43283/
> ---
> 
> (Updated Feb. 7, 2016, 8:18 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jan Schlicht, and Paul Brett.
> 
> 
> Bugs: MESOS-4039
> https://issues.apache.org/jira/browse/MESOS-4039
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix PerfEventIsolatorTest.ROOT_CGROUPS_Sample failed on CentOS 7.1.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/43283/diff/
> 
> 
> Testing
> ---
> 
> This also fix similar error in 
> `CgroupsAnyHierarchyWithPerfEventTest.ROOT_CGROUPS_Perf`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Artem Harutyunyan]

> On Feb. 6, 2016, 7:39 p.m., Guangya Liu wrote:
> > @Maged, I think that you can now find a shephard for this. You can post an 
> > email to d...@mesos.apache.org to ask for a shephard for MESOS-4353

Folks, generally the expectation is that the shepherd is found *before* the 
code gets written and not after the patch has gone through a number of 
interations on the reviewboard.


- Artem


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


On Feb. 6, 2016, 7:34 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 6, 2016, 7:34 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 42589: Added test case for allocator recover with Quota.

[Klaus Ma]

> On Feb. 1, 2016, 7:57 p.m., Alexander Rukletsov wrote:
> > What is the use case you want to test here? Specifically, why do you test 
> > *both* resume paths simulataneously? Will the outcome of the test change if 
> > you remove one of those?

This is the test case for https://reviews.apache.org/r/42535 ; without r42535, 
this test will crash.


- Klaus


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


On Jan. 21, 2016, 2:27 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42589/
> ---
> 
> (Updated Jan. 21, 2016, 2:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for allocator recover with Quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42589/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER="HierarchicalAllocatorTest.RecoverWithQuota" (CHECK() 
> failed without fix)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43358: Added support for all additional Perf Events in PerfEventIsolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43358]

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

- Mesos ReviewBot


On Feb. 9, 2016, 12:54 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43358/
> ---
> 
> (Updated Feb. 9, 2016, 12:54 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Niklas Nielsen, 
> Paul Brett, Szymon Konefal, and Cong Wang.
> 
> 
> Bugs: MESOS-4595
> https://issues.apache.org/jira/browse/MESOS-4595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dynamic map `extra_events` for all additional perf events which are not 
> specified explicitly `PerfStatistics`
> TODO in next PRs: Add test and extend description for `--perf_events=` option 
> in Mesos flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/43358/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 43312: Updated docs/presentations.md.

[Guangya Liu]

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


Ship it!




I think that you need to update https://reviews.apache.org/r/42490/ by removing 
the update for presentations.md cause it has been covered by this patch.

- Guangya Liu


On 二月 8, 2016, 5:23 a.m., Disha  Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43312/
> ---
> 
> (Updated 二月 8, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-2444
> https://issues.apache.org/jira/browse/MESOS-2444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs/presentations.md.
> 
> 
> Diffs
> -
> 
>   docs/presentations.md 2ce4b12b2a1eceb927206d7e7828bb3bbee49b52 
> 
> Diff: https://reviews.apache.org/r/43312/diff/
> 
> 
> Testing
> ---
> 
> No testing needed.
> 
> 
> Thanks,
> 
> Disha  Singh
> 
>

Re: Review Request 43321: Speeded up SchedulerTest.Decline by advancing the clock.

[Guangya Liu]

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




src/tests/scheduler_tests.cpp (line 839)


Do we need Clock::settle() here to make sure the `recoverResources` 
messages to be dispatched and processed completely?



src/tests/scheduler_tests.cpp (line 850)


Does this still needed?


- Guangya Liu


On 二月 8, 2016, 4:20 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43321/
> ---
> 
> (Updated 二月 8, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4175
> https://issues.apache.org/jira/browse/MESOS-4175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speeded up SchedulerTest.Decline by advancing the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/43321/diff/
> 
> 
> Testing
> ---
> 
> sudo make check -j2 GTEST_FILTER='ContentType/SchedulerTest.Decline/*
> 
> ```sh
> [ RUN  ] ContentType/SchedulerTest.Decline/0
> [   OK ] ContentType/SchedulerTest.Decline/0 (114 ms)
> [ RUN  ] ContentType/SchedulerTest.Decline/1
> [   OK ] ContentType/SchedulerTest.Decline/1 (98 ms)
> ```
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 42982: Defined a virtual `status` method for Containerizer.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 8, 2016, 7:19 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42982/
> ---
> 
> (Updated Feb. 8, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4487
> https://issues.apache.org/jira/browse/MESOS-4487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Defined a virtual `status` method for Containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 08d1ed3cf50f50b5b6e71df60bffbd141490ad32 
> 
> Diff: https://reviews.apache.org/r/42982/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 43359: Fixed the build with gcc 5.2.

[Bernd Mathiske]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

(An unsigned<->signed comparison was failing.)


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
c20f33ebd6b0b1703c513c0d4bfe4f7d3a9201c8 

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


Testing
---


Thanks,

Bernd Mathiske

Re: Review Request 43358: Added support for all additional Perf Events in PerfEventIsolator.

[Bartek Plotka]

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

(Updated Feb. 9, 2016, 12:54 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Niklas Nielsen, 
Paul Brett, Szymon Konefal, and Cong Wang.


Changes
---

Fixed small style issues.


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


Repository: mesos


Description (updated)
---

Added dynamic map `extra_events` for all additional perf events which are not 
specified explicitly `PerfStatistics`
TODO in next PRs: Add test and extend description for `--perf_events=` option 
in Mesos flags.


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing (updated)
---

make check


Thanks,

Bartek Plotka

Re: Review Request 43358: Added support for all additional Perf Events in PerfEventIsolator.

[Bartek Plotka]

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

(Updated Feb. 9, 2016, 12:47 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Niklas Nielsen, 
Paul Brett, Szymon Konefal, and Cong Wang.


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


Repository: mesos


Description (updated)
---

Added dynamic map extra_events for all additional perf events which are not 
specified explicitly in PerfStatistics


Diffs
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing
---


Thanks,

Bartek Plotka

Re: Review Request 43358: Added support for all additional Perf Events in PerfEventIsolator.

[Bartek Plotka]

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

(Updated Feb. 9, 2016, 12:47 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Niklas Nielsen, 
Paul Brett, Szymon Konefal, and Cong Wang.


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


Repository: mesos


Description (updated)
---

Signed-off-by: Bartlomiej Plotka 


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing
---


Thanks,

Bartek Plotka

Re: Review Request 43359: Fixed the build with gcc 5.2.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 9, 2016, 1:36 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43359/
> ---
> 
> (Updated Feb. 9, 2016, 1:36 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> (An unsigned<->signed comparison was failing.)
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> c20f33ebd6b0b1703c513c0d4bfe4f7d3a9201c8 
> 
> Diff: https://reviews.apache.org/r/43359/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 43359: Fixed the build with gcc 5.2.

[Bernd Mathiske]

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

(Updated Feb. 9, 2016, 5:45 a.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Added testing info.


Repository: mesos


Description (updated)
---

An unsigned<->signed comparison was failing.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
c20f33ebd6b0b1703c513c0d4bfe4f7d3a9201c8 

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


Testing (updated)
---

Checked that the build completes "make check" with gcc 5.2.1.


Thanks,

Bernd Mathiske

Review Request 43358: Added support for all additional Perf Events in PerfEventIsolator.

[Bartek Plotka]

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

Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Niklas Nielsen, 
Paul Brett, Szymon Konefal, and Cong Wang.


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


Repository: mesos


Description
---

Added dynamic map `extra_events` for all additional perf events which are not 
specified explicitly in `PerfStatistics`

TODO in next RRs: Unit test & extra description in `--perf_event=` mesos slave 
option.


Diffs
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing
---


Thanks,

Bartek Plotka

Re: Review Request 43384: Improved error handling in generate-endpoint-help script.

[Kevin Klues]

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



I don't mind these changes.  I had catches similar to these in eralier versions 
of this script (https://reviews.apache.org/r/43220/diff/2/), but reviews 
pointed to removing them in favor of printing the "ugly" stack traces.

- Kevin Klues


On Feb. 9, 2016, 10:43 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43384/
> ---
> 
> (Updated Feb. 9, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two problems:
> 
>   (1) If the script exited when `current_subprocess` is None, it
>   would print an ugly exception stack trace.
> 
>   (2) If starting the Mesos master or agent failed, it would print
>   an ugly exception stack trace.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py 28333847e5603c942f25ec9d9a0429bd676f4541 
> 
> Diff: https://reviews.apache.org/r/43384/diff/
> 
> 
> Testing
> ---
> 
> Ran the script in error and non-error cases with the changes applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 3:54 p.m.)


Review request for mesos, Joseph Wu and Neil Conway.


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


Repository: mesos


Description (updated)
---

There are three components to generating new doxygen pages:

1. Now that we have a Doxyfile, pass it to `doxygen` command.
2. Genarate doxygen pages in a diretcory named `doxygen` in the current
   directory.
3. Once the doxygen pages have been generated, move them into
   `publish/api/latest/c++`.

After step 3, one can copy over the publish directory to mesos-site svn
repository.


Diffs (updated)
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43198: Added common appc spec utilities.

[Jie Yu]

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




include/mesos/appc/spec.hpp (line 27)


add a blank line below



include/mesos/appc/spec.hpp (line 46)


Why Future here. I think Try should be sufficient. Let's use os::read 
instead of io::read.

Also, the parameter is a 'Path' while the function right above this 
function is using 'string' as the image path. I think we should be consistent 
here, either using string or Path for all of them.



include/mesos/appc/spec.hpp (lines 69 - 71)


Hum, the semantics of this validation function is complicated. Can we 
instead of a simpler validation function just for imagePath?
```
Option validate(const std::string& imagePath)
{
  // validate layout
  // validate manifest
  // validate image id
}
```

We can pull the name/label matching to the top level (does not belong to 
validate, maybe introduce a 'match' function in appc::Store).



src/appc/spec.cpp (line 128)


Please use os::read instead.



src/appc/spec.cpp (lines 160 - 185)


This label comparision is nasty. I think one of the reason is because we're 
using mesos::Labels in Image::Appc while appc::spec::ImageManifest is using its 
own Label.

Can you first change Image::Appc:
```
message Appc {
  required string name = 1;
  optional string id = 2;
  repeated appc::spec::Label labels = 3;
}
```
(You need to pull Label in ImageManifest to the top level).

Then, add a C++ appc::spec::Labels class to wrap the repeated protobuf 
field (like we did for Resource) in spec.hpp|cpp

Then, you can introduce a == operator for Labels in spec.hpp|cpp.



src/appc/spec.cpp (line 192)


Let's move this function to appc::Store. Simple discovery is already 
removed from the spec, we're just add this as one of our extention to the spec.



src/appc/spec.cpp (line 214)


I got confused, we do we need to call os::uname for this? The target image 
should have nothing to do with the os/arch of the host, right (even default 
value does not make sense to me).


- Jie Yu


On Feb. 9, 2016, 7:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43198/
> ---
> 
> (Updated Feb. 9, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4596
> https://issues.apache.org/jira/browse/MESOS-4596
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds common utility functions such as :
>   - validating image information against actual data in the image directory.
>   - getting list of dependencies at depth 1 for an image.
>   - getting image path simple image discovery.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.hpp 462159daeb5c5a75dd5c2c340d797d3cfd4d7e11 
>   src/appc/spec.cpp fc36a0b5bc36c236cc4f49210d0058b34d0ffc40 
> 
> Diff: https://reviews.apache.org/r/43198/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

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

- Mesos ReviewBot


On Feb. 9, 2016, 7:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 9, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

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

(Updated Feb. 9, 2016, 10:08 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Modified agent to get container status from containerizer.


Diffs
-

  src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
  src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
  src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
  src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
  src/tests/status_update_manager_tests.cpp 
7bedd499a241a61938069381e0d4fafa4b8f96db 

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


Testing (updated)
---

make and make check


Also ran sudo tests . All tests passed except for these root tests:
DockerContainerizerTest.ROOT_DOCKER_Launch_Executor (this is failing on the 
master as well, and seems flaky ?)
CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen  (Since swap was 
enabled on the system)
CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS (Since swap was enabled 
on the system)


Thanks,

Avinash sridharan

Review Request 43384: Improved error handling in generate-endpoint-help script.

[Neil Conway]

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

Review request for mesos, Ben Mahler and Kevin Klues.


Repository: mesos


Description
---

Two problems:

  (1) If the script exited when `current_subprocess` is None, it
  would print an ugly exception stack trace.

  (2) If starting the Mesos master or agent failed, it would print
  an ugly exception stack trace.


Diffs
-

  support/generate-endpoint-help.py 28333847e5603c942f25ec9d9a0429bd676f4541 

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


Testing
---

Ran the script in error and non-error cases with the changes applied.


Thanks,

Neil Conway

Re: Review Request 43306: Migrated linux launcher systemd executor logic into subprocess hook.

[Benjamin Hindman]

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




src/linux/systemd.cpp (line 76)


Should you need to do this? Isn't the expectation if a hook fails then 
`Subprocess` will take care of cleaning up the child?


- Benjamin Hindman


On Feb. 9, 2016, 7:14 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43306/
> ---
> 
> (Updated Feb. 9, 2016, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Migrated linux launcher systemd executor logic into subprocess hook.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 5034308cb4d1bb0b66c097daf5ec53a880cf510a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e252ec6ed0d6d4c47e244f700315bd340cee5f 
> 
> Diff: https://reviews.apache.org/r/43306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 43118: Corrected mistakes in docs for volume/reservation HTTP endpoints.

[Neil Conway]

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

(Updated Feb. 9, 2016, 10:53 p.m.)


Review request for mesos, Greg Mann and Jie Yu.


Changes
---

Also change endpoint help text.


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


Repository: mesos


Description
---

Corrected mistakes in docs for volume/reservation HTTP endpoints.


Diffs (updated)
-

  docs/endpoints/master/create-volumes.md 
f167452377787920414e8e1ec7712556dae29a65 
  docs/endpoints/master/destroy-volumes.md 
a9a12adc017d61f77ddacc26c84daa37657c2355 
  docs/endpoints/master/reserve.md 6d4dee457b79ffd6db1520ab49c5c3331290510e 
  docs/endpoints/master/unreserve.md 81c72d3253141586d6dbe278a07ce1ff179488c6 
  docs/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
  docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
  src/master/http.cpp 3d7a624b78fd85a8d99bce609e37411ed660678c 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 3:55 p.m.)


Review request for mesos, Joseph Wu and Neil Conway.


Changes
---

Updated Rakefile to create publish/api/latest if not already present.


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


Repository: mesos


Description (updated)
---

There are three components to generating new doxygen pages:

1. Now that we have a Doxyfile, pass it to `doxygen` command.
2. Genarate doxygen pages in a diretcory named `doxygen` in the current
   directory.
3. Once the doxygen pages have been generated, move them into
   `publish/api/latest/c++`.

After step 3, one can copy over the publish directory to mesos-site svn
repository.


Diffs
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/slave.cpp (line 3098)


Please add a second NOTE here saying that we don't set the ContainerStatus 
(including the IP address in NetworkInfo) because the container no longer 
exists. We cannot use the slave IP because it's unclear what network isolation 
is used.



src/slave/slave.cpp (line 3153)


s/containerStatus/future/



src/slave/slave.cpp (line 3155)


No need for this. Please use `StatusUpdate update` as the parameter so that 
you can directly modify it



src/slave/slave.cpp (line 3157)


s/container/containerStatus/



src/slave/slave.cpp (lines 3170 - 3183)


I think we should only do this if 'future' is Ready. Otherwise, we cannot 
tell if we should fill IP or not because a network isolator might be used and 
it advertised a different IP.



src/slave/slave.cpp (line 3188)


Kill this line


- Jie Yu


On Feb. 9, 2016, 10:08 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> ---
> 
> (Updated Feb. 9, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4490
> https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
>   src/tests/status_update_manager_tests.cpp 
> 7bedd499a241a61938069381e0d4fafa4b8f96db 
> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Also ran sudo tests . All tests passed except for these root tests:
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor (this is failing on the 
> master as well, and seems flaky ?)
> CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen  (Since swap was 
> enabled on the system)
> CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS (Since swap was 
> enabled on the system)
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43306: Migrated linux launcher systemd executor logic into subprocess hook.

[Benjamin Hindman]

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


Fix it, then Ship it!





src/linux/systemd.cpp (line 66)


I'd say this is an assert which breaks the local expectation, i.e., 
non-local code outside of this systemd "library" might call this which fails 
the CHECK. Local reasoning IMHO here would just be scoped to within the 
systemd.cpp file, not all of Mesos. I don't see any reason why we wouldn't want 
to just return an `Error` here rather than abort the process?



src/slave/containerizer/mesos/linux_launcher.cpp (line 310)


What about something like:

```c++
// Capture the freezer cgroup for use in the subprocess hook below.
string freezerCgroup = cgroup(containerId);

// Set up subprocess hooks to be executed by the parent before exec'ing the 
child.
std::vector hooks = {
  [freezerHierarchy, freezerCgroup, containerId](pid_t child) {
// Move the child into the freezer cgroup. Any grandchildren will
// also be contained in the cgroup.
// TODO(jieyu): Move this logic to the subprocess (i.e.,
// mesos-containerizer launch).
Try assign = cgroups::assign(
freezerHierarchy,
freezerCgroup,
child);

if (assign.isError()) {
  LOG(ERROR) << "Failed to assign process " << child
  << " of container '" << containerId << "'"
  << " to its freezer cgroup: " << assign.error();
  ::kill(child, SIGKILL);
  return Error("Failed to contain process");
}

return Nothing();
  }
};

// If we are on systemd, then extend the life of the child. As with the
// freezer, any grandchildren will also be contained in the slice.
if (systemdHierarchy.isSome()) {
  hooks.emplace_back(Subprocess::Hook(::extendLifetime));
}
```

And now we don't need to use any pipes at all here IIUC?



src/slave/containerizer/mesos/linux_launcher.cpp (line 312)


Can we do `::extendLifetime` instead? MPark and I ran into a bug 
previous with one of the compilers where we didn't use `&` before the global 
function and I don't see any reason not to use it always.


- Benjamin Hindman


On Feb. 9, 2016, 7:14 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43306/
> ---
> 
> (Updated Feb. 9, 2016, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Migrated linux launcher systemd executor logic into subprocess hook.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 5034308cb4d1bb0b66c097daf5ec53a880cf510a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e252ec6ed0d6d4c47e244f700315bd340cee5f 
> 
> Diff: https://reviews.apache.org/r/43306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 43319: Remove markdown files from doxygen.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 3:55 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Updated site/README.md and src/main.dox.


Summary (updated)
-

Remove markdown files from doxygen.


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


Repository: mesos


Description (updated)
---

The doxygen html pages corresponding to doc/* markdown files are
redundant and have broken links. As such, they don't serve any
reasonable purpose in doxygen site.

Also updated the main page to include direct links to libprocess/stout
developer guides.


Diffs (updated)
-

  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 
  site/README.md 71ca3ebbc645c6c188223895d12fb436b55038f9 
  src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43200: Updated role documentation.

[Neil Conway]

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




docs/roles.md (line 130)


"Principals are used to identify a framework." is a bit vague to me. How 
about "A principal identifies an entity that interacts with Mesos; principals 
are similar to user names. For example, frameworks supply a principal when they 
register with the Mesos master, and operators provide a principal when using 
the operator HTTP endpoints."



docs/roles.md (line 139)


Doesn't seem useful to have the caveat about multi-role frameworks, IMO.



docs/roles.md (line 143)


Last sentence can probably be cut.


- Neil Conway


On Feb. 8, 2016, 6:56 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated Feb. 8, 2016, 6:56 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43199: Updated authorization documentation.

[Neil Conway]

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


Fix it, then Ship it!





docs/authorization.md (line 76)


"Unix"

s/user groups/groups/

First two sentences are a little fluffy to me; I would cut them. We don't 
actually say what a principal _is_, either. Can we have a sentence like, "A 
principal identifies an entity that interacts with Mesos."



docs/authorization.md (line 116)


Maybe we should say "operating system user" instead of just "user"?


- Neil Conway


On Feb. 9, 2016, 7:35 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43199/
> ---
> 
> (Updated Feb. 9, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated authorization documentation.
> 
> Added information about the distinction between roles and principals, as well 
> as a real-world authorization example.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
> 
> Diff: https://reviews.apache.org/r/43199/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43138: Updated a comment for oversubscribed resources for clarity.

[Ben Mahler]

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




src/master/allocator/mesos/hierarchical.cpp (lines 524 - 531)


How about we document the assumptions (or invariants that are assumed 
here). The current note suggests a bug in the existing code.


- Ben Mahler


On Feb. 3, 2016, 2:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43138/
> ---
> 
> (Updated Feb. 3, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
> 
> Diff: https://reviews.apache.org/r/43138/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43316: Decrease countCacheEntries in FetcherCacheTest.SimpleEviction .

[haosdent huang]

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

(Updated Feb. 9, 2016, 6:18 p.m.)


Review request for mesos and Bernd Mathiske.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Decrease countCacheEntries in FetcherCacheTest.SimpleEviction .


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp 1cf45660691860793ac600363f7934e13a2e7ddf 

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


Testing
---

Before

```
[   OK ] FetcherCacheTest.SimpleEviction (4516 ms)
[--] 1 test from FetcherCacheTest (4516 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (4527 ms total)
[  PASSED  ] 1 test.
```

After

```
[   OK ] FetcherCacheTest.SimpleEviction (3602 ms)
[--] 1 test from FetcherCacheTest (3602 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (3612 ms total)
[  PASSED  ] 1 test.
```


Thanks,

haosdent huang

Re: Review Request 42696: Speed up FetcherCacheTest.Local* test cases.

[haosdent huang]

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

(Updated Feb. 9, 2016, 6:17 p.m.)


Review request for mesos and Bernd Mathiske.


Changes
---

Rebase


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


Repository: mesos


Description
---

Speed up FetcherCacheTest.Local* test cases.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
6b81bf1ea3d571e8f5fb170076b85223b02efe2c 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
182ac9cfbacd4535fb355aac6ec38559d786e63a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
692fa483ee8a75dedae5ad0e7d2a4aa70dafa8a3 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sunos.hpp 
ec8e1f7562d5747113631351e6d8ff8989cf0fed 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
84a2a021859d4e5c8547ad2a509eebda428a8255 
  CHANGELOG fc387dc83d919228129ea1ef4ac6c17a8fb55a04 
  Makefile.am 2b43854b0d36e4ee9c24a0c6f67480c697b8684a 
  configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
  docs/endpoints/files/browse.json.md 7c7049344980a16978a25431e713fbfe61e1cc5f 
  docs/endpoints/files/browse.md 5aa685ad616116168db852ba49e063115f7671f2 
  docs/endpoints/files/debug.json.md 3e41fec14d014a46d3a8a0ffddf7f162a39b1347 
  docs/endpoints/files/debug.md f3ff3819b14fad3aa9ddf70169c2955d18595e85 
  docs/endpoints/files/download.json.md 
77c6b97988c30ddcd71d69da1ffa55a10e871051 
  docs/endpoints/files/download.md 2b8b3f564a55b18bb84d0268b4f7a20e92b05bea 
  docs/endpoints/files/read.json.md f86f22727a7d746e8047560a88f15ab82864c062 
  docs/endpoints/files/read.md 31dd90cf0c834aca3d130f5e856fb19c7d8500cc 
  docs/endpoints/index.md 69d2157f1edf96f608e1d6eaf5a81f2421286415 
  docs/endpoints/logging/toggle.md baa4d1b60ed7c55b75f12fdf4e2c10d062bfcb48 
  docs/endpoints/master/api/v1/scheduler.md 
6faa1c2449acc54a5dc0a240959ed70a9cd7c237 
  docs/endpoints/master/create-volumes.md 
f167452377787920414e8e1ec7712556dae29a65 
  docs/endpoints/master/destroy-volumes.md 
a9a12adc017d61f77ddacc26c84daa37657c2355 
  docs/endpoints/master/flags.md b63b6e2fc837aa59341d38dab96c14bd9ed63c46 
  docs/endpoints/master/frameworks.md bc21f1e3818cf259a5ee2da258afb29afdb7b82a 
  docs/endpoints/master/health.md 39af4f963c8d84d64d4c9dafa89f4e9129242f77 
  docs/endpoints/master/machine/down.md 
82cce61e2a02f7896e7db351bed7a08138e87768 
  docs/endpoints/master/machine/up.md 5bfd95e0945d82030ee536ee247665c455629a64 
  docs/endpoints/master/maintenance/schedule.md 
e91ee81a07b09b36db9c3c9eff36f0dbb515fdd0 
  docs/endpoints/master/maintenance/status.md 
17e3eef1c2fac12375892ec125a727a62a4ebfca 
  docs/endpoints/master/observe.md acdc18c65798e90459b2b595cc3c72a11f739be2 
  docs/endpoints/master/quota.md 26c7bb162f29db1542a4ac2d61368724436e835a 
  docs/endpoints/master/redirect.md 4a230e4b7438f8b265c4f5d0a2e5b91f888b39fe 
  docs/endpoints/master/reserve.md 6d4dee457b79ffd6db1520ab49c5c3331290510e 
  docs/endpoints/master/roles.json.md d67779c246cceae2209f2611f32ada4493ae6f83 
  docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
  docs/endpoints/master/slaves.md 0be05d79d6480038ca5cfc088b7b476315514027 
  docs/endpoints/master/state-summary.md 
a6d79f0e5703c3f9118869e287fbb512e86c22c0 
  docs/endpoints/master/state.json.md e13993e12526cba47ea7bd1e6f53acc62305250d 
  docs/endpoints/master/state.md 7571d31bc0d653644fda7c0cfa2fbf3e414314d7 
  docs/endpoints/master/tasks.json.md cb1856f296d7420ce3162a60bf634de0991cdde4 
  docs/endpoints/master/tasks.md e8dbf9370433ee34fc475f5dfebfc15d3b5c62e1 
  docs/endpoints/master/teardown.md 9cd86399b532d79d0b1da451320c7f01b948d513 
  docs/endpoints/master/unreserve.md 81c72d3253141586d6dbe278a07ce1ff179488c6 
  docs/endpoints/metrics/snapshot.md ab37ab47e4a1692d805698b45d101905029747b5 
  docs/endpoints/monitor/statistics.json.md 
5ce4fc69aaa4b54541841e58ffa29703363b73e2 
  docs/endpoints/monitor/statistics.md 602104b2484022cfa7f41b04affc106703e6f09f 
  docs/endpoints/profiler/start.md 244fd6f6e4695165ff23bc33302b76974bc3f321 
  docs/endpoints/profiler/stop.md 6b9738abd8a0b4247fbd1dfd7c3c145cf1b51f9f 
  docs/endpoints/registrar/registry.md 12b11fe62edfe47cc639fd5cd5224c04d93a24f9 
  docs/endpoints/slave/api/v1/executor.md 
e92df49b0a50e0152e54866e812438c9af63c4e0 
  docs/endpoints/slave/flags.md 8abbc72f14854cf2cdaab37f9858e9427394ea7e 
  docs/endpoints/slave/health.md 265dcfaaa46dfe86dcf8ed7c5357e1ac05bb1dae 
  docs/endpoints/slave/state.json.md 10f7a7487c3ab76af7d46c5e7deddf724a3df2c1 
  docs/endpoints/slave/state.md 40df6a810b3fd2cae113959d4f42a0c19afbd5d2 
  docs/endpoints/system/stats.json.md a5fb58f35c602c8b03ed911eb374d26f98449585 
  docs/endpoints/version.md 07a9f6c7c2e0239b428dcadabc9e81f7fce05d67 
  site/README.md 78609fb68182338199b4d0ad4c333c8f34274ee5 
  src/Makefile.am 

Review Request 43367: Speed up FetcherCache test cases by advance allocation_interval.

[haosdent huang]

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

Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description
---

Speed up FetcherCache test cases by advance allocation_interval.


Diffs
-

  src/tests/fetcher_cache_tests.cpp 1cf45660691860793ac600363f7934e13a2e7ddf 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 43353: Fixed SlaveRecoveryTest.RecoverStatusUpdateManager.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 9, 2016, 2:41 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43353/
> ---
> 
> (Updated Feb. 9, 2016, 2:41 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4571
> https://issues.apache.org/jira/browse/MESOS-4571
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed SlaveRecoveryTest.RecoverStatusUpdateManager.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp bccdf37ced5de8e759c6abb91337e7bfecc77b77 
> 
> Diff: https://reviews.apache.org/r/43353/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check -j4 
> GTEST_FILTER='SlaveRecoveryTest*.RecoverStatusUpdateManager'` on ubuntu 14.04
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43384: Improved error handling in generate-endpoint-help script.

[Neil Conway]

> On Feb. 9, 2016, 10:51 p.m., Kevin Klues wrote:
> > I don't mind these changes.  I had catches similar to these in eralier 
> > versions of this script (https://reviews.apache.org/r/43220/diff/2/), but 
> > reviews pointed to removing them in favor of printing the "ugly" stack 
> > traces.

The first behavior (printing an exception on exit if `current_subprocess` 
happens to be `None`) is a bug, in my opinion.

The second behavior is more debatable, I agree.


- Neil


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


On Feb. 9, 2016, 10:43 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43384/
> ---
> 
> (Updated Feb. 9, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two problems:
> 
>   (1) If the script exited when `current_subprocess` is None, it
>   would print an ugly exception stack trace.
> 
>   (2) If starting the Mesos master or agent failed, it would print
>   an ugly exception stack trace.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py 28333847e5603c942f25ec9d9a0429bd676f4541 
> 
> Diff: https://reviews.apache.org/r/43384/diff/
> 
> 
> Testing
> ---
> 
> Ran the script in error and non-error cases with the changes applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 43381: Updated Doxyfile to use relative path.

[Kapil Arya]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Without relative path, Doxygen generated links don't work since they refer to 
the absolute path.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
96ffc427007b3b1447d291bba0d029646118d4b3 
  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
6b66bce76838046d7c955188ab9ac05e12756a83 
  3rdparty/libprocess/include/process/event.hpp 
c44dc259f240b040aa52234c7d50b1b8cc2566d9 
  3rdparty/libprocess/include/process/filter.hpp 
cafa7be065244fe428dee78c5e02e4513f8c1c35 
  3rdparty/libprocess/include/process/process.hpp 
c9ef4e86a4735c1c0342793b6d5144d80fc853a9 
  3rdparty/libprocess/src/process.cpp 65a247a5bc7cf0bd6f23f6fc5c349ecce60f5ec0 
  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 
  docs/scheduler-http-api.md 42e2c514ad027b9007aadda46eabd5c530f89eaa 
  include/mesos/scheduler/scheduler.hpp 
5d10930e26f1049eeaf16a343bf6efcf95734906 
  site/source/blog/2016-02-02-mesos-0-27-0-released.md 
3be096bf277dc2ebaa0e0c2ed1463b28a58304b3 
  site/source/index.html.md 727c216a5902cd2bf845c557b3ef0f76a087e971 
  src/Makefile.am c7507cdb0e029713da0c3218d268f5faf5269b93 
  src/common/command_utils.cpp 68b7facf62b867ac831ae616dad6623ac271110e 
  src/master/master.cpp 08d0a86906574ee8381ac2e8809d6b867cc94aa1 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/tests/common/command_utils_tests.cpp 
9207a1bd6e53a8094db47ddecdcb9d39089c4c51 
  src/tests/reservation_tests.cpp f50ac67c9aadbf229ed8e28031539892eefa9972 
  src/webui/master/static/browse.html 04a4600f0c762a2412ddee078ba2c173d595aa8d 
  src/webui/master/static/css/bootstrap-3.0.3.min.css PRE-CREATION 
  src/webui/master/static/css/bootstrap-3.3.6.min.css 
4cf729e4342a51d8b300e8d43f2f78b0a6faf403 
  src/webui/master/static/framework.html 
ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
  src/webui/master/static/home.html ad3f092a8c7ae2965f2718078c8407e19ae70f10 
  src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
  src/webui/master/static/slave.html 4735d66c03418ed964a4b720632614e9f9668df7 
  src/webui/master/static/slave_executor.html 
5acb676390fe4ed17369143c5a13202c0981 
  src/webui/master/static/slave_framework.html 
8d88466c02becd00d7c7bbfc0a4d935aaf299cef 
  support/apply-reviews.py 8dc1817943d7681b90d64c10bcd82acd7b7cef9d 

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


Testing
---


Thanks,

Kapil Arya

Review Request 43382: Updated Doxyfile to use relative path.

[Kapil Arya]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Without relative path, Doxygen generated links don't work since they refer to 
the absolute path.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
96ffc427007b3b1447d291bba0d029646118d4b3 
  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
6b66bce76838046d7c955188ab9ac05e12756a83 
  3rdparty/libprocess/include/process/event.hpp 
c44dc259f240b040aa52234c7d50b1b8cc2566d9 
  3rdparty/libprocess/include/process/filter.hpp 
cafa7be065244fe428dee78c5e02e4513f8c1c35 
  3rdparty/libprocess/include/process/process.hpp 
c9ef4e86a4735c1c0342793b6d5144d80fc853a9 
  3rdparty/libprocess/src/process.cpp 65a247a5bc7cf0bd6f23f6fc5c349ecce60f5ec0 
  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 
  docs/scheduler-http-api.md 42e2c514ad027b9007aadda46eabd5c530f89eaa 
  include/mesos/scheduler/scheduler.hpp 
5d10930e26f1049eeaf16a343bf6efcf95734906 
  site/source/blog/2016-02-02-mesos-0-27-0-released.md 
3be096bf277dc2ebaa0e0c2ed1463b28a58304b3 
  site/source/index.html.md 727c216a5902cd2bf845c557b3ef0f76a087e971 
  src/Makefile.am c7507cdb0e029713da0c3218d268f5faf5269b93 
  src/common/command_utils.cpp 68b7facf62b867ac831ae616dad6623ac271110e 
  src/master/master.cpp 08d0a86906574ee8381ac2e8809d6b867cc94aa1 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/tests/common/command_utils_tests.cpp 
9207a1bd6e53a8094db47ddecdcb9d39089c4c51 
  src/tests/reservation_tests.cpp f50ac67c9aadbf229ed8e28031539892eefa9972 
  src/webui/master/static/browse.html 04a4600f0c762a2412ddee078ba2c173d595aa8d 
  src/webui/master/static/css/bootstrap-3.0.3.min.css PRE-CREATION 
  src/webui/master/static/css/bootstrap-3.3.6.min.css 
4cf729e4342a51d8b300e8d43f2f78b0a6faf403 
  src/webui/master/static/framework.html 
ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
  src/webui/master/static/home.html ad3f092a8c7ae2965f2718078c8407e19ae70f10 
  src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
  src/webui/master/static/slave.html 4735d66c03418ed964a4b720632614e9f9668df7 
  src/webui/master/static/slave_executor.html 
5acb676390fe4ed17369143c5a13202c0981 
  src/webui/master/static/slave_framework.html 
8d88466c02becd00d7c7bbfc0a4d935aaf299cef 
  support/apply-reviews.py 8dc1817943d7681b90d64c10bcd82acd7b7cef9d 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

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

(Updated Feb. 9, 2016, 11:19 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


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


Repository: mesos


Description
---

Updated authorization documentation.

Added information about the distinction between roles and principals, as well 
as a real-world authorization example.


Diffs (updated)
-

  docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43199: Updated authorization documentation.

[Neil Conway]

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




docs/authorization.md (line 78)


I don't think a "framework is used to represent various individuals or 
groups". A framework is just a piece of software. I'd just say "principals" 
here.


- Neil Conway


On Feb. 9, 2016, 11:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43199/
> ---
> 
> (Updated Feb. 9, 2016, 11:19 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated authorization documentation.
> 
> Added information about the distinction between roles and principals, as well 
> as a real-world authorization example.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
> 
> Diff: https://reviews.apache.org/r/43199/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43200: Updated role documentation.

[Neil Conway]

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


Fix it, then Ship it!





docs/roles.md (line 129)


I'm not sure it is useful to say that "principals are associated with 
frameworks." Principals are used by frameworks, slaves (`--credential` flag), 
and operators.


- Neil Conway


On Feb. 9, 2016, 11:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated Feb. 9, 2016, 11:18 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43328: Title for documentation webpages.

[haosdent huang]

> On Feb. 10, 2016, 1:12 a.m., Neil Conway wrote:
> > We should make sure we're consistent with capitalization of titles. Also we 
> > should make the title match the Markdown title (the "#" phrase at the top 
> > of the page), not the file name.
> > 
> > Lastly, I think we should probably have a standard format for all titles 
> > that includes the word "Mesos". For example, we could use titles like 
> > "Mesos: Framework Development Guide". What do you think?

Maybe we also need change the file name to match the Markdown title?


- haosdent


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


On Feb. 8, 2016, 3:28 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43328/
> ---
> 
> (Updated Feb. 8, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-4588
> https://issues.apache.org/jira/browse/MESOS-4588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Title for documentation webpages.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   docs/architecture.md d1b3c0e7ffab1b9e70fc51282a46cf97b7718a48 
>   docs/attributes-resources.md 53a27f48c94baa9b25a072e0fc503bde0b240ba2 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
>   docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
>   docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
>   docs/committer-candidate-checklist.md 
> fd55a398c836a30ba80203a64ba10fefe2cb9108 
>   docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
>   docs/committing.md eabc0aad9c4fb96586fd1ae3bd2935e1a1a9c3a7 
>   docs/configuration.md b1ef1314eb9482a55015baa42b51d59b9f464a29 
>   docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
>   docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
>   docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
>   docs/docker-containerizer.md 89dfa0ba4b8f7b9761ce48340b30daa5a30bd8cc 
>   docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
>   docs/engineering-principles-and-practices.md 
> 6babb929ead758ee5187d5fc5760d084876c3298 
>   docs/executor-http-api.md fd80005002694bcc72abd04ceea35a2d00814401 
>   docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1 
>   docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
>   docs/framework-rate-limiting.md af33c5e9ed7c5519dc58c7c5657f3c999de3dd1f 
>   docs/frameworks.md e3d50f608ca1e5752f65c411eb09483074146562 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
>   docs/high-availability-framework-guide.md 
> 2576fa2a6e3021c34d643d3da7585fc6d95913c7 
>   docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/logging.md d79a74708775e5dc01df559042cdf790dfd20bd6 
>   docs/maintenance.md e6bfe0f655581a6a72de4579bd7e5753625c0e51 
>   docs/mesos-containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
>   docs/mesos-provisioner.md 3597cfc1d484a25f885ff95ab6ce05d615fc2c74 
>   docs/modules.md 8e0249d9829a06b3245fc193f0ee383956d33375 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 
>   docs/networking-for-mesos-managed-containers.md 
> f2fbea54cc3a7f1727c53629c86feb25d7f5fd68 
>   docs/newbie-guide.md f6e61210ed834d245fd4922c1dbbf1e7d05ff64e 
>   docs/operational-guide.md 4680ee3397d081acd6f82499703de4456e7ca4f4 
>   docs/oversubscription.md 0b1c20bf9bae9c179d82f5f611638faf91f91431 
>   docs/persistent-volume.md 3a3e370695f6e5c0cb5be24d64eb885050296c0b 
>   docs/powered-by-mesos.md 6e113ee6923d88a4d8188fa55c6c23fd8aaa35b4 
>   docs/presentations.md 2ce4b12b2a1eceb927206d7e7828bb3bbee49b52 
>   docs/quota.md 208bfa06a9fc50843439bab9a041ccb557657b5d 
>   docs/reconciliation.md ea11905720a3cd60e88b18e64e5b882f5e250166 
>   docs/release-guide.md 7c9ee140b394d61581f736f2413969d650f18fa4 
>   docs/reporting-a-bug.md b980159a7988ba952d908034317b39007c12cf2d 
>   docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
>   docs/roadmap.md 1f68ba0f63325c2f4cf5718c7afc67d39622ae9c 
>   docs/roles.md 609a63cbff2d9c652af45ba16152ce3caf48 
>   docs/sandbox.md 276e1126f495e7af15afd4f4ad8c63beb40db739 
>   docs/scheduler-http-api.md 3c0b8ff8b831dfe595652229111b7f2ca1a2b933 
>   

Re: Review Request 43319: Remove markdown files from doxygen.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:13 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Updated descr.


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


Repository: mesos


Description (updated)
---

* The doxygen html pages corresponding to doc/* markdown files are
  redundant and have broken links. As such, they don't serve any
  reasonable purpose in doxygen site.
* Also updated the main page to include direct links to libprocess/stout
  developer guides.
* Updated site/README.md to include instructions for updating doxygen
  and javadocs pages.


Diffs (updated)
-

  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 
  site/README.md 71ca3ebbc645c6c188223895d12fb436b55038f9 
  src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:13 p.m.)


Review request for mesos, Joseph Wu and Neil Conway.


Changes
---

minor.


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


Repository: mesos


Description (updated)
---

There are three components to generating new doxygen pages:

1. Now that we have a Doxyfile, pass it to `doxygen` command.
2. Genarate doxygen pages in a diretcory named `doxygen` in the current
   directory.
3. Once the doxygen pages have been generated, move them into
   `publish/api/latest/c++`.

After step 3, one can copy over the publish directory to mesos-site svn
repository.


Diffs (updated)
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

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

(Updated Feb. 9, 2016, 11:47 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Modified agent to get container status from containerizer.


Diffs (updated)
-

  src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
  src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
  src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
  src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
  src/tests/status_update_manager_tests.cpp 
7bedd499a241a61938069381e0d4fafa4b8f96db 

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


Testing
---

make and make check


Also ran sudo tests . All tests passed except for these root tests:
DockerContainerizerTest.ROOT_DOCKER_Launch_Executor (this is failing on the 
master as well, and seems flaky ?)
CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen  (Since swap was 
enabled on the system)
CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS (Since swap was enabled 
on the system)


Thanks,

Avinash sridharan

Re: Review Request 43388: Revised authentication documentation.

[Neil Conway]

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

(Updated Feb. 10, 2016, 12:10 a.m.)


Review request for mesos, Adam B, Greg Mann, and Vinod Kone.


Changes
---

Tweak link syntax.


Repository: mesos


Description
---

Revised authentication documentation.


Diffs (updated)
-

  docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 42754: Added support for labels to resource reservations.

[Michael Park]

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


Fix it, then Ship it!





include/mesos/type_utils.hpp (line 71)


We should keep the 2 newlines here. There are other parts that need 2 lines 
also, for example between `bool operator!=(const TaskStatus& left, const 
TaskStatus& right);` and `inline bool operator==(const ContainerID& left, const 
ContainerID& right)`.



src/common/resources.cpp (lines 1494 - 1500)


```
stream << label.key();

if (label.has_value()) {
  stream << ": " << label.value();
}

if (i + 1 < labels.labels().size()) {
  stream << ", ";
}
```

Same in the v1 version.



src/tests/mesos.hpp (lines 494 - 495)


Newline here



src/tests/reservation_endpoints_tests.cpp (line 743)


`s/offerResources/offeredResources/`? here and below


- Michael Park


On Feb. 5, 2016, 2:19 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42754/
> ---
> 
> (Updated Feb. 5, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4479
> https://issues.apache.org/jira/browse/MESOS-4479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Labels are free-form key-value pairs that can be used to associate
> metadata with reserved resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/type_utils.hpp 16f9cda83edd5064527b89e94392a7a54411 
>   include/mesos/v1/mesos.hpp b294f022345dc892c0581622c5b389efbde911a9 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/type_utils.cpp 42e3061818c49c6b62d4fee2479db078dfe0fc89 
>   src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
>   src/v1/mesos.cpp 21b14cf7050270facab1ba2b844c3fde664665a4 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42754/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 9, 2016, 11:47 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> ---
> 
> (Updated Feb. 9, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4490
> https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
>   src/tests/status_update_manager_tests.cpp 
> 7bedd499a241a61938069381e0d4fafa4b8f96db 
> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Also ran sudo tests . All tests passed except for these root tests:
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor (this is failing on the 
> master as well, and seems flaky ?)
> CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen  (Since swap was 
> enabled on the system)
> CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS (Since swap was 
> enabled on the system)
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43118: Corrected mistakes in docs for volume/reservation HTTP endpoints.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43118]

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

- Mesos ReviewBot


On Feb. 9, 2016, 10:53 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43118/
> ---
> 
> (Updated Feb. 9, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jie Yu.
> 
> 
> Bugs: MESOS-4421
> https://issues.apache.org/jira/browse/MESOS-4421
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected mistakes in docs for volume/reservation HTTP endpoints.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/create-volumes.md 
> f167452377787920414e8e1ec7712556dae29a65 
>   docs/endpoints/master/destroy-volumes.md 
> a9a12adc017d61f77ddacc26c84daa37657c2355 
>   docs/endpoints/master/reserve.md 6d4dee457b79ffd6db1520ab49c5c3331290510e 
>   docs/endpoints/master/unreserve.md 81c72d3253141586d6dbe278a07ce1ff179488c6 
>   docs/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
>   docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
>   src/master/http.cpp 3d7a624b78fd85a8d99bce609e37411ed660678c 
> 
> Diff: https://reviews.apache.org/r/43118/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43322: Exposed `ContainerStatus` in `state.json`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 8, 2016, 8:33 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43322/
> ---
> 
> (Updated Feb. 8, 2016, 8:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4358
> https://issues.apache.org/jira/browse/MESOS-4358
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed `ContainerStatus` in `state.json`.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp a56c3f1fdc1beec0bf842eba826279e82d4648ec 
> 
> Diff: https://reviews.apache.org/r/43322/diff/
> 
> 
> Testing
> ---
> 
> make and 'make check'
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43388: Revised authentication documentation.

[Greg Mann]

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



It's unfortunate that the docs don't currently maintain a single convention for 
line-wrapping, but they are locally consistent within each file. I would 
recommend picking either wrapping at 80 characters or not at all, and keep it 
consistent throughout this entire file.


docs/authentication.md (line 26)


Perhaps a bit misleading to call the secret an "arbitrary string" here. 
Maybe something like "the secret is used to verify that identity."?



docs/authentication.md (line 43)


Nit: perhaps true/false should be monospace formatted here, rather than 
single-quoted?



docs/authentication.md (line 67)


Not sure if you want to add it in this patch, but it would be useful to 
also have an example of credentials in JSON format.



docs/authentication.md (line 72)


Monospace formatting for the filename, here and below?


- Greg Mann


On Feb. 10, 2016, 12:10 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43388/
> ---
> 
> (Updated Feb. 10, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised authentication documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
> 
> Diff: https://reviews.apache.org/r/43388/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43319: Remove markdown files from doxygen.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:22 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


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


Repository: mesos


Description
---

* The doxygen html pages corresponding to doc/* markdown files are
  redundant and have broken links. As such, they don't serve any
  reasonable purpose in doxygen site.
* Also updated the main page to include direct links to libprocess/stout
  developer guides.
* Updated site/README.md to include instructions for updating doxygen
  and javadocs pages.


Diffs (updated)
-

  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 
  site/README.md 78609fb68182338199b4d0ad4c333c8f34274ee5 
  src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43382: Updated Doxyfile to use relative path.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:22 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description (updated)
---

Updated Doxyfile to use relative path.


Diffs (updated)
-

  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

> On Feb. 9, 2016, 10:23 p.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3186-3199
> > 
> >
> > I think we should only do this if 'future' is Ready. Otherwise, we 
> > cannot tell if we should fill IP or not because a network isolator might be 
> > used and it advertised a different IP.

Agreed.


- Avinash


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


On Feb. 9, 2016, 11:47 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> ---
> 
> (Updated Feb. 9, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4490
> https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
>   src/tests/status_update_manager_tests.cpp 
> 7bedd499a241a61938069381e0d4fafa4b8f96db 
> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Also ran sudo tests . All tests passed except for these root tests:
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor (this is failing on the 
> master as well, and seems flaky ?)
> CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen  (Since swap was 
> enabled on the system)
> CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS (Since swap was 
> enabled on the system)
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

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

(Updated Feb. 10, 2016, 12:32 a.m.)


Review request for mesos, Neil Conway and Vinod Kone.


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


Repository: mesos


Description
---

Updated authorization documentation.

Added information about the distinction between roles and principals, as well 
as a real-world authorization example.


Diffs (updated)
-

  docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

> On Feb. 9, 2016, 11:27 p.m., Neil Conway wrote:
> > docs/authorization.md, line 78
> > 
> >
> > I don't think a "framework is used to represent various individuals or 
> > groups". A framework is just a piece of software. I'd just say "principals" 
> > here.

This was a typo; intended for it to read "principals and roles might be used..."


- Greg


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


On Feb. 10, 2016, 12:32 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43199/
> ---
> 
> (Updated Feb. 10, 2016, 12:32 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated authorization documentation.
> 
> Added information about the distinction between roles and principals, as well 
> as a real-world authorization example.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
> 
> Diff: https://reviews.apache.org/r/43199/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42696: Speed up FetcherCacheTest.Local* test cases.

[haosdent huang]

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

(Updated Feb. 10, 2016, 12:48 a.m.)


Review request for mesos and Bernd Mathiske.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Speed up FetcherCacheTest.Local* test cases.


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp 1cf45660691860793ac600363f7934e13a2e7ddf 

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


Testing
---

Before apply the patch:
```
$ sudo ./bin/mesos-tests.sh --gtest_filter="FetcherCacheTest.Local*"
[   OK ] FetcherCacheTest.LocalUncached (2580 ms)
[   OK ] FetcherCacheTest.LocalCached (2524 ms)
[   OK ] FetcherCacheTest.LocalUncachedExtract (2514 ms)
[   OK ] FetcherCacheTest.LocalCachedExtract (2551 ms)
```


After apply the patch:
```
$ sudo ./bin/mesos-tests.sh --gtest_filter="FetcherCacheTest.Local*"
[   OK ] FetcherCacheTest.LocalUncached (873 ms)
[   OK ] FetcherCacheTest.LocalCached (1609 ms)
[   OK ] FetcherCacheTest.LocalUncachedExtract (926 ms)
[   OK ] FetcherCacheTest.LocalCachedExtract (1509 ms)
```

Also test the recovery test manually:

```
sudo ./bin/mesos-tests.sh 
--gtest_filter="FetcherCacheHttpRecoveryTest.HttpCachedRecovery"
```


Thanks,

haosdent huang

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[haosdent huang]

> On Feb. 9, 2016, 7:13 p.m., Joerg Schad wrote:
> > src/docker/docker.cpp, line 307
> > 
> >
> > Are these additional checks which should apply in both cases (i.e. 
> > deprecated and new `addressLocation`? I.e. prior to your change it did not 
> > return an error if `HostConfig.NetworkMode` was not set, or am I mistaken?
> > Feel free to drop if this is the intended behavior!

`HostConfig.NetworkMode` exists since api 
[1.15](https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/) I 
think it's better we use the old way to get ip first. If we get the ip by old 
way failed, try use `HostConfig.NetworkMode` and the new way to get ip again.


- haosdent


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43318: Updated release guide to include doxygen site generation.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:14 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description (updated)
---

Updated release guide to include doxygen site generation.


Diffs (updated)
-

  docs/release-guide.md 3a6b7d984222cc20c2d3cd547f74c802b3a24ce9 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43201: Updated reservation documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43199, 43200, 43201]

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

- Mesos ReviewBot


On Feb. 9, 2016, 7:32 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43201/
> ---
> 
> (Updated Feb. 9, 2016, 7:32 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated reservation documentation.
> 
> Clarified differences between the RESERVE offer operation and the `/reserve` 
> operator endpoint.
> 
> 
> Diffs
> -
> 
>   docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
> 
> Diff: https://reviews.apache.org/r/43201/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 43388: Revised authentication documentation.

[Neil Conway]

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

Review request for mesos, Adam B, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

Revised authentication documentation.


Diffs
-

  docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 43328: Title for documentation webpages.

[Neil Conway]

> On Feb. 8, 2016, 4:47 p.m., Kevin Klues wrote:
> > We recently added a script to generate markdown files for all http 
> > endpoints in the master and agent. We then commit these back to the repo.  
> > We probably need to modify this script to insert this header as well as 
> > update the files that have been committed back.  The script is 
> > atsupport/generate-endpoints-help.py and the generated files are under 
> > docs/endpoints
> 
> Abhishek Dasgupta wrote:
> Yes, that was I thinking after seeing your commit. May be, we can right 
> an automation script that would insert these header whenever there is any new 
> md file in docs folder. Can you tell me if there is any such script or else I 
> would raise an issue regarding this.

I think having a script to auto-insert the title tags is overkill for the time 
being.

Can you revise this RR to:

1. Update the `support/generate-endpoints-help.py` to include a title tag
2. Rerun the script to produce new Markdown; include the changes to the 
generated Markdown in the RR.

Thanks!


- Neil


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


On Feb. 8, 2016, 3:28 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43328/
> ---
> 
> (Updated Feb. 8, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-4588
> https://issues.apache.org/jira/browse/MESOS-4588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Title for documentation webpages.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   docs/architecture.md d1b3c0e7ffab1b9e70fc51282a46cf97b7718a48 
>   docs/attributes-resources.md 53a27f48c94baa9b25a072e0fc503bde0b240ba2 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
>   docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
>   docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
>   docs/committer-candidate-checklist.md 
> fd55a398c836a30ba80203a64ba10fefe2cb9108 
>   docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
>   docs/committing.md eabc0aad9c4fb96586fd1ae3bd2935e1a1a9c3a7 
>   docs/configuration.md b1ef1314eb9482a55015baa42b51d59b9f464a29 
>   docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
>   docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
>   docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
>   docs/docker-containerizer.md 89dfa0ba4b8f7b9761ce48340b30daa5a30bd8cc 
>   docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
>   docs/engineering-principles-and-practices.md 
> 6babb929ead758ee5187d5fc5760d084876c3298 
>   docs/executor-http-api.md fd80005002694bcc72abd04ceea35a2d00814401 
>   docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1 
>   docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
>   docs/framework-rate-limiting.md af33c5e9ed7c5519dc58c7c5657f3c999de3dd1f 
>   docs/frameworks.md e3d50f608ca1e5752f65c411eb09483074146562 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
>   docs/high-availability-framework-guide.md 
> 2576fa2a6e3021c34d643d3da7585fc6d95913c7 
>   docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/logging.md d79a74708775e5dc01df559042cdf790dfd20bd6 
>   docs/maintenance.md e6bfe0f655581a6a72de4579bd7e5753625c0e51 
>   docs/mesos-containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
>   docs/mesos-provisioner.md 3597cfc1d484a25f885ff95ab6ce05d615fc2c74 
>   docs/modules.md 8e0249d9829a06b3245fc193f0ee383956d33375 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 
>   docs/networking-for-mesos-managed-containers.md 
> f2fbea54cc3a7f1727c53629c86feb25d7f5fd68 
>   docs/newbie-guide.md f6e61210ed834d245fd4922c1dbbf1e7d05ff64e 
>   docs/operational-guide.md 4680ee3397d081acd6f82499703de4456e7ca4f4 
>   docs/oversubscription.md 0b1c20bf9bae9c179d82f5f611638faf91f91431 
>   docs/persistent-volume.md 3a3e370695f6e5c0cb5be24d64eb885050296c0b 
>   docs/powered-by-mesos.md 6e113ee6923d88a4d8188fa55c6c23fd8aaa35b4 
>   docs/presentations.md 2ce4b12b2a1eceb927206d7e7828bb3bbee49b52 
>   docs/quota.md 208bfa06a9fc50843439bab9a041ccb557657b5d 
>   docs/reconciliation.md 

Re: Review Request 43200: Updated role documentation.

[Greg Mann]

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

(Updated Feb. 10, 2016, 12:41 a.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Removed unnecessary text from "Role vs. Principal".


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


Repository: mesos


Description
---

Updated role documentation.

Added information on the distinction between roles and principals.


Diffs (updated)
-

  docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Review Request 43398: CMake: Added AppC spec protocol buffer to build.

[Alex Clemmer]

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

Review request for mesos, Jie Yu, Joris Van Remoortere, and switched to 
'mcypark'.


Repository: mesos


Description
---

CMake: Added AppC spec protocol buffer to build.


Diffs
-

  src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43328: Title for documentation webpages.

[Neil Conway]

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



We should make sure we're consistent with capitalization of titles. Also we 
should make the title match the Markdown title (the "#" phrase at the top of 
the page), not the file name.

Lastly, I think we should probably have a standard format for all titles that 
includes the word "Mesos". For example, we could use titles like "Mesos: 
Framework Development Guide". What do you think?


docs/allocation-module.md (line 2)


"Allocation Modules"



docs/app-framework-development-guide.md (line 2)


"Framework Development Guide"



docs/deploy-scripts.md (line 2)


"Deployment Scripts"



docs/frameworks.md (line 2)


Shoul be "Software projects built on Mesos", although we might want to use 
title case ("Software Projects Built On Mesos"), both here and in the markdown 
itself.



docs/high-availability-framework-guide.md (line 2)


"Designing Highly Available Mesos Frameworks"



docs/maintenance.md (line 2)


"Maintenance Primitives"



docs/monitoring.md (line 2)


"Mesos Observability Metrics"



docs/networking-for-mesos-managed-containers.md (line 2)


We should fix the title to be consistent with the markdown, for both 
hyphenation and capitalization. So I'd suggest "Networking for Mesos-Managed 
Containers"



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


"Submitting a Patch"


- Neil Conway


On Feb. 8, 2016, 3:28 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43328/
> ---
> 
> (Updated Feb. 8, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-4588
> https://issues.apache.org/jira/browse/MESOS-4588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Title for documentation webpages.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   docs/architecture.md d1b3c0e7ffab1b9e70fc51282a46cf97b7718a48 
>   docs/attributes-resources.md 53a27f48c94baa9b25a072e0fc503bde0b240ba2 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
>   docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
>   docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
>   docs/committer-candidate-checklist.md 
> fd55a398c836a30ba80203a64ba10fefe2cb9108 
>   docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
>   docs/committing.md eabc0aad9c4fb96586fd1ae3bd2935e1a1a9c3a7 
>   docs/configuration.md b1ef1314eb9482a55015baa42b51d59b9f464a29 
>   docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
>   docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
>   docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
>   docs/docker-containerizer.md 89dfa0ba4b8f7b9761ce48340b30daa5a30bd8cc 
>   docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
>   docs/engineering-principles-and-practices.md 
> 6babb929ead758ee5187d5fc5760d084876c3298 
>   docs/executor-http-api.md fd80005002694bcc72abd04ceea35a2d00814401 
>   docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1 
>   docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
>   docs/framework-rate-limiting.md af33c5e9ed7c5519dc58c7c5657f3c999de3dd1f 
>   docs/frameworks.md e3d50f608ca1e5752f65c411eb09483074146562 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
>   docs/high-availability-framework-guide.md 
> 2576fa2a6e3021c34d643d3da7585fc6d95913c7 
>   docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/logging.md d79a74708775e5dc01df559042cdf790dfd20bd6 
>   docs/maintenance.md e6bfe0f655581a6a72de4579bd7e5753625c0e51 
>   docs/mesos-containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
>   docs/mesos-provisioner.md 3597cfc1d484a25f885ff95ab6ce05d615fc2c74 
>   docs/modules.md 8e0249d9829a06b3245fc193f0ee383956d33375 
>   

Re: Review Request 43388: Revised authentication documentation.

[Adam B]

> On Feb. 9, 2016, 5:28 p.m., Adam B wrote:
> > Looks good, but I'd include a mention of FrameworkInfo.principal for use 
> > with authorization.

Probably worth linking to 
http://mesos.apache.org/documentation/latest/authorization/ there too


- Adam


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


On Feb. 9, 2016, 4:10 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43388/
> ---
> 
> (Updated Feb. 9, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised authentication documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
> 
> Diff: https://reviews.apache.org/r/43388/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43156: Added test: dynamic reservations with same role, different principals.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 5, 2016, 5:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43156/
> ---
> 
> (Updated Feb. 5, 2016, 5:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4590
> https://issues.apache.org/jira/browse/MESOS-4590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test: dynamic reservations with same role, different principals.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
> 
> Diff: https://reviews.apache.org/r/43156/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43388: Revised authentication documentation.

[Adam B]

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


Fix it, then Ship it!




Looks good, but I'd include a mention of FrameworkInfo.principal for use with 
authorization.


docs/authentication.md (line 47)


We may be deprecating the plaintext format soon



docs/authentication.md (lines 61 - 63)


To enable authorization based on the authenticated principal, the framework 
developer should also copy the Credential.principal into 
FrameworkInfo.principal when registering.



docs/authentication.md (line 86)


Step 6.?


- Adam B


On Feb. 9, 2016, 4:10 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43388/
> ---
> 
> (Updated Feb. 9, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised authentication documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
> 
> Diff: https://reviews.apache.org/r/43388/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43200: Updated role documentation.

[Greg Mann]

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

(Updated Feb. 9, 2016, 11:18 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


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


Repository: mesos


Description
---

Updated role documentation.

Added information on the distinction between roles and principals.


Diffs (updated)
-

  docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:22 p.m.)


Review request for mesos, Joseph Wu and Neil Conway.


Changes
---

rebased


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


Repository: mesos


Description
---

There are three components to generating new doxygen pages:

1. Now that we have a Doxyfile, pass it to `doxygen` command.
2. Genarate doxygen pages in a diretcory named `doxygen` in the current
   directory.
3. Once the doxygen pages have been generated, move them into
   `publish/api/latest/c++`.

After step 3, one can copy over the publish directory to mesos-site svn
repository.


Diffs (updated)
-

  site/Rakefile 10817d42eb572aa6e250a2d2fad9f3b385839f8b 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43318: Updated release guide to include doxygen site generation.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 6:22 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

Updated release guide to include doxygen site generation.


Diffs (updated)
-

  docs/release-guide.md a0d27cbafeb5311d2c704dcb632b4de179ba994c 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43382: Updated Doxyfile to use relative path.

[Kapil Arya]

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

(Updated Feb. 9, 2016, 7:16 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description (updated)
---

By default, doxygen generated html pages use absolute path for links. Thus te 
links break when these pages are moved to a different directory.


Diffs
-

  Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43388: Revised authentication documentation.

[Vinod Kone]

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


Fix it, then Ship it!





docs/authentication.md (line 11)


I would just say the same thing that we did with frameworks above

"""
To require that slaves be authenticated in order to register with the 
master.
"""



docs/authentication.md (lines 14 - 19)


Since we modularized the authenticator, there is no strict dependency on 
SASL/CRAM-MD5. So I would reword this to say that the default implementation 
uses SASL/CRAM-MD5.



docs/authentication.md (line 31)


used by the slave to run tasks/executors


- Vinod Kone


On Feb. 10, 2016, 12:10 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43388/
> ---
> 
> (Updated Feb. 10, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised authentication documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
> 
> Diff: https://reviews.apache.org/r/43388/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43398: CMake: Added AppC spec protocol buffer to build.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 10, 2016, 1:12 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43398/
> ---
> 
> (Updated Feb. 10, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and switched to 
> 'mcypark'.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added AppC spec protocol buffer to build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
> 
> Diff: https://reviews.apache.org/r/43398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 43399: Windows: Added `stout/os/read.hpp` include to `windows/os.hpp`.

[Alex Clemmer]

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

Review request for mesos, Jie Yu, Joris Van Remoortere, and switched to 
'mcypark'.


Repository: mesos


Description
---

Windows: Added `stout/os/read.hpp` include to `windows/os.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
a2fd0c7fc5f4a11918ee92601f95e8979288cd55 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 42689: Clarified a comment that occurs in several test cases.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 9, 2016, 9:13 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42689/
> ---
> 
> (Updated Feb. 9, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified a comment that occurs in several test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> bec4966daeb756c3217f52907d36991889b923ce 
>   src/tests/persistent_volume_tests.cpp 
> cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
> 
> Diff: https://reviews.apache.org/r/42689/diff/
> 
> 
> Testing
> ---
> 
> Seems like these snuck in due to copy-and-pastes that were concurrent with 
> the patch to fixup the rest of the instances of this comment.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43321: Speeded up SchedulerTest.Decline by advancing the clock.

[haosdent huang]

> On Feb. 9, 2016, 2:29 p.m., Guangya Liu wrote:
> > src/tests/scheduler_tests.cpp, line 839
> > 
> >
> > Do we need Clock::settle() here to make sure the `recoverResources` 
> > messages to be dispatched and processed completely?

+1 for add settle


> On Feb. 9, 2016, 2:29 p.m., Guangya Liu wrote:
> > src/tests/scheduler_tests.cpp, line 850
> > 
> >
> > Does this still needed?

I think still need, we don't sure what would happend in `Shutdown()` in the 
future.


- haosdent


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


On Feb. 8, 2016, 4:20 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43321/
> ---
> 
> (Updated Feb. 8, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4175
> https://issues.apache.org/jira/browse/MESOS-4175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speeded up SchedulerTest.Decline by advancing the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/43321/diff/
> 
> 
> Testing
> ---
> 
> sudo make check -j2 GTEST_FILTER='ContentType/SchedulerTest.Decline/*
> 
> ```sh
> [ RUN  ] ContentType/SchedulerTest.Decline/0
> [   OK ] ContentType/SchedulerTest.Decline/0 (114 ms)
> [ RUN  ] ContentType/SchedulerTest.Decline/1
> [   OK ] ContentType/SchedulerTest.Decline/1 (98 ms)
> ```
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43388: Revised authentication documentation.

[Neil Conway]

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

(Updated Feb. 10, 2016, 2:06 a.m.)


Review request for mesos, Adam B, Greg Mann, and Vinod Kone.


Changes
---

Address review comments.


Repository: mesos


Description
---

Revised authentication documentation.


Diffs (updated)
-

  docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway