Re: Review Request 43136: Support libprocess build run in aarch64 cpu.

2016-02-04 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [43136]

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

Error:
2016-02-04 10:52:41 URL:https://reviews.apache.org/r/43136/diff/raw/ 
[1711/1711] -> "43136.patch" [1]
Total errors found: 0
Checking 2 files
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  src/tests/containerizer/routing_tests.cpp
stout:
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp

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

- Mesos ReviewBot


On Feb. 4, 2016, 8:56 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43136/
> ---
> 
> (Updated Feb. 4, 2016, 8:56 a.m.)
> 
> 
> Review request for mesos, BenjaminVW BenjaminVW, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-4577
> https://issues.apache.org/jira/browse/MESOS-4577
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support libprocess build run in aarch64 cpu.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> f8a4d5ae12cfc2d2bd4a24226e200e1aabe0b8a8 
>   src/tests/containerizer/routing_tests.cpp 
> 4ebb01213ce20f954c2633df95e898cf66a2d9e9 
> 
> Diff: https://reviews.apache.org/r/43136/diff/
> 
> 
> Testing
> ---
> 
> test run ok in AArch64.
> 
> 
> Thanks,
> 
> Andy Pang
> 
>



Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

2016-02-04 Thread Alexander Rukletsov

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




include/mesos/mesos.proto (lines 643 - 649)


This looks like an internal information, the *source* of a revocable 
resource. While we definitely need this is the allocator, I'm not sure we 
should expose it in the public protobuf. I think frameworks are more interested 
in the *quality* of a resource. Can we keep the source private somehow?


- Alexander Rukletsov


On Jan. 7, 2016, 3:43 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Jan. 7, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 43183: Support routing_tests run in aarch64 cpu.

2016-02-04 Thread haosdent huang

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


Ship it!




Please also fill the `Bug` field to `MESOS-4577` in this patch.

- haosdent huang


On Feb. 4, 2016, 9:34 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43183/
> ---
> 
> (Updated Feb. 4, 2016, 9:34 a.m.)
> 
> 
> Review request for mesos, BenjaminVW BenjaminVW, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support routing_tests run in aarch64 cpu.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 4ebb01213ce20f954c2633df95e898cf66a2d9e9 
> 
> Diff: https://reviews.apache.org/r/43183/diff/
> 
> 
> Testing
> ---
> 
> make test ok
> 
> 
> Thanks,
> 
> Andy Pang
> 
>



Re: Review Request 41871: Replaced libtool with dolt to speed up compiler invocations.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [41871]

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 Jan. 11, 2016, 5:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41871/
> ---
> 
> (Updated Jan. 11, 2016, 5:22 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4271
> https://issues.apache.org/jira/browse/MESOS-4271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The added m4 macro definitions come from
> git://anongit.freedesktop.org/git/dolt revision
> 2eae53fa6d31175e9de62a312cc2191fd8a25cb7.
> 
> Review: https://reviews.apache.org/r/41871
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 40d60a63cdba41d06305f09141f4d14d6e229d95 
>   m4/dolt.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41871/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 43182: Support libprocess build run in aarch64 cpu.

2016-02-04 Thread haosdent huang

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


Ship it!




Actually I not sure whether use a var to replace `2` or `1` would better or 
not. If it really could works on aarch64(I don't have a aarch64 machine), it 
looks good to me.

- haosdent huang


On Feb. 4, 2016, 9:33 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43182/
> ---
> 
> (Updated Feb. 4, 2016, 9:33 a.m.)
> 
> 
> Review request for mesos, BenjaminVW BenjaminVW, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-4577
> https://issues.apache.org/jira/browse/MESOS-4577
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support libprocess build run in aarch64 cpu.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> f8a4d5ae12cfc2d2bd4a24226e200e1aabe0b8a8 
> 
> Diff: https://reviews.apache.org/r/43182/diff/
> 
> 
> Testing
> ---
> 
> test run ok in AArch64.
> 
> 
> Thanks,
> 
> Andy Pang
> 
>



Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43032]

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. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 43082: Added new flag to command executor for command passing.

2016-02-04 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Feb. 3, 2016, 8:42 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43082/
> ---
> 
> (Updated Feb. 3, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4004
> https://issues.apache.org/jira/browse/MESOS-4004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new flag to command executor for command passing.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
> 
> Diff: https://reviews.apache.org/r/43082/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 42957: Added remove() calls to process::Help.

2016-02-04 Thread Benjamin Bannier

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




3rdparty/libprocess/src/help.cpp (line 107)


How could the 2nd part of the condition arise? -- below you perform pruning 
of empty branches.

I think either the 2nd part is redundant, or you should perform pruning 
before returning `false`.



3rdparty/libprocess/src/help.cpp (lines 123 - 129)


Is this equivalent to just `return helps.erase(id)`?


- Benjamin Bannier


On Feb. 4, 2016, 9:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> ---
> 
> (Updated Feb. 4, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, there was no way to remove an installed help
> string for a given endpoint.
> 
> This commit adds the ability to remove these help strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 2e76a6a5b1069abce879374a88cea65036873f1d 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/42957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

2016-02-04 Thread Avinash sridharan

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

(Updated Feb. 4, 2016, 9:03 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added unit-test for `NetClsHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan



Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

2016-02-04 Thread Anand Mazumdar

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

(Updated Feb. 4, 2016, 11:11 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 43167: Added working dir flag to mesos containerizer launch.

2016-02-04 Thread Jie Yu

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




src/slave/containerizer/mesos/launch.cpp (line 56)


The sandbox for the executor.



src/slave/containerizer/mesos/launch.cpp (lines 61 - 62)


The working directory for the executor. If will be ignored if container 
root filesystem is not specified.



src/slave/containerizer/mesos/launch.cpp (lines 258 - 283)


I would combine the logics here so that we don't end up with two chdir in 
some cases.
```
// Determine the current working directory for the executor.
string cwd;
if (flags.rootfs.isSome() && flags.working_directory.isSome()) {
  cwd = flags.working_directory.get();
} else {
  cwd = flags.sandbox.get();
}

Try chdir = os::chdir(cwd);
if (chdir.isError()) {
  ...
}

```



src/slave/containerizer/mesos/launch.cpp (lines 276 - 281)


Let's put this warning to Mesos containerizer.


- Jie Yu


On Feb. 4, 2016, 12:35 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43167/
> ---
> 
> (Updated Feb. 4, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added working dir flag to mesos containerizer launch.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 722031956494647c81436a95400d939eec4466de 
>   src/slave/containerizer/mesos/launch.cpp 
> 0cfdc0ca1a7e8c6107f8ea46e7a4c6e52811d7ac 
> 
> Diff: https://reviews.apache.org/r/43167/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Review Request 43223: Pin intra-package python dependencies to the same internal version

2016-02-04 Thread Benjamin Staffin

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

Review request for mesos.


Repository: mesos


Description
---

Avoids weird situations that arise from having differing versions of
mesos.native and/or mesos.interface on the same system, especially
while building system packages (e.g. debian builds)


Diffs
-

  src/python/setup.py.in 737066952fe72382bcf80ca6d3e8457ea07a65bf 

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


Testing
---


Thanks,

Benjamin Staffin



Re: Review Request 43081: Supported entrypoint and cmd in docker runtime isolator.

2016-02-04 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Feb. 4, 2016, 6:58 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43081/
> ---
> 
> (Updated Feb. 4, 2016, 6:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4004
> https://issues.apache.org/jira/browse/MESOS-4004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported entrypoint and cmd in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43081/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



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

2016-02-04 Thread Travis Hegner

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


Changes
---

Couple of build errors fixed.


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


Repository: mesos


Description
---

Fixes [MESOS-4370]


Diffs (updated)
-

  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



Review Request 43207: Don't remove IP from the logger's environment.

2016-02-04 Thread Joseph Wu

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


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


Repository: mesos


Description
---

Unsetting the `LIBPROCESS_IP` was originally unnecessary, as this variable does 
not result in port conflicts.  Unsetting this environment variable may cause 
the subprocess to exit if DNS cannot resolve the agent's hostname.


Diffs
-

  src/slave/container_loggers/lib_logrotate.cpp 
8d2f89503cd8ace1971933006a46ed4d3936ff8e 

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


Testing
---

make check (OSX)


Thanks,

Joseph Wu



Re: Review Request 41290: Modified `TestContainerizer` to handle HTTP based executors.

2016-02-04 Thread Anand Mazumdar

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

(Updated Feb. 4, 2016, 11:12 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change modifies the `TestContainerizer` class to handle HTTP based 
executors. Previously, the containerizer test class only used to handle the 
driver interface.


Diffs (updated)
-

  src/tests/containerizer.hpp bd9ee2c189b774979095f8483b4e9ba41a7bdb24 
  src/tests/containerizer.cpp 15c8b19f3116e60d80671c64ff33580b552dc1ec 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 43168: Added working dir flag to command executor.

2016-02-04 Thread Jie Yu

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




src/launcher/executor.cpp (line 215)


Can you add a TODO here saying that this is a hacky way to detect if root 
filesystem is specified for the command task.



src/launcher/executor.cpp (lines 351 - 364)


Ditto on merging them so that we don't chdir twice.
```
string cwd;
if (workingDirectory.isSome()) {
  cwd = workingDirectory.get();
} else {
  CHECK_SOME(sandboxDirectory);
  cwd = sandboxDirectory.get();
}

Try chdir = os::chdir(cwd);
if (chdir.isError()) {
  ...
}
```



src/launcher/executor.cpp (lines 780 - 781)


Adjust the help message here according to my comments in the other review.


- Jie Yu


On Feb. 4, 2016, 12:35 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43168/
> ---
> 
> (Updated Feb. 4, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added working dir flag to command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
> 
> Diff: https://reviews.apache.org/r/43168/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

2016-02-04 Thread Avinash sridharan


> On Feb. 4, 2016, 7:21 p.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, lines 374-395
> > 
> >
> > Hum, I don't think this test fixture is needed. Just create 
> > handleManager in each test.
> > 
> > ```
> > NetClsHandleManager manager(
> > (Bound::closed(2),
> >  Bound::closed(3));
> >  
> > for (int primary = 2; primary <= 3; primary++) {
> >   ...
> > }
> > 
> > for (int primary = 2; primary <= 3; primary++) {
> >   ...
> > }
> > ```

Couldn't use (Bound::closed(2),
 Bound::closed(3) had to use 
Intervalset((Bound::closed(2),
 Bound::closed(3))


> On Feb. 4, 2016, 7:21 p.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, lines 411-412
> > 
> >
> > These two can be combined into:
> > ```
> > EXPECT_SOME_TRUE(manager->isUsed(handle.get());
> > ```

Thanks !! Simplified.


- Avinash


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


On Feb. 4, 2016, 9:03 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42588/
> ---
> 
> (Updated Feb. 4, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit-test for `NetClsHandleManager`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
>   src/tests/containerizer/isolator_tests.cpp 
> 8d101df957fd36adac388310eddba2db1f98c029 
> 
> Diff: https://reviews.apache.org/r/42588/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 42957: Added remove() calls to process::Help.

2016-02-04 Thread Kevin Klues


> On Feb. 4, 2016, 9:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/help.cpp, lines 123-129
> > 
> >
> > Is this equivalent to just `return helps.erase(id)`?

Yes, these are equivalent. Updated.


> On Feb. 4, 2016, 9:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/help.cpp, line 107
> > 
> >
> > How could the 2nd part of the condition arise? -- below you perform 
> > pruning of empty branches.
> > 
> > I think either the 2nd part is redundant, or you should perform pruning 
> > before returning `false`.

This case can occur if an invalid name is passed in that is not contained in 
the map.  That said, I've updated the logic to be more clear.


- Kevin


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


On Feb. 4, 2016, 8:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> ---
> 
> (Updated Feb. 4, 2016, 8:30 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, there was no way to remove an installed help
> string for a given endpoint.
> 
> This commit adds the ability to remove these help strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 2e76a6a5b1069abce879374a88cea65036873f1d 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/42957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42957: Added remove() calls to process::Help.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 4, 2016, 10:05 p.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


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


Repository: mesos


Description
---

Previously, there was no way to remove an installed help
string for a given endpoint.

This commit adds the ability to remove these help strings.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
2e76a6a5b1069abce879374a88cea65036873f1d 
  3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 

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


Testing
---


Thanks,

Kevin Klues



Review Request 43225: Introduced gtest actions for subscribe, sending updates.

2016-02-04 Thread Anand Mazumdar

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds some gtest actions that can be used to subscribe/send status 
updates using the executor library. These are in similar vein to the existing 
actions like `SendStatusUpdateFromTask`.


Diffs
-

  src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

2016-02-04 Thread Anand Mazumdar

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

(Updated Feb. 4, 2016, 11:12 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new executor HTTP 
library instead of the old driver based interface.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

2016-02-04 Thread Anand Mazumdar


> On Feb. 3, 2016, 11:41 p.m., Vinod Kone wrote:
> > src/tests/scheduler_tests.cpp, line 201
> > 
> >
> > s/execCallbacks/executorCallbacks/

Renamed this to `executor` now as per our discussion.


> On Feb. 3, 2016, 11:41 p.m., Vinod Kone wrote:
> > src/tests/scheduler_tests.cpp, lines 303-305
> > 
> >
> > It would be gret if we can send a TASK_RUNNING update right from here, 
> > like we did with the driver based executor to minimize the boiler plate 
> > code needed for every test.
> > 
> > This likely means that the ExecutorCallbacks class needs access to a 
> > pointer of TestMesos class.

Added the ability to pass the pointer to `TestMesos`. Cleaned up things a lot. 
Thanks for the suggestion.


> On Feb. 3, 2016, 11:41 p.m., Vinod Kone wrote:
> > src/tests/scheduler_tests.cpp, line 308
> > 
> >
> > s/exec/executor/

We no longer get access to `TestMesos` now. Hence, the issue is no longer 
valid. Dropping.


- Anand


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


On Feb. 4, 2016, 11:12 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41291/
> ---
> 
> (Updated Feb. 4, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4457
> https://issues.apache.org/jira/browse/MESOS-4457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the existing scheduler tests to use the new executor 
> HTTP library instead of the old driver based interface.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/41291/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41290: Modified `TestContainerizer` to handle HTTP based executors.

2016-02-04 Thread Anand Mazumdar


> On Feb. 3, 2016, 10:55 p.m., Vinod Kone wrote:
> > src/tests/containerizer.hpp, line 140
> > 
> >
> > s/v1callbacks/v1Callbacks/

Modified it to `v1Executors` now.


> On Feb. 3, 2016, 10:55 p.m., Vinod Kone wrote:
> > src/tests/containerizer.cpp, lines 104-108
> > 
> >
> > move this to #163

As per our offline discussion, we save the container to an existing map after 
we perform this check. If we move this to L163, we might have erroneously 
inserted a container without having an executor to launch it.


- Anand


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


On Feb. 4, 2016, 11:12 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41290/
> ---
> 
> (Updated Feb. 4, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4457
> https://issues.apache.org/jira/browse/MESOS-4457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the `TestContainerizer` class to handle HTTP based 
> executors. Previously, the containerizer test class only used to handle the 
> driver interface.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.hpp bd9ee2c189b774979095f8483b4e9ba41a7bdb24 
>   src/tests/containerizer.cpp 15c8b19f3116e60d80671c64ff33580b552dc1ec 
> 
> Diff: https://reviews.apache.org/r/41290/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42186: Added tests for recovery for HTTP based executors.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42181, 43131, 42844, 42185, 42186]

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. 4, 2016, 7:42 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42186/
> ---
> 
> (Updated Feb. 4, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
> 
> Diff: https://reviews.apache.org/r/42186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 43166: Renamed 'directory' flag to 'sandbox' in mesos containerizer launch.

2016-02-04 Thread Jie Yu

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


Ship it!




You forgot to modify port mapping tests. I'll fix it for you.

- Jie Yu


On Feb. 4, 2016, 12:35 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43166/
> ---
> 
> (Updated Feb. 4, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed 'directory' flag to 'sandbox' in mesos containerizer launch.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/launch.hpp 
> 722031956494647c81436a95400d939eec4466de 
>   src/slave/containerizer/mesos/launch.cpp 
> 0cfdc0ca1a7e8c6107f8ea46e7a4c6e52811d7ac 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
> 
> Diff: https://reviews.apache.org/r/43166/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 43240: Removed implicit, value changing conversion.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43240]

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. 5, 2016, 3:28 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43240/
> ---
> 
> (Updated Feb. 5, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whether `char` is signed or not is up to the implementation, so be more 
> explicit
> here. Also, since we want a `char` with all bits set, use a value 
> representation
> that more clearly communicates that.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
> 
> Diff: https://reviews.apache.org/r/43240/diff/
> 
> 
> Testing
> ---
> 
> When compiling with trunk clang, compilation fails with
> 
> ../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
> from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
> [-Wconstant-conversion]
> 
> After inserting the explicit cast the code compiles fine.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



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

2016-02-04 Thread Klaus Ma


> On Feb. 4, 2016, 9:38 p.m., Klaus Ma wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2211
> > 
> >
> > Just `os::getenv("LIB...")` is OK.
> 
> Maged Michael wrote:
> The string is used in two places: getenv and VLOG(WARNING). I thought to 
> avoid mismatch between the two literals.
> How about 
>constexpr auto env_var = "LIBPROCESS_MAX_WORKER_THREADS";
> ?

That's OK to me.


- Klaus


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


On Feb. 5, 2016, 3:55 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> 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 
> 
> 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 43168: Added working dir flag to command executor.

2016-02-04 Thread Gilbert Song

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

(Updated Feb. 4, 2016, 5:24 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added working dir flag to command executor.


Diffs (updated)
-

  src/launcher/executor.cpp b214a3f876a023c1d68dab4288b18fe5068b2b5a 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 43217: Added ability to return the /help endpoint as a JSON object.

2016-02-04 Thread Ben Mahler

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




3rdparty/libprocess/src/help.cpp (lines 187 - 188)


You should be able to use the (Option, T) equality operator to just do 
the following:

```
if (request.url.query.get("format") == "json") {
```

Although we probably have to do the following because of the string literal:

```
if (request.url.query.get("format") == Some("json")) {
```



3rdparty/libprocess/src/help.cpp (lines 189 - 191)


This should be setting the content type already:

```
return http::OK(jsonify(*this));
```


- Ben Mahler


On Feb. 4, 2016, 8:39 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43217/
> ---
> 
> (Updated Feb. 4, 2016, 8:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the /help endpoint could only be returned as either pure
> markdown or html.
> 
> This commit introduces a query parameter: format=json to allow the
> help endpoint to return a JSON object containing the help information
> for all other endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/43217/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

2016-02-04 Thread Avinash sridharan

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

(Updated Feb. 5, 2016, 2:16 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added unit-test for `NetClsHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
85004aea4c1ee4b25e106f3ce40025c69f1ce030 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 
  src/tests/containerizer/isolator_tests.cpp 
67322abc776cbd501385932676852a79b74ef248 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan



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

2016-02-04 Thread Guangya Liu


> On 二月 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2212-2213
> > 
> >
> > Why not 
> > 
> > Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");

I see what you mean: it will be used in the log message.


- Guangya


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


On 二月 4, 2016, 7:55 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 4, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> 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 
> 
> 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 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Guangya Liu

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




src/docker/docker.cpp (lines 533 - 538)


I prefer that we put version logic inside the block of `if 
(labels.isSome()) {` to improve performance.

if (labels.isSome()) {
   get version;
   if (version.isReady()) {
 if version.get() < Version(1, 6, 0) {
   error
 } else {
   handle labels.
 }
  }
}
}


- Guangya Liu


On 二月 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [41288, 43225, 41290, 41291]

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. 4, 2016, 11:12 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41291/
> ---
> 
> (Updated Feb. 4, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4457
> https://issues.apache.org/jira/browse/MESOS-4457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the existing scheduler tests to use the new executor 
> HTTP library instead of the old driver based interface.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/41291/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 43200: Updated role documentation.

2016-02-04 Thread Guangya Liu

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




docs/roles.md (line 21)


Maybe `Configure Mesos to provide guaranteed resource allocations for use 
by a role.` is better? You can refer to `home.md` to check the explanation for 
`quota.md`


- Guangya Liu


On 二月 4, 2016, 7:55 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated 二月 4, 2016, 7:55 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 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta

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

(Updated Feb. 5, 2016, 7:15 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta



Re: Review Request 43083: Supported working dir in docker runtime isolator.

2016-02-04 Thread Jie Yu

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




src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 97 - 118)


How about restructuring the code like the following:

```

Option environment = getEnvironment(
containerId,
containerConfig);

Option workingDirectory = getWorkingDirectory(
containerId,
containerConfig);

Try command = getLaunchCommand(
containerId,
containerConfig);

if (command.isError()) {
  ...
}

// Set 'launchInfo'.
ContainerLaunchInfo launchInfo;

if (environment.isSome()) {
  launchInfo.mutable_environment()->CopyFrom(environment.get());
}

if (!containerConfig.has_task_info()) {
  // Custom executor case.
  launchInfo.set_working_directory(workingDirectory.get());
  launchInfo.mutable_command()->CopyFrom(command.get());
} else {
  // Command task case.
  CommandInfo executorCommand = containerConfig.executor_info().command();
  
  if (environment.isSome()) {
launchInfo.mutable_environment()->CopyFrom(environment.get());
  }
  
  // Pass working directory to command executor as a flag.
  if (workingDirectory.isSome()) {
executorCommand.add_arguments(
"--working_directory=" + workingDirectory.get());
  }
  
  // Pass task command as a flag, which will be loaded by
  // command executor.
  executorCommand.add_arguments(
  "--task_command=" +
  stringify(JSON::protobuf(command.get(;
  
  launchInfo.mutable_command()->CopyFrom(executorCommand);
}
```



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 323)


s/getWorkingDir/getWorkingDirectory/


- Jie Yu


On Feb. 4, 2016, 12:49 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43083/
> ---
> 
> (Updated Feb. 4, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported working dir in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43083/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

2016-02-04 Thread Jie Yu

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp (line 374)


You don't need to inherit from MesosTest for this. Just inherit from 
::testing::Test



src/tests/containerizer/isolator_tests.cpp (lines 377 - 379)


Can you move this, along with NetClsIsolatorTest to a separate file (e.g., 
cgroups_net_cls_isolator_tests.cpp)



src/tests/containerizer/isolator_tests.cpp (line 382)


IntervalSet here is not needed. Just do

```
NetClsHandleManager manager(
(Bound::closed(3),
 Bound::closed(3));
 

```



src/tests/containerizer/isolator_tests.cpp (line 405)


Ditto.



src/tests/containerizer/isolator_tests.cpp (line 417)


Ditto.


- Jie Yu


On Feb. 4, 2016, 9:03 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42588/
> ---
> 
> (Updated Feb. 4, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit-test for `NetClsHandleManager`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
>   src/tests/containerizer/isolator_tests.cpp 
> 8d101df957fd36adac388310eddba2db1f98c029 
> 
> Diff: https://reviews.apache.org/r/42588/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

2016-02-04 Thread Jie Yu

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


Fix it, then Ship it!





src/linux/cgroups.cpp (line 2471)


use ' instead of `



src/linux/cgroups.cpp (line 2488)


no need for this temp variable.


- Jie Yu


On Feb. 3, 2016, 10:41 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43096/
> ---
> 
> (Updated Feb. 3, 2016, 10:41 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function in cgroup for supporting net_cls subsystem.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
>   src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 
> 
> Diff: https://reviews.apache.org/r/43096/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Shuai Lin

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

(Updated Feb. 5, 2016, 3:08 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Erased libprocess related env vars for mesos-fetcher.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp dbf968df4b219fbbafd59d5560492b7c1ace83f3 

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


Testing
---

Created a simple framework that launchs a task with a hdfs:// uri

- Without this patch: fetcher failed with traceback like reported in jira
- With this patch: fetcher succeeds and the task is running


Thanks,

Shuai Lin



Re: Review Request 43215: Added code to remove a process's endpoint help strings upon termination.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:21 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Updated based on comments made by bmahler offline.


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


Repository: mesos


Description
---

Previously, the global help process would retain the help information
of processes even after they were termianted.

This commit adds code to remove these help strings.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 1672e081bd379cdf0467ea312f1798404f66eaa6 

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


Testing (updated)
---

Unit test in a subsequent commit.


Thanks,

Kevin Klues



Re: Review Request 43183: Support routing_tests run in aarch64 cpu.

2016-02-04 Thread Andy Pang

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

(Updated 二月 5, 2016, 1:13 a.m.)


Review request for mesos, BenjaminVW BenjaminVW, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Support routing_tests run in aarch64 cpu.


Diffs
-

  src/tests/containerizer/routing_tests.cpp 
4ebb01213ce20f954c2633df95e898cf66a2d9e9 

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


Testing
---

make test ok


Thanks,

Andy Pang



Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

2016-02-04 Thread Avinash sridharan

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

(Updated Feb. 5, 2016, 1:22 a.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan



Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Guangya Liu

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




src/docker/docker.cpp (lines 540 - 543)


why not 

Try validateVersion =
docker->validateVersion(Version(1, 6, 0));
  if (validateVersion.isError()) {
LOG(WARNING) << "Current docker version does not support 
labels."
   << "So TaskInfo labels are ignored to set as docker"
   << "labels.";
  } else {
  }


- Guangya Liu


On 二月 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Till Toenshoff

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


Ship it!




We need a centralized solution here. Same as MESOS-4598 however, this is fine 
for a fix, I believe.

- Till Toenshoff


On Feb. 5, 2016, 3:09 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 42957: Added remove() calls to process::Help.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:20 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Addressed comments made by bmahler offline.


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


Repository: mesos


Description
---

Previously, there was no way to remove an installed help
string for a given endpoint.

This commit adds the ability to remove these help strings.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
2e76a6a5b1069abce879374a88cea65036873f1d 
  3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 

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


Testing (updated)
---

Unit test done in a subsequent commit.


Thanks,

Kevin Klues



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

2016-02-04 Thread Neil Conway


> On Feb. 4, 2016, 1:32 a.m., Michael Park wrote:
> > src/tests/resources_tests.cpp, line 35
> > 
> >
> > Should come after `std` using declarations.

Happy to change this, but I notice that a few other places don't do this 
consistently (e.g., `slave_tests.cpp`, `sorter_tests.cpp`, 
`slave_recovery_tests.cpp`, `role_tests.cpp`). Do we have a canonical rule for 
how to order `using` declarations?


- Neil


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


On Feb. 3, 2016, 11:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42754/
> ---
> 
> (Updated Feb. 3, 2016, 11:32 p.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 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/type_utils.hpp 16f9cda83edd5064527b89e94392a7a54411 
>   include/mesos/v1/mesos.hpp b294f022345dc892c0581622c5b389efbde911a9 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/type_utils.cpp 42e3061818c49c6b62d4fee2479db078dfe0fc89 
>   src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 
>   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 42751: Tweaked some resource test cases.

2016-02-04 Thread Neil Conway

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

(Updated Feb. 5, 2016, 2:19 a.m.)


Review request for mesos and Michael Park.


Repository: mesos


Description
---

We should check that two reservations with the same role but different
principals are considered distinct.


Diffs (updated)
-

  src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 

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


Testing
---

make check


Thanks,

Neil Conway



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

2016-02-04 Thread Neil Conway

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

  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 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Shuai Lin


> On Feb. 4, 2016, 7:44 p.m., Joseph Wu wrote:
> > src/slave/containerizer/fetcher.cpp, line 756
> > 
> >
> > I just found a bug related to removing this env var.
> > 
> > See this: https://issues.apache.org/jira/browse/MESOS-4598

I see the discussion in https://issues.apache.org/jira/browse/MESOS-4598 , 
which makes sense to solve this problem in a the libprocess level. I updated 
the patch anyway if we decide to have a quick fix instead of waiting for the 
centralized solution.


- Shuai


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


On Feb. 5, 2016, 3:09 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 42908: Fixed a flaky test in quota tests.

2016-02-04 Thread Till Toenshoff

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



lgtm


src/tests/master_quota_tests.cpp (line 901)


I feel this comment as-is does not add much.

Either removing it or supplying a reasoning in that comment would be more 
helpful, no?


- Till Toenshoff


On Jan. 29, 2016, 10:34 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42908/
> ---
> 
> (Updated Jan. 29, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Michael Park and Qian Zhang.
> 
> 
> Bugs: MESOS-4542
> https://issues.apache.org/jira/browse/MESOS-4542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AvailableResourcesAfterRescinding` test became flaky after we
> stopped offering unreserved resources beyond quota in
> https://reviews.apache.org/r/42835. Hence the allocator offers
> rescinded resources to `framework1` if an allocation happens before
> the test finishes, which violates the expectation that `framework1`
> receives resources only once. Since we do not really care about
> allocations in this test but rather about rescinded resources, the
> fix is just to ignore subsequent offers to `framework1`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
> 
> Diff: https://reviews.apache.org/r/42908/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
> ./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
> --gtest_repeat=5000`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43177]

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. 4, 2016, 7:16 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 4, 2016, 7:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 43167: Added working dir flag to mesos containerizer launch.

2016-02-04 Thread Gilbert Song

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

(Updated Feb. 4, 2016, 5:24 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added working dir flag to mesos containerizer launch.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.hpp 
78d6b9d4f97207ec26ccfdb375db823486a30b37 
  src/slave/containerizer/mesos/launch.cpp 
23970a2ed239c5f4d057361964edffee8f5084b1 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



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

2016-02-04 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [43093]

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

Error:
2016-02-05 01:44:23 URL:https://reviews.apache.org/r/43093/diff/raw/ 
[2180/2180] -> "43093.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must end in a period.

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

- Mesos ReviewBot


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 42751: Tweaked some resource test cases.

2016-02-04 Thread Neil Conway


> On Feb. 3, 2016, 10:45 p.m., Michael Park wrote:
> > src/tests/resources_tests.cpp, line 957
> > 
> >
> > We used to have a `Resources::size()` function which essentially did 
> > this, but intentionally removed it so that people don't rely on number of 
> > `Resource` instances. Is there a reason why we want to check for this?
> > 
> > Here and below.
> 
> Neil Conway wrote:
> The # of `Resource` instances is part of the public API of `Resources` 
> (e.g., clients can iterate over every `Resource`). If it is part of the 
> public API, it seems like something it would be worth testing.
> 
> In this particular case, it doesn't matter that much, but in other test 
> cases (e.g., `AdditionDynamicallyReservedWithDistinctLabels`) it seems useful 
> to check.
> 
> Michael Park wrote:
> Ok, synced with Jie on this as well. Let's re-introduce 
> `Resources::size()` and use that instead.
> 
> I agree that the # of `Resource` instances is part of the public API 
> since as you say, one can iterate and count.
> However, this still does not mean that such iterator math works. For 
> example, we could internally store the `Resource` objects
> in a `std::set` (or any other data structure that does not guarantee 
> contiguous memory), rather than `std::vector`.
> 
> Providing a `Resources::size()` would more accurately capture this 
> intention anyway. Would you be ok taking that on?

Sounds good.


- Neil


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


On Feb. 3, 2016, 11:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42751/
> ---
> 
> (Updated Feb. 3, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should check that two reservations with the same role but different
> principals are considered distinct.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
> 
> Diff: https://reviews.apache.org/r/42751/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 42753: Allowed `createLabel` to take an optional "value".

2016-02-04 Thread Neil Conway

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

(Updated Feb. 5, 2016, 2:18 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

This better matches the underlying protobuf definition.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 7280c9fe36726df6b02ff468c7bd5ecedf5f5023 
  src/common/protobuf_utils.cpp e5a8efcaaa8ec0dece1dd70ae658e9d7d19ccb52 

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


Testing
---


Thanks,

Neil Conway



Re: Review Request 42755: Added documentation for labeled reserved resources.

2016-02-04 Thread Neil Conway

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

(Updated Feb. 5, 2016, 2:18 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Added documentation for labeled reserved resources.


Diffs (updated)
-

  docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 

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


Testing
---


Thanks,

Neil Conway



Review Request 43239: Added Resources::size().

2016-02-04 Thread Neil Conway

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Added Resources::size().


Diffs
-

  include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
  include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 

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


Testing
---


Thanks,

Neil Conway



Re: Review Request 42752: Fixed some typos in test case comments.

2016-02-04 Thread Neil Conway

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

(Updated Feb. 5, 2016, 2:19 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Fixed some typos in test case comments.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
26261b0b3a7ee8bb4d32ae7b2366f1a2de0a8c10 
  src/tests/monitor_tests.cpp d3a1801c074b7111aed2e7e25252c570850447f6 
  src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 

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


Testing
---


Thanks,

Neil Conway



Re: Review Request 43207: Don't remove IP from the logger's environment.

2016-02-04 Thread Till Toenshoff

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


Ship it!




As a fix, this seems fine. We should think about a centralized fix as proposed 
in the comments of the attached JIRA issue.

- Till Toenshoff


On Feb. 4, 2016, 11:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43207/
> ---
> 
> (Updated Feb. 4, 2016, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4598
> https://issues.apache.org/jira/browse/MESOS-4598
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unsetting the `LIBPROCESS_IP` was originally unnecessary, as this variable 
> does not result in port conflicts.  Unsetting this environment variable may 
> cause the subprocess to exit if DNS cannot resolve the agent's hostname.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 8d2f89503cd8ace1971933006a46ed4d3936ff8e 
> 
> Diff: https://reviews.apache.org/r/43207/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



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

2016-02-04 Thread Guangya Liu

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



The document should also be updated `docs/configuration.md` by adding some 
explanation for this parameter.


3rdparty/libprocess/src/process.cpp (lines 2212 - 2213)


Why not 

Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");



3rdparty/libprocess/src/process.cpp (line 2220)


I think we should add some logs here if the `cpus` override the 
`max_cpus.get()`, the end user may be confused for such case: Why my env does 
not take effect?



3rdparty/libprocess/src/process.cpp (line )


I think that warning message should also be enhanced that 

`"Ignoring invalid value " << value.get() << " for 
LIBPROCESS_MAX_WORKER_THREADS, using system defined value " << cpus;`


- Guangya Liu


On 二月 4, 2016, 7:55 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 4, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> 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 
> 
> 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 43221: Updated Rakefile to support subdirectories in /docs folder.

2016-02-04 Thread Ben Mahler

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




site/Rakefile (lines 30 - 37)


Mind documenting the reason we had to do this, for posterity? I certainly 
will not remember :)



site/Rakefile (line 34)


Do we need the `-p` here on mkdir?



site/Rakefile (line 42)


Could we put the slash in the regex instead of in the variable?



site/Rakefile (lines 44 - 46)


Not yours, but could we add a TODO here to move away from regexes and do a 
programmatic set of steps to perform the replacement? That will be way easier 
to reason about, this regex has become nasty.


- Ben Mahler


On Feb. 4, 2016, 8:38 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43221/
> ---
> 
> (Updated Feb. 4, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Rakefile for generating the mesos documentation
> website didn't support subdirectories containing markdown in the /docs
> folder. Specifically, no subdirectores were cpoied from the /docs
> folder into the site/documentation/latest folder for the website.
> Moreover, the regex used to patch relative links for the markdown
> files was incomplete. It didn't support subfolders contaning a
> top level index.md file (e.g. mydirectory/index.md). Also, due to a
> limitation in middleman, it didn't support .md files named (e.g.
> state.json.md). Middlman would generate an *html* file called
> state.json from this instead of generating the standard
> state.json/index.html like it does for other .md files.
> 
> This commit updates the Rakefile to support these features properly.
> 
> 
> Diffs
> -
> 
>   site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 
> 
> Diff: https://reviews.apache.org/r/43221/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

2016-02-04 Thread Jie Yu

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




src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 38)


No need for this.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 160)


kill this line



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 161)


Hum, this should be Option handle.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 167)


Can you use an empty IntervetSet to represent the None case? I.e., no need 
to use Option here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 174)


No need for the leading underscore.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 200)


What's defaultPrimary? Who is the user? Can you just do the following:

```
if (!primaries.empty()) {
  handleManager = NetClsHandleManager(primaries.get());
}
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 243 - 245)


No need to for this. Can you set the default behavior to be not allocating 
handles?

You may need to set a default value for 'primaries' in 
CgroupsNetClsIsolatorProcess constructor.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 265 - 267)


Make it less jagged. Also, no need to quote containerId because it's 
generated by Mesos (guaranteed no space in it, you only need to quote something 
that user provides).

```
return Failure(
"Failed to check cgroup for container " +
stringify(containerId));
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 284 - 285)


Ditto on remove quote for containerId.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 320 - 322)


Ditto.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 407 - 408)


This fits one line?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 408)


Can you make 'defaultPrimiary' a helper method instead of a field member. 
The field member will go away in the future once we introduce other allocation 
policies like one primary per framweork.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 412)


space after ':'

s/for the container//



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 441)


Ditto on removing quote.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 449)


I would rather use handle.isSome as the condition here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 503)


I would use handle.isSOme() as the condition here.


- Jie Yu


On Feb. 4, 2016, 1:42 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Feb. 4, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43216: Added json() function for jsonification of the global help process.

2016-02-04 Thread Ben Mahler

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




3rdparty/libprocess/src/help.cpp (lines 146 - 160)


How about the following?

```
{
  "processes":
  [
{ "id": "", "endpoints": [{"name": "", "text": ""}] },
  ]
}
```

Then the consumption code looks like this:

```
for process in processes:
  for endpoint in process.endpoints:
process.id, endpoint.name, endpoint.text
```

We might want a comment that shows the format to help the reader of this 
code?


- Ben Mahler


On Feb. 4, 2016, 8:40 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43216/
> ---
> 
> (Updated Feb. 4, 2016, 8:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to dump the help strings into a JSON object when
> hitting the /help endpoint with a specific request for JSON data.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 2e76a6a5b1069abce879374a88cea65036873f1d 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/43216/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42791: Added link to HTTP Endpoints doc in home.md.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42955, 42956, 42957, 43215, 43216, 43217, 43218, 43219, 
43220, 42790, 43221, 42791]

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. 4, 2016, 8:38 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42791/
> ---
> 
> (Updated Feb. 4, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added link to HTTP Endpoints doc in home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
> 
> Diff: https://reviews.apache.org/r/42791/diff/
> 
> 
> Testing
> ---
> 
> Documentation is live here:
> http://c99.millennium.berkeley.edu/documentation/latest/
> http://c99.millennium.berkeley.edu/documentation/latest/endpoints/
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

2016-02-04 Thread Avinash sridharan


> On Feb. 5, 2016, 12:04 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 382
> > 
> >
> > IntervalSet here is not needed. Just do
> > 
> > ```
> > NetClsHandleManager manager(
> > (Bound::closed(3),
> >  Bound::closed(3));
> >  
> > 
> > ```

As discussed need IntervalSet due to explicit constructor in 
IntervalSet


> On Feb. 5, 2016, 12:04 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 405
> > 
> >
> > Ditto.

Same reason as above.


> On Feb. 5, 2016, 12:04 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 417
> > 
> >
> > Ditto.

Same reason as above.


- Avinash


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


On Feb. 4, 2016, 9:03 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42588/
> ---
> 
> (Updated Feb. 4, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit-test for `NetClsHandleManager`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
>   src/tests/containerizer/isolator_tests.cpp 
> 8d101df957fd36adac388310eddba2db1f98c029 
> 
> Diff: https://reviews.apache.org/r/42588/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43217: Added ability to return the /help endpoint as a JSON object.

2016-02-04 Thread Ben Mahler

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




3rdparty/libprocess/src/help.cpp (lines 185 - 192)


Can you add a comment here for posterity, given that we discussed this 
quite a bit:

```
// NOTE: We avoided relying on the 'Accept' header to
// specify the format because if users are hitting the
// endpoint from a browser it is difficult to change
// the 'Accept' header.
```


- Ben Mahler


On Feb. 4, 2016, 8:39 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43217/
> ---
> 
> (Updated Feb. 4, 2016, 8:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the /help endpoint could only be returned as either pure
> markdown or html.
> 
> This commit introduces a query parameter: format=json to allow the
> help endpoint to return a JSON object containing the help information
> for all other endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/43217/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 43239: Added Resources::size().

2016-02-04 Thread Guangya Liu

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


Ship it!




I saw the test case was already covered in https://reviews.apache.org/r/42751

- Guangya Liu


On 二月 5, 2016, 2:18 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43239/
> ---
> 
> (Updated 二月 5, 2016, 2:18 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Resources::size().
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
> 
> Diff: https://reviews.apache.org/r/43239/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 43239: Added Resources::size().

2016-02-04 Thread Guangya Liu

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



Can you please add a test case for size()?

- Guangya Liu


On 二月 5, 2016, 2:18 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43239/
> ---
> 
> (Updated 二月 5, 2016, 2:18 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Resources::size().
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
> 
> Diff: https://reviews.apache.org/r/43239/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 42751: Tweaked some resource test cases.

2016-02-04 Thread Guangya Liu

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




src/tests/resources_tests.cpp (lines 326 - 327)


Just a question, do we have some guidelines for when to use `ASSERT_EQ` and 
when to use `EXPECT_EQ` ?


- Guangya Liu


On 二月 5, 2016, 2:19 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42751/
> ---
> 
> (Updated 二月 5, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should check that two reservations with the same role but different
> principals are considered distinct.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
> 
> Diff: https://reviews.apache.org/r/42751/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 43217: Added ability to return the /help endpoint as a JSON object.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:23 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Addressed comments by bmahler.


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


Repository: mesos


Description
---

Previously, the /help endpoint could only be returned as either pure
markdown or html.

This commit introduces a query parameter: format=json to allow the
help endpoint to return a JSON object containing the help information
for all other endpoints.


Diffs (updated)
-

  3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 

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


Testing (updated)
---

Unit test in a subsequent commit.


Thanks,

Kevin Klues



Re: Review Request 42957: Added remove() calls to process::Help.

2016-02-04 Thread Ben Mahler

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


Ship it!




Ship It!

- Ben Mahler


On Feb. 5, 2016, 3:20 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> ---
> 
> (Updated Feb. 5, 2016, 3:20 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, there was no way to remove an installed help
> string for a given endpoint.
> 
> This commit adds the ability to remove these help strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 2e76a6a5b1069abce879374a88cea65036873f1d 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/42957/diff/
> 
> 
> Testing
> ---
> 
> Unit test done in a subsequent commit.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42911: Removed extra blank line.

2016-02-04 Thread Guangya Liu

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


Ship it!




Ship It!

- Guangya Liu


On Jan. 28, 2016, 1:13 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42911/
> ---
> 
> (Updated Jan. 28, 2016, 1:13 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed extra blank line.
> 
> 
> Diffs
> -
> 
>   src/master/quota.cpp e7ce0f1fc1c83b2ec30185db005d2671a93baed6 
> 
> Diff: https://reviews.apache.org/r/42911/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

2016-02-04 Thread Klaus Ma


> On Feb. 4, 2016, 7:43 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, lines 643-649
> > 
> >
> > This looks like an internal information, the *source* of a revocable 
> > resource. While we definitely need this is the allocator, I'm not sure we 
> > should expose it in the public protobuf. I think frameworks are more 
> > interested in the *quality* of a resource. Can we keep the source private 
> > somehow?

Framework need to know revocable type as master did not allow to use both 
USAGE_SLACK and ALLOCATION_SLACK for one task; it's because the controller 
(QoSController & evicting modules) is different for now.


- Klaus


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


On Jan. 7, 2016, 11:43 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Jan. 7, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 43149: Add LIBPROCESS_THREAD_COUNT to override the thread pool size.

2016-02-04 Thread Klaus Ma

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




3rdparty/libprocess/src/process.cpp (line 2204)


Check negative and zero value.



3rdparty/libprocess/src/process.cpp (line 2205)


Add test for this patch.


- Klaus Ma


On Feb. 4, 2016, 8:40 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43149/
> ---
> 
> (Updated Feb. 4, 2016, 8:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4592
> https://issues.apache.org/jira/browse/MESOS-4592
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check the LIBPROCESS_THREAD_COUNT environment variable to determine
> the number of threads in the libprocess thread pool.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 65a247a5bc7cf0bd6f23f6fc5c349ecce60f5ec0 
> 
> Diff: https://reviews.apache.org/r/43149/diff/
> 
> 
> Testing
> ---
> 
> make check on OS X. Running in production for many months.
> 
> 
> Thanks,
> 
> James Peach
> 
>



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

2016-02-04 Thread Guangya Liu

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


Ship it!




Ship It!

- Guangya Liu


On Feb. 3, 2016, 12:39 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43118/
> ---
> 
> (Updated Feb. 3, 2016, 12:39 a.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/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
>   docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
> 
> Diff: https://reviews.apache.org/r/43118/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 43124: Clarified units (megabytes) for "disk" and "mem" resource types.

2016-02-04 Thread Guangya Liu

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




docs/attributes-resources.md (lines 94 - 96)


why adjust the order here?



docs/attributes-resources.md (line 101)


For which case does a slave do not have `cpus` and `mem`? I think that the 
slave should always have values for those two metrics?


- Guangya Liu


On Feb. 3, 2016, 12:25 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43124/
> ---
> 
> (Updated Feb. 3, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4377
> https://issues.apache.org/jira/browse/MESOS-4377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that I also removed an old TODO from `mesos.proto`: it has been partially
> implemented, and in general the need for more first-class resource values is
> well-known (e.g., many JIRAs on this topic).
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md 53a27f48c94baa9b25a072e0fc503bde0b240ba2 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
> 
> Diff: https://reviews.apache.org/r/43124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 43216: Added json() function for jsonification of the global help process.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:22 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Addressed bmahler's comments.


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


Repository: mesos


Description
---

This will be used to dump the help strings into a JSON object when
hitting the /help endpoint with a specific request for JSON data.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
2e76a6a5b1069abce879374a88cea65036873f1d 
  3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 

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


Testing (updated)
---

Unit test in a subsequent commit.


Thanks,

Kevin Klues



Re: Review Request 43215: Added code to remove a process's endpoint help strings upon termination.

2016-02-04 Thread Ben Mahler

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


Ship it!




Ship It!

- Ben Mahler


On Feb. 5, 2016, 3:21 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43215/
> ---
> 
> (Updated Feb. 5, 2016, 3:21 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the global help process would retain the help information
> of processes even after they were termianted.
> 
> This commit adds code to remove these help strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 1672e081bd379cdf0467ea312f1798404f66eaa6 
> 
> Diff: https://reviews.apache.org/r/43215/diff/
> 
> 
> Testing
> ---
> 
> Unit test in a subsequent commit.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 43221: Updated Rakefile to support subdirectories in /docs folder.

2016-02-04 Thread Ben Mahler

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


Ship it!





site/Rakefile (lines 31 - 34)


Some whitespace and typo touchups needed here.


- Ben Mahler


On Feb. 5, 2016, 3:27 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43221/
> ---
> 
> (Updated Feb. 5, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Rakefile for generating the mesos documentation
> website didn't support subdirectories containing markdown in the /docs
> folder. Specifically, no subdirectores were cpoied from the /docs
> folder into the site/documentation/latest folder for the website.
> Moreover, the regex used to patch relative links for the markdown
> files was incomplete. It didn't support subfolders contaning a
> top level index.md file (e.g. mydirectory/index.md). Also, due to a
> limitation in middleman, it didn't support .md files named (e.g.
> state.json.md). Middlman would generate an *html* file called
> state.json from this instead of generating the standard
> state.json/index.html like it does for other .md files.
> 
> This commit updates the Rakefile to support these features properly.
> 
> 
> Diffs
> -
> 
>   site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 
> 
> Diff: https://reviews.apache.org/r/43221/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42791: Added link to HTTP Endpoints doc in home.md.

2016-02-04 Thread Ben Mahler

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


Ship it!




Ship It!

- Ben Mahler


On Feb. 5, 2016, 3:28 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42791/
> ---
> 
> (Updated Feb. 5, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added link to HTTP Endpoints doc in home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
> 
> Diff: https://reviews.apache.org/r/42791/diff/
> 
> 
> Testing
> ---
> 
> Documentation is live here:
> http://c99.millennium.berkeley.edu/documentation/latest/
> http://c99.millennium.berkeley.edu/documentation/latest/endpoints/
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



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

2016-02-04 Thread Guangya Liu

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




3rdparty/libprocess/src/process.cpp (line )


Can you please also log the system defined number into log message?


- Guangya Liu


On 二月 5, 2016, 3:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated 二月 5, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> 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 
> 
> 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 43219: Added unit tests for hitting the /help endpoints of a process.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:24 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Addressed comments by bmahler.


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


Repository: mesos


Description
---

These unit tests make sure that help endpoints are up and
running for a process's installed routes. They also ensure that
deleting a process removes the help information for all of it's routes
(to make sure stale information isn't sticking around if a new process
comes along with the same id later).


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
66d185e3fd8a165d8a98d3d2752e756f8de3499b 

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


Testing (updated)
---

This commit is a unit test that tests JSONification of the /help endpoint as 
well as the removal of help strings after a process has been terminated.

GTEST_FILTER="HTTPTest.EndpointsHelp:HTTPTest.EndpointsHelpRemoval" make -j 
check


Thanks,

Kevin Klues



Re: Review Request 42791: Added link to HTTP Endpoints doc in home.md.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:28 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated based on comments.


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


Repository: mesos


Description
---

Added link to HTTP Endpoints doc in home.md.


Diffs (updated)
-

  docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 

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


Testing
---

Documentation is live here:
http://c99.millennium.berkeley.edu/documentation/latest/
http://c99.millennium.berkeley.edu/documentation/latest/endpoints/


Thanks,

Kevin Klues



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Shuai Lin


> On Feb. 5, 2016, 3:20 a.m., Till Toenshoff wrote:
> > We need a centralized solution here. Same as MESOS-4598 however, this is 
> > fine for a fix, I believe.
> 
> Till Toenshoff wrote:
> ow. please fix this - I totally missed the fact that you are removing the 
> IP and not the PORT.

Fixed, sorry for the mistake.


- Shuai


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


On Feb. 5, 2016, 3:36 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:36 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Till Toenshoff

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




src/slave/containerizer/fetcher.cpp (lines 754 - 757)


Will add a blank line inbetween while committing to give this code some air 
:).


- Till Toenshoff


On Feb. 5, 2016, 3:36 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:36 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Shuai Lin

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

(Updated Feb. 5, 2016, 3:36 a.m.)


Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Erased libprocess related env vars for mesos-fetcher.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp dbf968df4b219fbbafd59d5560492b7c1ace83f3 

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


Testing
---

Created a simple framework that launchs a task with a hdfs:// uri

- Without this patch: fetcher failed with traceback like reported in jira
- With this patch: fetcher succeeds and the task is running


Thanks,

Shuai Lin



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Till Toenshoff


> On Feb. 5, 2016, 3:38 a.m., Till Toenshoff wrote:
> > src/slave/containerizer/fetcher.cpp, lines 754-757
> > 
> >
> > Will add a blank line inbetween while committing to give this code some 
> > air :).
> 
> Shuai Lin wrote:
> Right, please do that.

Thanks for this Shuai!


- Till


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


On Feb. 5, 2016, 3:36 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:36 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 42790: Added documentation for all http endpoints.

2016-02-04 Thread Ben Mahler

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


Ship it!




Ship It!

- Ben Mahler


On Feb. 5, 2016, 3:26 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42790/
> ---
> 
> (Updated Feb. 5, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These .md files were auto generated using
> support/generate-endpoints-help.py.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md PRE-CREATION 
>   docs/endpoints/files/browse.md PRE-CREATION 
>   docs/endpoints/files/debug.json.md PRE-CREATION 
>   docs/endpoints/files/debug.md PRE-CREATION 
>   docs/endpoints/files/download.json.md PRE-CREATION 
>   docs/endpoints/files/download.md PRE-CREATION 
>   docs/endpoints/files/read.json.md PRE-CREATION 
>   docs/endpoints/files/read.md PRE-CREATION 
>   docs/endpoints/index.md PRE-CREATION 
>   docs/endpoints/logging/toggle.md PRE-CREATION 
>   docs/endpoints/master/api/v1/scheduler.md PRE-CREATION 
>   docs/endpoints/master/create-volumes.md PRE-CREATION 
>   docs/endpoints/master/destroy-volumes.md PRE-CREATION 
>   docs/endpoints/master/flags.md PRE-CREATION 
>   docs/endpoints/master/frameworks.md PRE-CREATION 
>   docs/endpoints/master/health.md PRE-CREATION 
>   docs/endpoints/master/machine/down.md PRE-CREATION 
>   docs/endpoints/master/machine/up.md PRE-CREATION 
>   docs/endpoints/master/maintenance/schedule.md PRE-CREATION 
>   docs/endpoints/master/maintenance/status.md PRE-CREATION 
>   docs/endpoints/master/observe.md PRE-CREATION 
>   docs/endpoints/master/quota.md PRE-CREATION 
>   docs/endpoints/master/redirect.md PRE-CREATION 
>   docs/endpoints/master/reserve.md PRE-CREATION 
>   docs/endpoints/master/roles.json.md PRE-CREATION 
>   docs/endpoints/master/roles.md PRE-CREATION 
>   docs/endpoints/master/slaves.md PRE-CREATION 
>   docs/endpoints/master/state-summary.md PRE-CREATION 
>   docs/endpoints/master/state.json.md PRE-CREATION 
>   docs/endpoints/master/state.md PRE-CREATION 
>   docs/endpoints/master/tasks.json.md PRE-CREATION 
>   docs/endpoints/master/tasks.md PRE-CREATION 
>   docs/endpoints/master/teardown.md PRE-CREATION 
>   docs/endpoints/master/unreserve.md PRE-CREATION 
>   docs/endpoints/metrics/snapshot.md PRE-CREATION 
>   docs/endpoints/monitor/statistics.json.md PRE-CREATION 
>   docs/endpoints/monitor/statistics.md PRE-CREATION 
>   docs/endpoints/profiler/start.md PRE-CREATION 
>   docs/endpoints/profiler/stop.md PRE-CREATION 
>   docs/endpoints/registrar/registry.md PRE-CREATION 
>   docs/endpoints/slave/api/v1/executor.md PRE-CREATION 
>   docs/endpoints/slave/flags.md PRE-CREATION 
>   docs/endpoints/slave/health.md PRE-CREATION 
>   docs/endpoints/slave/state.json.md PRE-CREATION 
>   docs/endpoints/slave/state.md PRE-CREATION 
>   docs/endpoints/system/stats.json.md PRE-CREATION 
>   docs/endpoints/version.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42790/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42908: Fixed a flaky test in quota tests.

2016-02-04 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42908]

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 Jan. 29, 2016, 10:34 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42908/
> ---
> 
> (Updated Jan. 29, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Michael Park and Qian Zhang.
> 
> 
> Bugs: MESOS-4542
> https://issues.apache.org/jira/browse/MESOS-4542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AvailableResourcesAfterRescinding` test became flaky after we
> stopped offering unreserved resources beyond quota in
> https://reviews.apache.org/r/42835. Hence the allocator offers
> rescinded resources to `framework1` if an allocation happens before
> the test finishes, which violates the expectation that `framework1`
> receives resources only once. Since we do not really care about
> allocations in this test but rather about rescinded resources, the
> fix is just to ignore subsequent offers to `framework1`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
> 
> Diff: https://reviews.apache.org/r/42908/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
> ./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
> --gtest_repeat=5000`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 43223: Pin intra-package python dependencies to the same internal version

2016-02-04 Thread Mesos ReviewBot

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On Feb. 4, 2016, 9:39 p.m., Benjamin Staffin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43223/
> ---
> 
> (Updated Feb. 4, 2016, 9:39 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoids weird situations that arise from having differing versions of
> mesos.native and/or mesos.interface on the same system, especially
> while building system packages (e.g. debian builds)
> 
> 
> Diffs
> -
> 
>   src/python/setup.py.in 737066952fe72382bcf80ca6d3e8457ea07a65bf 
> 
> Diff: https://reviews.apache.org/r/43223/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Staffin
> 
>



Re: Review Request 43220: Added script to generate docs from endpoint help strings.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:30 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated to match new JSON schema.


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


Repository: mesos


Description
---

Invoke this script from anywhere within the mesos source tree
(including in the build directory) to autogenerate documentation for
all endpoint strings and install them into the docs/endpoint folder.
In a subsequent commit we commit all of these autogenerated files back
to the source repo and add a link in home.md to find them.


Diffs (updated)
-

  support/generate-endpoint-help.py PRE-CREATION 

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


Testing
---


Thanks,

Kevin Klues



Re: Review Request 43177: Erased libprocess related env vars for mesos-fetcher.

2016-02-04 Thread Till Toenshoff


> On Feb. 5, 2016, 3:20 a.m., Till Toenshoff wrote:
> > We need a centralized solution here. Same as MESOS-4598 however, this is 
> > fine for a fix, I believe.

ow. please fix this - I totally missed the fact that you are removing the IP 
and not the PORT.


- Till


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


On Feb. 5, 2016, 3:09 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43177/
> ---
> 
> (Updated Feb. 5, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4585
> https://issues.apache.org/jira/browse/MESOS-4585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Erased libprocess related env vars for mesos-fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> dbf968df4b219fbbafd59d5560492b7c1ace83f3 
> 
> Diff: https://reviews.apache.org/r/43177/diff/
> 
> 
> Testing
> ---
> 
> Created a simple framework that launchs a task with a hdfs:// uri
> 
> - Without this patch: fetcher failed with traceback like reported in jira
> - With this patch: fetcher succeeds and the task is running
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 43217: Added ability to return the /help endpoint as a JSON object.

2016-02-04 Thread Ben Mahler

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


Fix it, then Ship it!





3rdparty/libprocess/src/help.cpp (line 204)


Period at the end here?



3rdparty/libprocess/src/help.cpp (lines 206 - 207)


Looks like we can directly return:

```
return http::OK(jsonify(*this));
```


- Ben Mahler


On Feb. 5, 2016, 3:23 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43217/
> ---
> 
> (Updated Feb. 5, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the /help endpoint could only be returned as either pure
> markdown or html.
> 
> This commit introduces a query parameter: format=json to allow the
> help endpoint to return a JSON object containing the help information
> for all other endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/43217/diff/
> 
> 
> Testing
> ---
> 
> Unit test in a subsequent commit.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



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

2016-02-04 Thread Maged Michael


> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2212-2213
> > 
> >
> > Why not 
> > 
> > Option value = os::getenv("LIBPROCESS_MAX_WORKER_THREADS");
> 
> Guangya Liu wrote:
> I see what you mean: it will be used in the log message.

The string is used twice, once as argument of getenv and another time in the 
warning. My concern about repeating the string is that there could be a 
mismatch (e.g., we decide to change the environment variabvle but forget to 
update the warning) that is not caught by the compiler.


> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2220
> > 
> >
> > I think we should add some logs here if the `cpus` override the 
> > `max_cpus.get()`, the end user may be confused for such case: Why my env 
> > does not take effect?

How about?
  if (max_cpus.get() < cpus) {
cpus = max_cpus.get();
VLOG(1) << "Overriding the system defined number of worker threads, "
<< "using the value " << cpus;
  } else {
VLOG(1) << "The system defined number of worker threads " << cpus
<< " is unchanged, as it is not greater than " << env_var
<< "=" << max_cpus.get();
  }


> On Feb. 5, 2016, 2:51 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 
> > 
> >
> > I think that warning message should also be enhanced that 
> > 
> > `"Ignoring invalid value " << value.get() << " for 
> > LIBPROCESS_MAX_WORKER_THREADS, using system defined value " << cpus;`

Done.


- Maged


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


On Feb. 5, 2016, 3:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> 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 
> 
> 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 42790: Added documentation for all http endpoints.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:26 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated based on new python script for generating the .md files.


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


Repository: mesos


Description
---

These .md files were auto generated using
support/generate-endpoints-help.py.


Diffs (updated)
-

  docs/endpoints/files/browse.json.md PRE-CREATION 
  docs/endpoints/files/browse.md PRE-CREATION 
  docs/endpoints/files/debug.json.md PRE-CREATION 
  docs/endpoints/files/debug.md PRE-CREATION 
  docs/endpoints/files/download.json.md PRE-CREATION 
  docs/endpoints/files/download.md PRE-CREATION 
  docs/endpoints/files/read.json.md PRE-CREATION 
  docs/endpoints/files/read.md PRE-CREATION 
  docs/endpoints/index.md PRE-CREATION 
  docs/endpoints/logging/toggle.md PRE-CREATION 
  docs/endpoints/master/api/v1/scheduler.md PRE-CREATION 
  docs/endpoints/master/create-volumes.md PRE-CREATION 
  docs/endpoints/master/destroy-volumes.md PRE-CREATION 
  docs/endpoints/master/flags.md PRE-CREATION 
  docs/endpoints/master/frameworks.md PRE-CREATION 
  docs/endpoints/master/health.md PRE-CREATION 
  docs/endpoints/master/machine/down.md PRE-CREATION 
  docs/endpoints/master/machine/up.md PRE-CREATION 
  docs/endpoints/master/maintenance/schedule.md PRE-CREATION 
  docs/endpoints/master/maintenance/status.md PRE-CREATION 
  docs/endpoints/master/observe.md PRE-CREATION 
  docs/endpoints/master/quota.md PRE-CREATION 
  docs/endpoints/master/redirect.md PRE-CREATION 
  docs/endpoints/master/reserve.md PRE-CREATION 
  docs/endpoints/master/roles.json.md PRE-CREATION 
  docs/endpoints/master/roles.md PRE-CREATION 
  docs/endpoints/master/slaves.md PRE-CREATION 
  docs/endpoints/master/state-summary.md PRE-CREATION 
  docs/endpoints/master/state.json.md PRE-CREATION 
  docs/endpoints/master/state.md PRE-CREATION 
  docs/endpoints/master/tasks.json.md PRE-CREATION 
  docs/endpoints/master/tasks.md PRE-CREATION 
  docs/endpoints/master/teardown.md PRE-CREATION 
  docs/endpoints/master/unreserve.md PRE-CREATION 
  docs/endpoints/metrics/snapshot.md PRE-CREATION 
  docs/endpoints/monitor/statistics.json.md PRE-CREATION 
  docs/endpoints/monitor/statistics.md PRE-CREATION 
  docs/endpoints/profiler/start.md PRE-CREATION 
  docs/endpoints/profiler/stop.md PRE-CREATION 
  docs/endpoints/registrar/registry.md PRE-CREATION 
  docs/endpoints/slave/api/v1/executor.md PRE-CREATION 
  docs/endpoints/slave/flags.md PRE-CREATION 
  docs/endpoints/slave/health.md PRE-CREATION 
  docs/endpoints/slave/state.json.md PRE-CREATION 
  docs/endpoints/slave/state.md PRE-CREATION 
  docs/endpoints/system/stats.json.md PRE-CREATION 
  docs/endpoints/version.md PRE-CREATION 

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


Testing
---


Thanks,

Kevin Klues



Re: Review Request 43216: Added json() function for jsonification of the global help process.

2016-02-04 Thread Ben Mahler

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


Ship it!





3rdparty/libprocess/src/help.cpp (lines 135 - 149)


How about:

```
// Write the help strings contained in the help process to JSON.
// The schema looks as follows:
// {
//   "processes":
//   [
// {
//   "id": id,
//   "endpoints": [ { "name" : name, "text" : text }, ... ]
// },
// ...
//   ]
// }
```


- Ben Mahler


On Feb. 5, 2016, 3:22 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43216/
> ---
> 
> (Updated Feb. 5, 2016, 3:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to dump the help strings into a JSON object when
> hitting the /help endpoint with a specific request for JSON data.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 2e76a6a5b1069abce879374a88cea65036873f1d 
>   3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae 
> 
> Diff: https://reviews.apache.org/r/43216/diff/
> 
> 
> Testing
> ---
> 
> Unit test in a subsequent commit.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 43221: Updated Rakefile to support subdirectories in /docs folder.

2016-02-04 Thread Kevin Klues

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

(Updated Feb. 5, 2016, 3:27 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated based on bmahler's comments.


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


Repository: mesos


Description
---

Previously, the Rakefile for generating the mesos documentation
website didn't support subdirectories containing markdown in the /docs
folder. Specifically, no subdirectores were cpoied from the /docs
folder into the site/documentation/latest folder for the website.
Moreover, the regex used to patch relative links for the markdown
files was incomplete. It didn't support subfolders contaning a
top level index.md file (e.g. mydirectory/index.md). Also, due to a
limitation in middleman, it didn't support .md files named (e.g.
state.json.md). Middlman would generate an *html* file called
state.json from this instead of generating the standard
state.json/index.html like it does for other .md files.

This commit updates the Rakefile to support these features properly.


Diffs (updated)
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kevin Klues



Review Request 43240: Removed implicit, value changing conversion.

2016-02-04 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Whether `char` is signed or not is up to the implementation, so be more explicit
here. Also, since we want a `char` with all bits set, use a value representation
that more clearly communicates that.


Diffs
-

  src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 

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


Testing
---

When compiling with trunk clang, compilation fails with

../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
[-Wconstant-conversion]

After inserting the explicit cast the code compiles fine.


Thanks,

Benjamin Bannier



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

2016-02-04 Thread Maged Michael

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

(Updated Feb. 5, 2016, 3:39 a.m.)


Review request for mesos and Klaus Ma.


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

  3rdparty/libprocess/src/process.cpp d8a74d7 

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



  1   2   >