Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

[Alexander Rojas]

> On Dec. 13, 2015, 11:55 a.m., Alexander Rojas wrote:
> > Ship It!

to clarify, it is a fiix-it the. ship it.


- Alexander


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


On Dec. 13, 2015, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 13, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On Dec. 13, 2015, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 13, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41320: Clarified Subprocess PIPE usage in Subprocess tests.

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On Dec. 13, 2015, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41320/
> ---
> 
> (Updated Dec. 13, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we use PIPE() in many places where it is not necessary. This
> switches these instances to use the default (re-inherit the parent FD).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 869b920b4f25a74a69813ac4777e843d38300cbd 
> 
> Diff: https://reviews.apache.org/r/41320/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

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

(Updated Dec. 13, 2015, 11:35 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


Repository: mesos


Description
---

WIP: Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

> On Dec. 14, 2015, 2:14 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 431
> > 
> >
> > Honestly, I'd like to separate optimistic resources into other counter 
> > in allocator; it's a litte different with oversubscription: here, one 
> > reserved resources are counting twice in total.

The allocation slack was decreased in 1304-1329


- Guangya


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


On Dec. 13, 2015, 11:35 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Dec. 13, 2015, 11:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41254: Enable master get ALLOCATION_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 13, 2015, 3:08 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enable master get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "master/cpus_alloc_slack_revocable_total": 3,
  "master/cpus_alloc_slack_revocable_used": 0,
  "master/disk_alloc_slack_revocable_total": 0,
  "master/disk_alloc_slack_revocable_used": 0,
  "master/mem_alloc_slack_revocable_total": 1000,
  "master/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41250: Enabled slave get ALLOCATION_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 13, 2015, 3:08 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "slave/cpus_alloc_slack_revocable_percent": 0,
  "slave/cpus_alloc_slack_revocable_total": 3,
  "slave/cpus_alloc_slack_revocable_used": 0,
  "slave/disk_alloc_slack_revocable_percent": 0,
  "slave/disk_alloc_slack_revocable_total": 0,
  "slave/disk_alloc_slack_revocable_used": 0,
  "slave/mem_alloc_slack_revocable_percent": 0,
  "slave/mem_alloc_slack_revocable_total": 1000,
  "slave/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu

Review Request 41326: Fixed error in authentication cleanup function.

[Alexander Rojas]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Repository: mesos


Description
---

When a process is destroyed, it deregisters all its endpoints from 
authentication. However the paths were malformed and the endpoints were left 
inside the authentication router.


Diffs
-

  3rdparty/libprocess/src/process.cpp af3cefb59c506863959bf80aedb27ea5d22d7b74 

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


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 41327: Fixed a race in Authentication tests where it was possible to connect to a non yet routed endpoint.

[Alexander Rojas]

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

Review request for mesos, Ben Mahler and Joris Van Remoortere.


Repository: mesos


Description
---

The call to `ProcessBase::initialize()` occurs in an asyncrhonous way.
This allows processes to be used before the complete initialization
happens, which is particularly bad for `route` and `install` because
it allows HTTP clients to do request to possible non yet routed or
installed endpoints. This was apparent in the AuthenticationTests.

This patch fixes the race for such tests while a deeper discussion
occurs on the semantics of `ProcessBase::initialize()`


Diffs
-

  3rdparty/libprocess/src/process.cpp af3cefb59c506863959bf80aedb27ea5d22d7b74 
  3rdparty/libprocess/src/tests/http_tests.cpp 
9fe27039416290296e43c6327c85721342d02cb9 

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


Testing
---

On OSX, and Ubuntu 14.04:

```bash
$ make -j8 check
$ ./3rdparty/libprocess/libprocess-tests 
--gtest_filter="HttpAuthenticationTest.*" --gtest_repeat=10 --verbose 
--gtest_break_on_failure
```


Thanks,

Alexander Rojas

Re: Review Request 40966: Corrected termination of Docker containers.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40966]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 11, 2015, 11:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40966/
> ---
> 
> (Updated Dec. 11, 2015, 11:15 a.m.)
> 
> 
> Review request for mesos, Greg Mann, haosdent huang, Jojy Varghese, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests cases have to wait until a container has been terminated by the
> DockerContainerizer. Otherwise there could be artifacts (e.g. locked cgroups)
> that can affect later test cases (see MESOS-4025, where cgroups couldn't be
> removed).
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/40966/diff/
> 
> 
> Testing
> ---
> 
> make check
> sudo ./bin/mesos-tests.sh --gtest_repeat=50 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_*:SlaveRecoveryTest*GCExecutor"
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

[Klaus Ma]

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

(Updated Dec. 13, 2015, 11:28 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase the code; @mcypark, please help to review when you have time.


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am 9762f85 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 5211f54 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 41327: Fixed a race in Authentication tests where it was possible to connect to a non yet routed endpoint.

[Alexander Rojas]

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

(Updated Dec. 13, 2015, 5:34 p.m.)


Review request for mesos, Ben Mahler and Joris Van Remoortere.


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


Repository: mesos


Description
---

The call to `ProcessBase::initialize()` occurs in an asyncrhonous way.
This allows processes to be used before the complete initialization
happens, which is particularly bad for `route` and `install` because
it allows HTTP clients to do request to possible non yet routed or
installed endpoints. This was apparent in the AuthenticationTests.

This patch fixes the race for such tests while a deeper discussion
occurs on the semantics of `ProcessBase::initialize()`


Diffs
-

  3rdparty/libprocess/src/process.cpp af3cefb59c506863959bf80aedb27ea5d22d7b74 
  3rdparty/libprocess/src/tests/http_tests.cpp 
9fe27039416290296e43c6327c85721342d02cb9 

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


Testing
---

On OSX, and Ubuntu 14.04:

```bash
$ make -j8 check
$ ./3rdparty/libprocess/libprocess-tests 
--gtest_filter="HttpAuthenticationTest.*" --gtest_repeat=10 --verbose 
--gtest_break_on_failure
```


Thanks,

Alexander Rojas

Re: Review Request 41212: Adjust timeout value in HealthCheckTest.CheckCommandTimeout.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41212]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 8:11 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41212/
> ---
> 
> (Updated Dec. 12, 2015, 8:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4024
> https://issues.apache.org/jira/browse/MESOS-4024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjust timeout value in HealthCheckTest.CheckCommandTimeout.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
> 
> Diff: https://reviews.apache.org/r/41212/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41302, 41305, 41306, 41308]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 9:58 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Dec. 12, 2015, 9:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: Unit Test for moving getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
>   src/tests/master_tests.cpp 865fa4a 
>   src/tests/master_validation_tests.cpp fbf8fad 
>   src/tests/monitor_tests.cpp a848d14 
>   src/tests/reservation_endpoints_tests.cpp d5d2aa7 
>   src/tests/slave_recovery_tests.cpp c0e4ff7 
>   src/tests/slave_tests.cpp 4975bea 
> 
> Diff: https://reviews.apache.org/r/41308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 10:12 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Dec. 12, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 9bd86e1 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

[Ben Mahler]

> On Dec. 13, 2015, 12:34 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 1778-1794
> > 
> >
> > This test also suffers from the same problem.
> > Mind fixing this one as well?

Thanks for noticing that!


> On Dec. 13, 2015, 12:34 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1416
> > 
> >
> > We capture the response, but don't use it later.
> > Should we either:
> > (1) not capture it
> > (2) `ASSERT_AWAIT_READY(response)` before `AWAIT_READY(body)`
> > (3) `ASSERT_AWAIT_READY(connection.send(request))`
> > What are your thoughts?

Hm.. what I'll do to make this even more explicit is I'll add an 
`EXPECT_TRUE(response.isPending())`. We won't receive a response (that's the 
root of the issue here) because libprocess does not send one for this old-style 
pseudo-HTTP protocol.


- Ben


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


On Dec. 12, 2015, 11:11 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41275: Introduced a field for specifying grace period in Shutdown Event for Executor V1 API.

[Qian Zhang]

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



include/mesos/executor/executor.proto (line 93)


Just a quicky question, how will agent fill this field when it asks the 
executor to shutdown? Is this something that executor itself can 
decide/configure? Or it is a per-slave configuration via slave command line 
option?


- Qian Zhang


On Dec. 12, 2015, 7:51 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41275/
> ---
> 
> (Updated Dec. 12, 2015, 7:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a field `grace_period_seconds` that specifies the time 
> an agent would wait for the executor to terminate before forcefully 
> destroying the container.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 8f97ad8952b3156e9c299a5a297693c8d2ffccd4 
>   include/mesos/v1/executor/executor.proto 
> 7eaa40bf38845befeba477d074396e7660a7b231 
> 
> Diff: https://reviews.apache.org/r/41275/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 36816: Support HTTP checks in Mesos health check program

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36816]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 4:08 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Dec. 12, 2015, 4:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Michael Park, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support HTTP checks in Mesos health check program
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
>   src/health-check/main.cpp 0beaed575ec865d81e6e3d83d8a0c894613acba4 
>   src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskThroughHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo

[Guangya Liu]

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



include/mesos/mesos.proto (lines 509 - 512)


It is better to add some comments to explain the meaning for each of the 
field as others.


- Guangya Liu


On 十二月 12, 2015, 9:52 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41302/
> ---
> 
> (Updated 十二月 12, 2015, 9:52 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: add slave's configuration into SlaveInfo
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 
> 
> Diff: https://reviews.apache.org/r/41302/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 41334: Added helper functions to filter allocation slack resources.

[Guangya Liu]

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Added helper functions to filter allocation slack resources.


Diffs
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
  src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
  src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 

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


Testing
---


Thanks,

Guangya Liu

Review Request 41333: Added helper functions to filter usage slack resources.

[Guangya Liu]

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Added helper functions to filter usage slack resources.


Diffs
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
  src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
  src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Jian Qiu]

> On Dec. 14, 2015, 2:14 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 431
> > 
> >
> > Honestly, I'd like to separate optimistic resources into other counter 
> > in allocator; it's a litte different with oversubscription: here, one 
> > reserved resources are counting twice in total.
> 
> Guangya Liu wrote:
> The allocation slack was decreased in 1304-1329

Just wondering the reason to set resources in allocaction slack when add/update 
slave. It seems to me that you just need to calculate available resources in 
allocate() and add allocation slack to the available there?


- Jian


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


On Dec. 13, 2015, 11:35 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Dec. 13, 2015, 11:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Klaus Ma]

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



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


Honestly, I'd like to separate optimistic resources into other counter in 
allocator; it's a litte different with oversubscription: here, one reserved 
resources are counting twice in total.


- Klaus Ma


On Dec. 13, 2015, 7:35 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Dec. 13, 2015, 7:35 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41275: Introduced a field for specifying grace period in Shutdown Event for Executor V1 API.

[Anand Mazumdar]

> On Dec. 13, 2015, 11:47 p.m., Qian Zhang wrote:
> > include/mesos/executor/executor.proto, line 96
> > 
> >
> > Just a quicky question, how will agent fill this field when it asks the 
> > executor to shutdown? Is this something that executor itself can 
> > decide/configure? Or it is a per-slave configuration via slave command line 
> > option?

The agent can fill this field when it asks the executor to shutdown i.e. ( via 
the `Event::Shutdown` message ). The value of this field can be set via a slave 
command line option.

Currently, the existing executor library implementation just uses a hard-coded 
value https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L83


- Anand


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


On Dec. 11, 2015, 11:51 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41275/
> ---
> 
> (Updated Dec. 11, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a field `grace_period_seconds` that specifies the time 
> an agent would wait for the executor to terminate before forcefully 
> destroying the container.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 8f97ad8952b3156e9c299a5a297693c8d2ffccd4 
>   include/mesos/v1/executor/executor.proto 
> 7eaa40bf38845befeba477d074396e7660a7b231 
> 
> Diff: https://reviews.apache.org/r/41275/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41299: Allow DockerContainerizer log to console.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41299]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 13, 2015, 5:35 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41299/
> ---
> 
> (Updated Dec. 13, 2015, 5:35 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4141
> https://issues.apache.org/jira/browse/MESOS-4141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow DockerContainerizer log to console.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> dcdf98fea4ca6f96658886db5d09c99f3bff501d 
>   src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 3f199e622dd337cdbf32d4368f4ead424c39823c 
>   src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
>   src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
>   src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 
> 
> Diff: https://reviews.apache.org/r/41299/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40529: Added helper function to get stateless resources.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 5:56 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Added helper function to get stateless resources.


Diffs (updated)
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
  src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
  src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 

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


Testing (updated)
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 40339: Added a flag to master to enable oversubscription for reservations.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 6:05 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Added a flag to master to enable optimistic offers.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
e239b4746494fcc2b362a83afb634a2ce5e25f9b 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
  src/tests/persistent_volume_endpoints_tests.cpp 
c0feedee393b8475fd27b0af9344d306a392893e 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/reservation_tests.cpp f429472e6b93a5d6d8fe6a5f7d5b94fc331f7295 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 

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


Testing
---

Ubuntu 14.04
make
make check


Thanks,

Guangya Liu

Re: Review Request 41246: Enabled slave get USAGE_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 6:48 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get USAGE_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
usage_slack
  "slave/cpus_usage_slack_revocable_percent": 0,
  "slave/cpus_usage_slack_revocable_total": 0,
  "slave/cpus_usage_slack_revocable_used": 0,
  "slave/disk_usage_slack_revocable_percent": 0,
  "slave/disk_usage_slack_revocable_total": 0,
  "slave/disk_usage_slack_revocable_used": 0,
  "slave/mem_usage_slack_revocable_percent": 0,
  "slave/mem_usage_slack_revocable_total": 0,
  "slave/mem_usage_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

[Qian Zhang]

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



src/master/validation.cpp 


So now we allow a task has both ExecutorInfo and CommandInfo, right? If so, 
then you may need to update the following comments for TaskInfo as well.
/**
 * ... Either ExecutorInfo or CommandInfo should be set.
 * ...
 */
message TaskInfo {
...
}

However, I still prefer the old way: either ExecutorInfo or CommandInfo 
should be set rather than both. So I think in `Master::_accept()` you can 
separate `validateExecutorInfo()` from `validation::task::validate()`, and call 
it separately before adding command executor into command task.


- Qian Zhang


On Dec. 12, 2015, 5:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41306/
> ---
> 
> (Updated Dec. 12, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: use command line executor to launch tasks
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41254: Enabled master get ALLOCATION_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 7:15 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


Summary (updated)
-

Enabled master get ALLOCATION_SLACK metrics.


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


Repository: mesos


Description
---

Enable master get ALLOCATION_SLACK metrics.


Diffs
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "master/cpus_alloc_slack_revocable_total": 3,
  "master/cpus_alloc_slack_revocable_used": 0,
  "master/disk_alloc_slack_revocable_total": 0,
  "master/disk_alloc_slack_revocable_used": 0,
  "master/mem_alloc_slack_revocable_total": 1000,
  "master/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

> On Dec. 14, 2015, 2:14 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 431
> > 
> >
> > Honestly, I'd like to separate optimistic resources into other counter 
> > in allocator; it's a litte different with oversubscription: here, one 
> > reserved resources are counting twice in total.
> 
> Guangya Liu wrote:
> The allocation slack was decreased in 1304-1329
> 
> Jian Qiu wrote:
> Just wondering the reason to set resources in allocaction slack when 
> add/update slave. It seems to me that you just need to calculate available 
> resources in allocate() and add allocation slack to the available there?

If added slave with optimistic offer phase 1 enabled, then I want to make sure 
that the log message in allocator is able to reflect that there are allocation 
slack resources. SO just initializt allocation slack resources in add/update 
slave and use it in allocate().


- Guangya


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


On Dec. 13, 2015, 11:35 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Dec. 13, 2015, 11:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master

[Qian Zhang]

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



src/master/master.cpp (line 2830)


So if task's resources has no cpu, then we will not add cpu to executor's 
resources, right? But I think we should always add DEFAULT_EXECUTOR_CPUS to 
executor's resources in this case.



src/master/master.cpp (line 2838)


So if task's resources has no mem, then we will not add mem to executor's 
resources, right? But I think we should always add DEFAULT_EXECUTOR_MEM to 
executor's resources in this case.



src/master/master.cpp (line 3556)


Just wondering why we want to do `task.resources() - executor.resources())` 
here? I think in `validateResourceUsage()`, we will validate whether the task's 
resources + executor's resources can be contained by the offered resources, if 
here we substract executor's resources from task's resources, then we will only 
validate whether task's resources can be contained by the offered resources in 
`validateResourceUsage()` which seems not correct to me.



src/slave/slave.cpp (line 1370)


Before trying to get task's executor via `task.executor()`, suggest to add 
a CHECK to ensure the task always has an executor.


- Qian Zhang


On Dec. 12, 2015, 5:54 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41305/
> ---
> 
> (Updated Dec. 12, 2015, 5:54 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: move getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 
>   src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
>   src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41305/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40632: WIP: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 6:31 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


Repository: mesos


Description
---

WIP: Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 41333: Added helper functions to filter usage slack resources.

[Jian Qiu]

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



src/v1/resources.cpp (line 696)


two lines


- Jian Qiu


On Dec. 14, 2015, 5:23 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41333/
> ---
> 
> (Updated Dec. 14, 2015, 5:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper functions to filter usage slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/41333/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40632: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 6:41 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


Summary (updated)
-

Enabled oversubscribed resources for reservations in allocator.


Repository: mesos


Description (updated)
---

Enabled oversubscribed resources for reservations in allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 

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


Testing (updated)
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 41250: Enabled slave get ALLOCATION_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 7:13 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "slave/cpus_alloc_slack_revocable_percent": 0,
  "slave/cpus_alloc_slack_revocable_total": 3,
  "slave/cpus_alloc_slack_revocable_used": 0,
  "slave/disk_alloc_slack_revocable_percent": 0,
  "slave/disk_alloc_slack_revocable_total": 0,
  "slave/disk_alloc_slack_revocable_used": 0,
  "slave/mem_alloc_slack_revocable_percent": 0,
  "slave/mem_alloc_slack_revocable_total": 1000,
  "slave/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41254: Enable master get ALLOCATION_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 7:13 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enable master get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "master/cpus_alloc_slack_revocable_total": 3,
  "master/cpus_alloc_slack_revocable_used": 0,
  "master/disk_alloc_slack_revocable_total": 0,
  "master/disk_alloc_slack_revocable_used": 0,
  "master/mem_alloc_slack_revocable_total": 1000,
  "master/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41248: Enabled master get USAGE_SLACK metrics.

[Guangya Liu]

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

(Updated Dec. 14, 2015, 7:13 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled master get USAGE_SLACK metrics.


Diffs (updated)
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
usage_slack
  "master/cpus_usage_slack_revocable_percent": 0,
  "master/cpus_usage_slack_revocable_total": 0,
  "master/cpus_usage_slack_revocable_used": 0,
  "master/disk_usage_slack_revocable_percent": 0,
  "master/disk_usage_slack_revocable_total": 0,
  "master/disk_usage_slack_revocable_used": 0,
  "master/mem_usage_slack_revocable_percent": 0,
  "master/mem_usage_slack_revocable_total": 0,
  "master/mem_usage_slack_revocable_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

[Qian Zhang]

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



src/slave/slave.cpp 


So the method Containerizer::launch() with both taskInfo and executorInfo 
as parameters will never be called, and we only need to call the method 
Containerizer::launch() with only taskInfo as parameter, right?
But it seems not correct to me, e.g., in the code of Docker Containerizer 
(`DockerContainerizerProcess::launch()`), it actually needs taskinfo.


- Qian Zhang


On Dec. 12, 2015, 5:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41306/
> ---
> 
> (Updated Dec. 12, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: use command line executor to launch tasks
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>