Re: Review Request 40978: Modify the Http test process in FetcherTests to be symmetric in spawn/terminate.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 4, 2015, 6:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40978/
> ---
> 
> (Updated Dec. 4, 2015, 6:49 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses the comment here: 
> https://reviews.apache.org/r/40501/#comment168391
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 
> 
> Diff: https://reviews.apache.org/r/40978/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40978: Modify the Http test process in FetcherTests to be symmetric in spawn/terminate.

[Bernd Mathiske]

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

Ship it!


This looks a lot like other HTTP tests now. Good job replicating the prevailing 
patterns.

- Bernd Mathiske


On Dec. 4, 2015, 10:49 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40978/
> ---
> 
> (Updated Dec. 4, 2015, 10:49 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses the comment here: 
> https://reviews.apache.org/r/40501/#comment168391
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 
> 
> Diff: https://reviews.apache.org/r/40978/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41050: Added a paragraph to the release guide that handles API updates.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 7, 2015, 9:06 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41050/
> ---
> 
> (Updated Dec. 7, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 6a05a2600154c70228946cf13b86d5aa85ee45d4 
> 
> Diff: https://reviews.apache.org/r/41050/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40292: Added style guideline for writing numbers to markdown styleguide.

[Alexander Rojas]

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


-1 to this review. We are getting so many rules which regulate everything that 
instead of help its becomming a burden.

- Alexander Rojas


On Nov. 17, 2015, 9:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40292/
> ---
> 
> (Updated Nov. 17, 2015, 9:07 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added style guideline for writing numbers to markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 
> 
> Diff: https://reviews.apache.org/r/40292/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40224]

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

- Mesos ReviewBot


On Dec. 9, 2015, 2:20 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Dec. 9, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40978: Modify the Http test process in FetcherTests to be symmetric in spawn/terminate.

[Benjamin Bannier]

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

Ship it!



src/tests/fetcher_tests.cpp (line 278)


Unneeded dead code which should be removed (otherwise should be `= 
default`).



src/tests/fetcher_tests.cpp (line 295)


You could use `spawn(*process)` here to be more brief, have an additional 
`NULL` check, and to keep the appearance already present in `EXPECT_CALL`s here.


- Benjamin Bannier


On Dec. 4, 2015, 6:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40978/
> ---
> 
> (Updated Dec. 4, 2015, 6:49 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses the comment here: 
> https://reviews.apache.org/r/40501/#comment168391
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 
> 
> Diff: https://reviews.apache.org/r/40978/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41119: Cleaned up DRF allocator tests.

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Dec. 9, 2015, 5:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41119/
> ---
> 
> (Updated Dec. 9, 2015, 5:54 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up DRF allocator tests.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/41119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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 十二月 9, 2015, 12:38 p.m.)


Review request for mesos and Klaus Ma.


Repository: mesos


Description
---

WIP: Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  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.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40375, 40339]

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

Error:
 2015-12-09 13:01:36 URL:https://reviews.apache.org/r/40339/diff/raw/ 
[26264/26264] -> "40339.patch" [1]
error: patch failed: src/tests/reservation_tests.cpp:406
error: src/tests/reservation_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Dec. 9, 2015, 12:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Dec. 9, 2015, 12:38 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> 
> 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 十二月 9, 2015, 1:05 p.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 7acdc0a5d23a606eac2f37f4b7dd021c5a4fceb7 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
fb214a829a57529d3f5c49730ae9733f53e622ca 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_quota_tests.cpp 12820fcf7c0d2a791071464ad8ed738664ad85de 
  src/tests/persistent_volume_endpoints_tests.cpp 
0a03b5f1ac7dec14bd99c31768f86100f2b60616 
  src/tests/reservation_endpoints_tests.cpp 
c3833d0949df42c9f8dadf3c6e6b0b49e6cbdce9 
  src/tests/reservation_tests.cpp 3405b47c367e3c20e2a9139ab95dd8fd1805ea8d 
  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 40978: Modify the Http test process in FetcherTests to be symmetric in spawn/terminate.

[Bernd Mathiske]

> On Dec. 9, 2015, 2:31 a.m., Benjamin Bannier wrote:
> > src/tests/fetcher_tests.cpp, line 278
> > 
> >
> > Unneeded dead code which should be removed (otherwise should be `= 
> > default`).

You are right, but all other process constructors use the syntax used here. I 
suggest we leave it for now and discuss changing this offline (from this 
ticket).


> On Dec. 9, 2015, 2:31 a.m., Benjamin Bannier wrote:
> > src/tests/fetcher_tests.cpp, line 295
> > 
> >
> > You could use `spawn(*process)` here to be more brief, have an 
> > additional `NULL` check, and to keep the appearance already present in 
> > `EXPECT_CALL`s here.

Unfortunately there is no precedence for this. All other code like this uses 
get(). I think this is good enough because it is clear that this is a fresh 
instance that gets used here.


- Bernd


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


On Dec. 4, 2015, 10:49 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40978/
> ---
> 
> (Updated Dec. 4, 2015, 10:49 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses the comment here: 
> https://reviews.apache.org/r/40501/#comment168391
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 
> 
> Diff: https://reviews.apache.org/r/40978/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40978: Modify the Http test process in FetcherTests to be symmetric in spawn/terminate.

[Bernd Mathiske]

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



src/tests/fetcher_tests.cpp (line 278)


You are right, but all other process constructors use the syntax used here. 
I suggest we leave it for now and discuss changing this offline (from this 
ticket).



src/tests/fetcher_tests.cpp (line 295)


Unfortunately there is no precedence for this. All other code like this 
uses get(). I think this is good enough because it is clear that this is a 
fresh instance that gets used here.


- Bernd Mathiske


On Dec. 4, 2015, 10:49 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40978/
> ---
> 
> (Updated Dec. 4, 2015, 10:49 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This addresses the comment here: 
> https://reviews.apache.org/r/40501/#comment168391
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 
> 
> Diff: https://reviews.apache.org/r/40978/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40849: Fix flaky MemoryPressureMesosTests

[Benjamin Bannier]

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

Ship it!


Ship It!

- Benjamin Bannier


On Dec. 3, 2015, 7:01 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40849/
> ---
> 
> (Updated Dec. 3, 2015, 7:01 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Greg Mann, Artem Harutyunyan, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3586
> https://issues.apache.org/jira/browse/MESOS-3586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing tests will check that "low" pressure events occur at least as 
> often as "medium" pressure events (this is the documented behavior).  
> However, the order of events and the order in which we process said events is 
> not guaranteed.  When we collect the pressure events via a counter, there may 
> be some events that are in-flight, and thereby not accounted for in the 
> counters.
> 
> This patch modifies MemoryPressureMesosTests to wait for memory pressure 
> events to stop before checking the counts for correctness.
> The tests now stop the memory-pressure-triggering task and then wait for all 
> events to be processed before checking the counters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> e18b971c4df26c9b9c103ca73bdad4fd400d6c02 
> 
> Diff: https://reviews.apache.org/r/40849/diff/
> 
> 
> Testing
> ---
> 
> On Debian 8, Ubuntu 14, Centos 7 (Thanks Jan!):
> `make check`
> `sudo bin/mesos-tests.sh --gtest_filter="*MemoryPressureMesosTest*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> ^ Ran the above for a couple minutes or ~100 times.  It was previously 
> failing ~1/5 times.
> 
> ---
> Note on Centos 6 (Thanks Greg!):
> ```
> [ RUN  ] MemoryPressureMesosTest.CGROUPS_ROOT_Statistics
> ../../src/tests/mesos.cpp:849: Failure
> Value of: _baseHierarchy.get()
>   Actual: "/cgroup"
> Expected: baseHierarchy
> Which is: "/tmp/mesos_test_cgroup"
> -
> Multiple cgroups base hierarchies detected:
>   '/tmp/mesos_test_cgroup'
>   '/cgroup'
> Mesos does not support multiple cgroups base hierarchies.
> Please unmount the corresponding (or all) subsystems.
> -
> ../../src/tests/mesos.cpp:932: Failure
> (cgroups::destroy(hierarchy, cgroup)).failure(): Failed to remove cgroup 
> '/tmp/mesos_test_cgroup/perf_event/mesos_test': Device or resource busy
> [  FAILED  ] MemoryPressureMesosTest.CGROUPS_ROOT_Statistics (12 ms)
> ```
> 
> ---
> Note on Ubuntu 14:
> There is some other flakiness (in Agent recovery).  This will be tracked in 
> https://issues.apache.org/jira/browse/MESOS-4047
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40903: Ported approximated Option CPU resource number comparison to v1 and improved the check expression for this.

[Avinash sridharan]

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

Ship it!


Ship It!

- Avinash sridharan


On Dec. 3, 2015, 11:09 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40903/
> ---
> 
> (Updated Dec. 3, 2015, 11:09 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Ben Mahler, 
> Greg Mann, Klaus Ma, Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied the changes from common/resources.cpp in r40767 to v1/reources.cpp, 
> also addressing BenM's post-review. Thanks to @greggomann for spotting this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/40903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 41076: Added tests for implicit roles.

[Alexander Rukletsov]

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


It would be great to have a test for "/roles" endpoint when:
- quota is set for a role and
- some resources are reserved dynamically for a role.


src/tests/role_tests.cpp (line 219)


Third person singular, please : ). Also below.



src/tests/role_tests.cpp (line 327)


I would suggest to pull the role name into a constant for readability.



src/tests/role_tests.cpp (line 402)


Do you want to check the status code as well using 
`AWAIT_EXPECT_RESPONSE_STATUS_EQ`?



src/tests/role_tests.cpp (line 463)


Again, how about pulling these roles into constants?


- Alexander Rukletsov


On Dec. 9, 2015, 5:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41076/
> ---
> 
> (Updated Dec. 9, 2015, 5:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added tests for the "/role" HTTP endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41076/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41075: Added support for implicit roles.

[Alexander Rukletsov]

> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1543-1545
> > 
> >
> > Why don't we consider roles without frameworks but with a non-default 
> > weight active? Or roles with quota but without frameworks?
> > 
> > Or if you would like to reserve the term "active" for roles with 
> > frameworks, how about expanding the terminology to something like:
> > ```
> > Visible = {active} U {weighted} U {quota'ed} U {with-dyn-res}
> > ```
> > 
> > Anyway, my point is that we should show all "visible" roles here.
> 
> Neil Conway wrote:
> Yeah, this is a good point. Do we think that a user wants to see 
> `{active} U {weighted} U {quota'ed} U {with-dyn-res}`?
> 
> That does seem reasonable to me, although with the way the data 
> structures are organized, it will be somewhat ugly to implement :-\
> 
> Neil Conway wrote:
> Thinking about this a bit more:
> 
> * `/roles` currently doesn't show quotas.
> * It also doesn't show dynamically reserved resources.
> * If you set ACLs that involve a role, you could argue that it should 
> show up in `/roles` as well.
> 
> Given that most of these issues are not related to implicit roles as 
> such, I'm inclined to say we should defer making these changes -- perhaps as 
> part of the work on dynamic weights/ACLs, we can reorganize the data 
> structures to make this easier to implement anyway. I created 
> https://issues.apache.org/jira/browse/MESOS-4097 to track this.

I'm fine to do it in a separate ticket, however, I would argue it's a rather 
important desing decision for implicit roles. Let me elaborate on that.

Currently, there is actually no need to check role with quotas and dynamic 
reservations, because an operator *knows* there is no way a quota (or a dynamic 
reservations) can be set for a role which had not been specified in `--roles`.

When we switch to implicit roles, it's much harder to an operator to track 
roles, that I call "visible". Hence if we do not improve the "/roles" endpoint, 
I would say we worsen the opertator experience. Does it make sense?

Reagrding to the implementation. I've though about that and see two 
alternatives. Maintain multiple containers (as you do) and then assemble 
everything in the ednpoint handler. Or track "visible" roles directly (similar 
to reference counting in smart pointers). Neither solution is nice and concise. 
Curious what'd you say.


- Alexander


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


On Dec. 9, 2015, 5:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> ---
> 
> (Updated Dec. 9, 2015, 5:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 5d33138d60e684b23f07e1781de7991209d3e161 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 7acdc0a5d23a606eac2f37f4b7dd021c5a4fceb7 
>   src/master/quota_handler.cpp 

Re: Review Request 40292: Added style guideline for writing numbers to markdown styleguide.

[Greg Mann]

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


-1

Personally, I have not yet encountered in a review the suggestion that I should 
change the style of numbering, so based on my own experience I'm not sure that 
including this in the style guide will save us any time in the review cycle? 
It's also not obvious to me that there is a real disadvantage to having 
inconsistency in our numbering styles.

- Greg Mann


On Nov. 17, 2015, 8:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40292/
> ---
> 
> (Updated Nov. 17, 2015, 8:07 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added style guideline for writing numbers to markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 
> 
> Diff: https://reviews.apache.org/r/40292/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 41092: Added CMake file for agent executable build.

[Alex Clemmer]

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



src/slave/CMakeLists.txt (lines 17 - 19)


Historically, we have put the `*_TARGET` inside the relevant 
`*Configure.cmake` files. I recommend we follow this convention and put this 
target inside the `SlaveConfigure.cmake`.

Also, what do you think of calling it `AGENT_TARGET` instead of 
`AGENT_EXEC_TARGET`? I think the former is a lot clearer, personally, 
especially since we have exec directories in the project.



src/slave/CMakeLists.txt (line 21)


Possibly there should be a space between `#` and `SOURCE`?



src/slave/CMakeLists.txt (line 22)


Can we make the end of the line with `###` line up with the comment?



src/slave/CMakeLists.txt (line 24)


In the `CMakeLists.txt` that already exists, we tend to refer to these as 
`AGENT_`, for example in `src/CMakeLists.txt`, we call it `AGENT_SRC`. I'd like 
to propose we use a similar naming scheme here.



src/slave/CMakeLists.txt (line 26)


Alright, just to be clear: this will break the build, which is why we 
didn't `add_subdirectory(slave)`, right?

That's fine, I just wanted to call this out on the review.



src/slave/CMakeLists.txt (line 30)


My guess is that this is left over from the `CMakeLists.txt` in process? If 
so it seems like the `PROCESS` word on this line snuck in accidentally. :)



src/slave/CMakeLists.txt (line 32)


My recollection is that, when called with an empty source list, 
`add_executable` will result in an error that claims you didn't pass it enough 
arguments.

Therefore, I recommend that we actually just move the `if (NOT WIN32)` 
conditionals to just not `add_subdirectory` on the `slave/` directory.



src/slave/CMakeLists.txt (line 37)


It seems like these should possibly be in alphabetical order?



src/slave/CMakeLists.txt (line 39)


Can we remove the trailing whitespace? And add a period?


- Alex Clemmer


On Dec. 9, 2015, 3:43 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> ---
> 
> (Updated Dec. 9, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CMake file for agent executable build.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41092/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41090: Second iteration of changes for cmake build on linux.

[Alex Clemmer]

> On Dec. 8, 2015, 6:18 p.m., Joseph Wu wrote:
> > src/slave/cmake/FindCurl.cmake, line 28
> > 
> >
> > (I'm a bit of a CMake noob.)  Where is this defined?

`find_package` is a built-in, and when you define a macro with the name 
`FindX.cmake`, you automatically get the ability to type `find_package(X)` for 
free.

Gross or neat, depending on your perspective. :)


> On Dec. 8, 2015, 6:18 p.m., Joseph Wu wrote:
> > src/slave/cmake/FindCurl.cmake, line 92
> > 
> >
> > Where was this variable defined/declared/documented?

This is part of the `find_package` API, _cf_.[1] If you look down for the 
string `_FIND_QUIETLY` you can find the documentation.

https://cmake.org/cmake/help/v3.0/command/find_package.html


- Alex


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


On Dec. 9, 2015, 3:45 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41090/
> ---
> 
> (Updated Dec. 9, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Second iteration of changes for cmake build on linux.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/FindCurl.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41090/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Alex Clemmer]

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



src/CMakeLists.txt (lines 223 - 226)


Actually, I think I should have been more diligent about reviewing 
carefully here.

I think that this might make more sense down when we set `MESOS_SRC` with 
all the other variables listed here.

Per our conversation, I think your intuition that we really want to append 
this to `AGENT_SRC` is actually totally right, but my counterpoint is that we 
actually want the `AGENT_SRC` to include a bunch of these other source groups 
anyway, so appending to the `AGENT_SRC` really doesn't have too many advantages 
in my view.


- Alex Clemmer


On Dec. 9, 2015, 3:45 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40951/
> ---
> 
> (Updated Dec. 9, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial set of source files missing for cmake agent binary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/40951/diff/
> 
> 
> Testing
> ---
> 
> Tested to make sure library builds successfully.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Alex Clemmer]

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



src/CMakeLists.txt (lines 267 - 286)


Sorry, one more thing. :)

The definition of `MESOS_SRC` is split up by these two definitions. Do you 
think it makes more sense to not split up this definition by putting this block 
of code somewhere else?


- Alex Clemmer


On Dec. 9, 2015, 3:45 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40951/
> ---
> 
> (Updated Dec. 9, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial set of source files missing for cmake agent binary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/40951/diff/
> 
> 
> Testing
> ---
> 
> Tested to make sure library builds successfully.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41104: Addes some additional missing source files and put the source files in proper order

[Alex Clemmer]

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



src/CMakeLists.txt (line 248)


Per our conversation and #40951, I think it makes sense that we keep this 
`MESOS_SRC` and move it to be with the other definitions of `MESOS_SRC`.


- Alex Clemmer


On Dec. 8, 2015, 10:13 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41104/
> ---
> 
> (Updated Dec. 8, 2015, 10:13 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addes some additional missing source files and put the source files in proper 
> order
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/41104/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41108: Add curl, sasl and dl link flags and add protobuf library directory

[Alex Clemmer]

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



src/slave/cmake/SlaveConfigure.cmake (line 39)


I suspect this works on your machine because curl is on your include path 
somewhere, but in general, we want to add `${CURL_INCLUDE_DIR}` tot this code 
for when it's not.



src/slave/cmake/SlaveConfigure.cmake (line 62)


Same comment about the include directories, but with libraries. We probably 
want to include `CURL_LIBS` here.



src/slave/cmake/SlaveConfigure.cmake (line 85)


Per our convention in other configure files, we tend to put 
platform-conditional include and lib definitions near their original blocks. 
That is, we'd move the `if (LINUX)` include block to be just under the first 
definition of `PROCESS_INCLUDE_DIR`, and so on.



src/slave/cmake/SlaveConfigure.cmake (line 86)


Do we not need SASL, cURL, and DL for OS X? Looking at 
`StoutTestsConfigure`, it looks like we do.


- Alex Clemmer


On Dec. 8, 2015, 11:09 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41108/
> ---
> 
> (Updated Dec. 8, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add curl, sasl and dl link flags and add protobuf library directory
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/SlaveConfigure.cmake 
> fbdfdaa27fbd8c7429861eea5baf401a221f748b 
> 
> Diff: https://reviews.apache.org/r/41108/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41096: Added LFLAGs need for linux cmake build

[Alex Clemmer]

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



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (line 102)


Oh, this is good. In `StoutTestsConfigure`, we're manually including the dl 
flag, you should replace that with this infinitely superior solution. :)


- Alex Clemmer


On Dec. 9, 2015, 3:37 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41096/
> ---
> 
> (Updated Dec. 9, 2015, 3:37 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added LFLAGs need for linux cmake build
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> a9cd9902567cef4c7ab4125463a68e19907db0b4 
> 
> Diff: https://reviews.apache.org/r/41096/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40056: Make hook execution order deterministic.

[haosdent huang]

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

(Updated Dec. 9, 2015, 5:25 p.m.)


Review request for mesos, Ben Mahler, Kapil Arya, and Niklas Nielsen.


Changes
---

Rebase


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


Repository: mesos


Description
---

Make hook execution order deterministic.


Diffs (updated)
-

  src/hook/manager.cpp efbb02561afbabcc451aafdcf204b1fe478f2677 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 40056: Make hook execution order deterministic.

[haosdent huang]

> On Dec. 4, 2015, 9:15 p.m., Niklas Nielsen wrote:
> > Hi Haosdent!
> > 
> > I apologize the tardy reply. The patch looks good but needs rebasing.
> > Also, have you thought of a way to test this?
> > 
> > With a test (maybe by just ensuring the existing ordering of the test 
> > modules are indeed run in order), we should be able to land this. Kapil, 
> > any concerns on this?
> 
> Guangya Liu wrote:
> @nnielsen, does this test help? Thanks! 
> https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp#L93-L106
> 
> Niklas Nielsen wrote:
> That should be fine - rebase and let's get it in. (Kapil, any objections?)

Thx @gyliu and @nnielsen, let me rebase.


- haosdent


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


On Dec. 9, 2015, 5:25 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40056/
> ---
> 
> (Updated Dec. 9, 2015, 5:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Kapil Arya, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3485
> https://issues.apache.org/jira/browse/MESOS-3485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make hook execution order deterministic.
> 
> 
> Diffs
> -
> 
>   src/hook/manager.cpp efbb02561afbabcc451aafdcf204b1fe478f2677 
> 
> Diff: https://reviews.apache.org/r/40056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41092: Added CMake file for agent executable build.

[Alex Clemmer]

> On Dec. 9, 2015, 3:30 a.m., Joseph Wu wrote:
> > src/slave/CMakeLists.txt, line 42
> > 
> >
> > The pre-commit hooks will complain about this line.

Ah, I didn't know this was true. I thought `mesos-style` only ran on `.cpp` 
files? Is that wrong?


- Alex


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


On Dec. 9, 2015, 3:43 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> ---
> 
> (Updated Dec. 9, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CMake file for agent executable build.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41092/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

[Niklas Nielsen]

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


This is looking great, Bartek! One more round and I think we are there.


src/slave/qos_controllers/load.hpp (lines 39 - 45)


Awesome comment! Let's make it a doxygen style one.



src/slave/qos_controllers/load.hpp (line 42)


We usually only add our handles to TODOs. Let's just make them `NOTE: ` :)



src/slave/qos_controllers/load.cpp (lines 54 - 55)


Fits on one line



src/slave/qos_controllers/load.cpp (line 89)


Also, why not inline `__corrections`? It doesn't look like a continuation 
and is only called from one call site (above).

'{' needs to go on next line



src/slave/qos_controllers/load.cpp (line 99)


Expand '5min'



src/slave/qos_controllers/load.cpp (line 108)


Same here



src/slave/qos_controllers/load.cpp (lines 126 - 142)


This is a bit dense block; mind breaking it up a bit?



src/slave/qos_controllers/load.cpp (line 224)


Let's remove the trailing '.'
Maybe we can include some guidance on where to set the thresholds (in the 
module parameters). No biggie if you don't want that in.



src/tests/oversubscription_tests.cpp 


Why this removal?



src/tests/oversubscription_tests.cpp (lines 1110 - 1116)


Awesome comment!



src/tests/oversubscription_tests.cpp (line )


s/will be above/exceeds/



src/tests/oversubscription_tests.cpp (line 1126)


Can we wrap this in a smart pointer? Is there a potential race between the 
test code and the load qos controller when changing the values of the stub 
load, or how do we guarantee that?



src/tests/oversubscription_tests.cpp (lines 1142 - 1149)


Wonder if we can reduce some of this boiler plate; have you taken a look at 
`createTasks()`?


- Niklas Nielsen


On Dec. 8, 2015, 3:16 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40617/
> ---
> 
> (Updated Dec. 8, 2015, 3:16 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-4076
> https://issues.apache.org/jira/browse/MESOS-4076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Load QoS Controller for the simple eviction when system load is above 
> configured system load threshold for 5min and 15min:
> - Made os::loadavg called from the lambda and passed via the contructor.
> - Added unit test.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
>   src/slave/qos_controllers/load.hpp PRE-CREATION 
>   src/slave/qos_controllers/load.cpp PRE-CREATION 
>   src/tests/oversubscription_tests.cpp 
> 0333281c247dd182860a49f39be791c00679bf6b 
> 
> Diff: https://reviews.apache.org/r/40617/diff/
> 
> 
> Testing
> ---
> 
> make check
> run mesos with org_apache_mesos_LoadQoSController module included.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

[Gilbert Song]

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

(Updated Dec. 9, 2015, 10:36 a.m.)


Review request for mesos, Artem Harutyunyan, Jojy Varghese, and Timothy Chen.


Repository: mesos


Description
---

Fixed protobuf parse failure when pulling a docker image.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/message.proto 
824a788af0d2c779a2ca82a69ba65b6361154038 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
89f61c20e52e5eff8d8e92748f03b3b461516cd2 
  src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
c63bf53fee40ef12536a16e11f4d5224c4e4278e 

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


Testing
---

make check(ubuntu14.04 + clang3.6)


Thanks,

Gilbert Song

Re: Review Request 40838: Environment variable: Implemented `Env` specified in docker image returned from docker pull.

[Gilbert Song]

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

(Updated Dec. 9, 2015, 10:41 a.m.)


Review request for mesos, Artem Harutyunyan and Timothy Chen.


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


Repository: mesos


Description
---

Environment variable: Implemented `Env` specified in docker image returned from 
docker pull.


Diffs (updated)
-

  src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
  src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 40884: Environment variable: Implemented passing user taskinfo and docker image env var for docker containerizer.

[Gilbert Song]

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

(Updated Dec. 9, 2015, 10:43 a.m.)


Review request for mesos, Artem Harutyunyan and Timothy Chen.


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


Repository: mesos


Description
---

Environment variable: Implemented passing user taskinfo and docker image env 
var for docker containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Diana Arroyo]

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

(Updated Dec. 9, 2015, 9:15 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Merge later review r41104 into this review.


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


Repository: mesos


Description
---

Initial set of source files missing for cmake agent binary.


Diffs (updated)
-

  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 

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


Testing
---

Tested to make sure library builds successfully.


Thanks,

Diana Arroyo

Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40944]

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

- Mesos ReviewBot


On Dec. 9, 2015, 6:36 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40944/
> ---
> 
> (Updated Dec. 9, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jojy Varghese, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed protobuf parse failure when pulling a docker image.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> 824a788af0d2c779a2ca82a69ba65b6361154038 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 89f61c20e52e5eff8d8e92748f03b3b461516cd2 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/40944/diff/
> 
> 
> Testing
> ---
> 
> make check(ubuntu14.04 + clang3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 37703: Add docker exec command.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37703]

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

- Mesos ReviewBot


On Dec. 9, 2015, 5:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37703/
> ---
> 
> (Updated Dec. 9, 2015, 5:42 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3291
> https://issues.apache.org/jira/browse/MESOS-3291
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker exec command.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
>   src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
>   src/tests/containerizer/docker_tests.cpp 
> d9e1345aea0ef9db0e50360e3993ef2708970298 
> 
> Diff: https://reviews.apache.org/r/37703/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Diana Arroyo]

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

(Updated Dec. 9, 2015, 10:48 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

Initial set of source files missing for cmake agent binary.


Diffs (updated)
-

  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 

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


Testing
---

Tested to make sure library builds successfully.


Thanks,

Diana Arroyo

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Alex Clemmer]

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

Ship it!


Ship It!

- Alex Clemmer


On Dec. 9, 2015, 10:48 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40951/
> ---
> 
> (Updated Dec. 9, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial set of source files missing for cmake agent binary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/40951/diff/
> 
> 
> Testing
> ---
> 
> Tested to make sure library builds successfully.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41156: Employed a better macro in tests, cleaned up formatting.

[Alexander Rukletsov]

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

(Updated Dec. 9, 2015, 11:36 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


Changes
---

Expanded the scope of the patch, updated summary and description, +MPark.


Summary (updated)
-

Employed a better macro in tests, cleaned up formatting.


Repository: mesos


Description (updated)
---

Replaced all instances of `AWAIT_EXPECT_EQ({true|false}, ...)` with more 
appropriate `AWAIT_EXPECT_{TRUE|FALSE}(...)`. Also wrapped typenames in 
backticks in "authorization_tests.cpp".


Diffs (updated)
-

  src/tests/authorization_tests.cpp 32beeea3584f093bcac24e0c160235de0e3abb28 
  src/tests/group_tests.cpp 9db6162dae0a2657e12f4e99334f9cab65d14e9a 

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


Testing (updated)
---

`GTEST_FILTER="*AuthorizationTest*:*GroupTest*" make check -j7` on Mac OS 
10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 40382: Windows: Added threadsafe `strerror_r` implementation.

[Alex Clemmer]

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

(Updated Dec. 10, 2015, 12:08 a.m.)


Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
Remoortere, and Joseph Wu.


Repository: mesos


Description
---

Windows: Added threadsafe `strerror_r` implementaiton.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp 
71689089a7818e3465e7e51becc43c7836b67240 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
1a7037d64afeedc340258c92067e95d1d3caa027 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 41001: Improved 'ReservationTest.ACLMultipleOperations'.

[Greg Mann]

> On Dec. 9, 2015, 5:56 a.m., Michael Park wrote:
> > src/tests/reservation_tests.cpp, line 1660
> > 
> >
> > This one gets dropped because it's an invalid unreserve request, right? 
> > This is still what's intended to be tested?

Ah sorry, I intended to make this a valid request. Fixed.


> On Dec. 9, 2015, 5:56 a.m., Michael Park wrote:
> > src/tests/reservation_tests.cpp, lines 1703-1704
> > 
> >
> > Just to make sure I understand the intent of this patch, this is the 
> > part where the operations are dropped due to authorization rather than 
> > invalid request. Correct?

Yep, that's correct. In `master.cpp`, `accept()` passes a couple lists along to 
its continuation: one of them is a list of authorization results, another is a 
list of operations, and they are iterated through at the same time (the 
authorization at a given index corresponds to the operation at the same index). 
Performing both successful and failed RESERVE/UNRESERVE operations, along with 
an always-successful LAUNCH operation, tests that these authorization results 
are being passed to the continuation, and in the correct order.


- Greg


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


On Dec. 10, 2015, 2:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41001/
> ---
> 
> (Updated Dec. 10, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved 'ReservationTest.ACLMultipleOperations'.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 3405b47c367e3c20e2a9139ab95dd8fd1805ea8d 
> 
> Diff: https://reviews.apache.org/r/41001/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="ReservationTest.ACLMultipleOperations" bin/mesos-tests.sh 
> --gtest_repeat=1000 --gtest_break_on_failure`
> 
> This test was originally written to test the interaction of multiple 
> RESERVE/UNRESERVE/LAUNCH operations in a single `acceptOffers()` call when 
> authorization is enabled. In order to probe the effect of authorization on 
> this interaction, some operations should fail due to failed authorization. 
> However, this test included operations that failed simply because they were 
> malformed. I altered the test so that now operations will fail due to failed 
> authorization, which tests the desired functionality more precisely.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41001: Improved 'ReservationTest.ACLMultipleOperations'.

[Greg Mann]

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

(Updated Dec. 10, 2015, 2:02 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Addressed comment.


Repository: mesos


Description
---

Improved 'ReservationTest.ACLMultipleOperations'.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 3405b47c367e3c20e2a9139ab95dd8fd1805ea8d 

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


Testing
---

`GTEST_FILTER="ReservationTest.ACLMultipleOperations" bin/mesos-tests.sh 
--gtest_repeat=1000 --gtest_break_on_failure`

This test was originally written to test the interaction of multiple 
RESERVE/UNRESERVE/LAUNCH operations in a single `acceptOffers()` call when 
authorization is enabled. In order to probe the effect of authorization on this 
interaction, some operations should fail due to failed authorization. However, 
this test included operations that failed simply because they were malformed. I 
altered the test so that now operations will fail due to failed authorization, 
which tests the desired functionality more precisely.


Thanks,

Greg Mann

Re: Review Request 41178: Fixed a message dropping bug in the health checker.

[Neil Conway]

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



src/tests/health_check_tests.cpp (line 633)


Comment needs updating.


- Neil Conway


On Dec. 10, 2015, 2:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41178/
> ---
> 
> (Updated Dec. 10, 2015, 2:01 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Timothy Chen.
> 
> 
> Bugs: MESOS-1613 and MESOS-4106
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Much like in the command executor, we need to sleep after we send
> the final message in the health checker. Otherwise, we may exit
> before libprocess is able to finish sending the message over the
> local network.
> 
> This led to the following issues:
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Diffs
> -
> 
>   src/health-check/main.cpp 83ee38cd853325b3adc7cb6bc2d1d67b343037f5 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/41178/diff/
> 
> 
> Testing
> ---
> 
> Running the `HealthCheckTest.DISABLED_ConsecutiveFailures` test in repetition 
> on a machine loaded with many `openssl speed` commands in the background 
> reproduces the flakiness. After this patch it is no longer flaky in this 
> setup.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

[Vinod Kone]

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


Are you also planning to update 
https://github.com/apache/mesos/blob/master/docs/oversubscription.md ?


src/Makefile.am (line 1600)


Great to see a default QoS controller!



src/Makefile.am (lines 1602 - 1603)


how come the fixed estimator only has a source file and this one has a 
header too?



src/Makefile.am (line 1674)


do we need this? i don't see slave/resource_estimators/fixed.cpp here?



src/slave/qos_controllers/load.hpp (line 41)


s/below/above/ ?



src/slave/qos_controllers/load.hpp (lines 44 - 45)


Move comment this above the constructor.



src/slave/qos_controllers/load.cpp (lines 96 - 111)


you can refactor this to use a boolean "loaded" flag and inline 
`evictRevocableExecutors()`

also, the log lines are misleading. the controller doesn't evict. just say 
that the "Xmin load is higher than the threshold.."



src/slave/qos_controllers/load.cpp (line 124)


the function name is misleading. this is not evicting executors, it is just 
setting up corrections. it is the slave that decides to evict it.

also, i don't think this needs to be a function. why not inline this in 
`__corrections()`.



src/slave/qos_controllers/load.cpp (lines 126 - 142)


s/targets/corrections/



src/slave/qos_controllers/load.cpp (line 128)


no need for this temp variable.



src/slave/qos_controllers/load.cpp (line 189)


kill new line.



src/slave/qos_controllers/load.cpp (line 206)


why is this a warning instead of an error? seems like the operator would 
want to know if he made a typo in the parameter.



src/slave/qos_controllers/load.cpp (line 215)


ditto.



src/tests/oversubscription_tests.cpp (line 1121)


s/on/one/



src/tests/oversubscription_tests.cpp (lines 1162 - 1175)


push this inside the lamba below?



src/tests/oversubscription_tests.cpp (line 1178)


i think our style guide discourages use of "=" in favor of explicit capture.



src/tests/oversubscription_tests.cpp (line 1188)


s/First/Second/



src/tests/oversubscription_tests.cpp (line 1196)


this looks bad. you are deleting `stubLoad` while the load qos controller 
has access to it.


- Vinod Kone


On Dec. 8, 2015, 11:16 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40617/
> ---
> 
> (Updated Dec. 8, 2015, 11:16 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-4076
> https://issues.apache.org/jira/browse/MESOS-4076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Load QoS Controller for the simple eviction when system load is above 
> configured system load threshold for 5min and 15min:
> - Made os::loadavg called from the lambda and passed via the contructor.
> - Added unit test.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
>   src/slave/qos_controllers/load.hpp PRE-CREATION 
>   src/slave/qos_controllers/load.cpp PRE-CREATION 
>   src/tests/oversubscription_tests.cpp 
> 0333281c247dd182860a49f39be791c00679bf6b 
> 
> Diff: https://reviews.apache.org/r/40617/diff/
> 
> 
> Testing
> ---
> 
> make check
> run mesos with org_apache_mesos_LoadQoSController module included.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

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

(Updated Dec. 9, 2015, 3:15 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

We noticed that in some cases we delivered some corrupt packets to applications 
running in our containers. This is clearly wrong. 

Here is what happens:

1) We receive a corrupt packet externally
2) The hardware driver is able to checksum it and notices it has a bad checksum
3) The driver delivers this packet anyway to wait for TCP layer to checksum it 
again and then drop it
4) This packet is moved to a veth interface because it is for a container
5) Both sides of the veth pair have RX checksum offloading by default
6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
device has rx checksum offloading
7) Packet is moved into the container TCP/IP stack
8) TCP layer is not going to checksum it since it is not necessary
9) The packet gets delivered to application layer


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 

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


Testing
---

1) Turn rx checksum off manually and the bug is gone
2) Test this patch and verify rx checksum is turned off as expected.
3) I don't see any noticable performance issue after turning this off


Thanks,

Cong Wang

Re: Review Request 40951: CMake: Added missing source files to src/CMakeLists.txt.

[Joris Van Remoortere]

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

Ship it!



src/CMakeLists.txt (line 319)


Comments end with periods.


- Joris Van Remoortere


On Dec. 9, 2015, 11:35 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40951/
> ---
> 
> (Updated Dec. 9, 2015, 11:35 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/40951/diff/
> 
> 
> Testing
> ---
> 
> Tested to make sure library builds successfully.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Jie Yu]

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


This is more like a question: do we need to turn off tx side as well?

- Jie Yu


On Dec. 9, 2015, 11:15 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41158/
> ---
> 
> (Updated Dec. 9, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: MESOS-4105
> https://issues.apache.org/jira/browse/MESOS-4105
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We noticed that in some cases we delivered some corrupt packets to 
> applications running in our containers. This is clearly wrong. 
> 
> Here is what happens:
> 
> 1) We receive a corrupt packet externally
> 2) The hardware driver is able to checksum it and notices it has a bad 
> checksum
> 3) The driver delivers this packet anyway to wait for TCP layer to checksum 
> it again and then drop it
> 4) This packet is moved to a veth interface because it is for a container
> 5) Both sides of the veth pair have RX checksum offloading by default
> 6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
> device has rx checksum offloading
> 7) Packet is moved into the container TCP/IP stack
> 8) TCP layer is not going to checksum it since it is not necessary
> 9) The packet gets delivered to application layer
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
> 
> Diff: https://reviews.apache.org/r/41158/diff/
> 
> 
> Testing
> ---
> 
> 1) Turn rx checksum off manually and the bug is gone
> 2) Test this patch and verify rx checksum is turned off as expected.
> 3) I don't see any noticable performance issue after turning this off
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Alex Clemmer]

> On Dec. 10, 2015, 1:37 a.m., Joris Van Remoortere wrote:
> > src/slave/state.cpp, lines 39-43
> > 
> >
> > what about ``?
> 
> Alex Clemmer wrote:
> You committed it already, so, uh, well, I guess we'll get it next time.
> 
> #ThanksForPaddingMyNumberOfCommits

Ooops. #WhenYourHashtagsGetInterpretedAsMarkdownHeaders


- Alex


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


On Dec. 10, 2015, 1:27 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39219/
> ---
> 
> (Updated Dec. 10, 2015, 1:27 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3615
> https://issues.apache.org/jira/browse/MESOS-3615
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `slave/state.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
>   src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 
> 
> Diff: https://reviews.apache.org/r/39219/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Alex Clemmer]

> On Dec. 10, 2015, 1:37 a.m., Joris Van Remoortere wrote:
> > src/slave/state.cpp, lines 39-43
> > 
> >
> > what about ``?

You committed it already, so, uh, well, I guess we'll get it next time.

#ThanksForPaddingMyNumberOfCommits


- Alex


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


On Dec. 10, 2015, 1:27 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39219/
> ---
> 
> (Updated Dec. 10, 2015, 1:27 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3615
> https://issues.apache.org/jira/browse/MESOS-3615
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `slave/state.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
>   src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 
> 
> Diff: https://reviews.apache.org/r/39219/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39560: CMake: Add state.cpp, flags.cpp to Windows agent build.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Nov. 16, 2015, 9:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39560/
> ---
> 
> (Updated Nov. 16, 2015, 9:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Add state.cpp, flags.cpp to Windows agent build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt a14518831553c5eb55fe648d349b91bb55e641d9 
> 
> Diff: https://reviews.apache.org/r/39560/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

We noticed that in some cases we delivered some corrupt packets to applications 
running in our containers. This is clearly wrong. 

Here is what happens:

1) We receive a corrupt packet externally
2) The hardware driver is able to checksum it and notices it has a bad checksum
3) The driver delivers this packet anyway to wait for TCP layer to checksum it 
again and then drop it
4) This packet is moved to a veth interface because it is for a container
5) Both sides of the veth pair have RX checksum offloading by default
6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
device has rx checksum offloading
7) Packet is moved into the container TCP/IP stack
8) TCP layer is not going to checksum it since it is not necessary
9) The packet gets delivered to application layer


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 

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


Testing
---

1) Turn rx checksum off manually and the bug is gone
2) Test this patch and verify rx checksum is turned off as expected.
3) I don't see any noticable performance issue after turning this off


Thanks,

Cong Wang

Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

[Alex Clemmer]

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

(Updated Dec. 10, 2015, 2:14 a.m.)


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


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


Repository: mesos


Description
---

Windows: Added `WindowsError` to parallel `ErrnoError`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
a8c35c086ecae21701f6a720f25231c1b0d4e329 
  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
fdd33512c8d8752093f72f597a7d647eb5e3c285 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
PRE-CREATION 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Review Request 41156: Cleaned up formatting and employed a better macro in authz tests.

[Alexander Rukletsov]

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

Review request for mesos, Neil Conway and Till Toenshoff.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/authorization_tests.cpp 32beeea3584f093bcac24e0c160235de0e3abb28 

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


Testing
---

`GTEST_FILTER="*AuthorizationTest*" make check -j7` on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 40195: Changed commit hook linting to ignore empty diffs.

[Alex Clemmer]

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

(Updated Dec. 9, 2015, 11:09 p.m.)


Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

On Windows, if you attempt to commit an empty changeset, the commit hooks will
attempt to lint the entire repository. This is because we pass blank arguments
to the call to `xargs` that kicks off the C++ linter (e.g., in
`support/pre-commit`). In the git bash, the default behavior of blank arguments
is *not* to ignore them, as it is on certain other platforms. Note that the
`-r` flag is provided to avoid this behavior, but it is only available on
some platforms, and hence is inadmissable here.

Hence, our solution is to check if the results of `git diff` are empty, and
only call `mesos-style.py` if not.


Diffs
-

  support/hooks/post-rewrite 7df1e0f29c6ce940a364c0b1d312251c6160e5e3 
  support/hooks/pre-commit ca9e9810aca921734be5224e3ef71fe7ff4aa03d 

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


Testing
---

Branches tested manually.


Thanks,

Alex Clemmer

Re: Review Request 41159: Corrected a comment in reservation endpoint tests.

[Greg Mann]

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

Ship it!


Good catch, thanks!!

- Greg Mann


On Dec. 9, 2015, 11:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41159/
> ---
> 
> (Updated Dec. 9, 2015, 11:22 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> c3833d0949df42c9f8dadf3c6e6b0b49e6cbdce9 
> 
> Diff: https://reviews.apache.org/r/41159/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41001: Improved 'ReservationTest.ACLMultipleOperations'.

[Greg Mann]

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

(Updated Dec. 10, 2015, 1:18 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Addressed comments.


Repository: mesos


Description
---

Improved 'ReservationTest.ACLMultipleOperations'.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 3405b47c367e3c20e2a9139ab95dd8fd1805ea8d 

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


Testing
---

`GTEST_FILTER="ReservationTest.ACLMultipleOperations" bin/mesos-tests.sh 
--gtest_repeat=1000 --gtest_break_on_failure`

This test was originally written to test the interaction of multiple 
RESERVE/UNRESERVE/LAUNCH operations in a single `acceptOffers()` call when 
authorization is enabled. In order to probe the effect of authorization on this 
interaction, some operations should fail due to failed authorization. However, 
this test included operations that failed simply because they were malformed. I 
altered the test so that now operations will fail due to failed authorization, 
which tests the desired functionality more precisely.


Thanks,

Greg Mann

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Alex Clemmer]

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

(Updated Dec. 10, 2015, 1:27 a.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `slave/state.cpp`.


Diffs
-

  src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
  src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 

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


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Joris Van Remoortere]

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

Ship it!



src/slave/state.cpp (lines 36 - 40)


what about ``?


- Joris Van Remoortere


On Dec. 10, 2015, 1:27 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39219/
> ---
> 
> (Updated Dec. 10, 2015, 1:27 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3615
> https://issues.apache.org/jira/browse/MESOS-3615
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `slave/state.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
>   src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 
> 
> Diff: https://reviews.apache.org/r/39219/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 41178: Fixed a message dropping bug in the health checker.

[Ben Mahler]

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

Review request for mesos, Artem Harutyunyan and Timothy Chen.


Bugs: MESOS-1613 and MESOS-4106
https://issues.apache.org/jira/browse/MESOS-1613
https://issues.apache.org/jira/browse/MESOS-4106


Repository: mesos


Description
---

Much like in the command executor, we need to sleep after we send
the final message in the health checker. Otherwise, we may exit
before libprocess is able to finish sending the message over the
local network.

This led to the following issues:
https://issues.apache.org/jira/browse/MESOS-1613
https://issues.apache.org/jira/browse/MESOS-4106


Diffs
-

  src/health-check/main.cpp 83ee38cd853325b3adc7cb6bc2d1d67b343037f5 
  src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 

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


Testing
---

Running the `HealthCheckTest.DISABLED_ConsecutiveFailures` test in repetition 
on a machine loaded with many `openssl speed` commands in the background 
reproduces the flakiness. After this patch it is no longer flaky in this setup.


Thanks,

Ben Mahler

Re: Review Request 40195: Changed commit hook linting to ignore empty diffs.

[Alex Clemmer]

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

(Updated Dec. 9, 2015, 11:01 p.m.)


Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
Remoortere, and Joseph Wu.


Repository: mesos


Description (updated)
---

On Windows, if you attempt to commit an empty changeset, the commit hooks will
attempt to lint the entire repository. This is because we pass blank arguments
to the call to `xargs` that kicks off the C++ linter (e.g., in
`support/pre-commit`). In the git bash, the default behavior of blank arguments
is *not* to ignore them, as it is on certain other platforms. Note that the
`-r` flag is provided to avoid this behavior, but it is only available on
some platforms, and hence is inadmissable here.

Hence, our solution is to check if the results of `git diff` are empty, and
only call `mesos-style.py` if not.


Diffs
-

  support/hooks/post-rewrite 7df1e0f29c6ce940a364c0b1d312251c6160e5e3 
  support/hooks/pre-commit ca9e9810aca921734be5224e3ef71fe7ff4aa03d 

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


Testing
---

Branches tested manually.


Thanks,

Alex Clemmer

Re: Review Request 40195: Changed commit hook linting to ignore empty diffs.

[Joris Van Remoortere]

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

Ship it!


After updating the description to match the code.

- Joris Van Remoortere


On Dec. 9, 2015, 11:01 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40195/
> ---
> 
> (Updated Dec. 9, 2015, 11:01 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, if you attempt to commit an empty changeset, the commit hooks will
> attempt to lint the entire repository. This is because we pass blank arguments
> to the call to `xargs` that kicks off the C++ linter (e.g., in
> `support/pre-commit`). In the git bash, the default behavior of blank 
> arguments
> is *not* to ignore them, as it is on certain other platforms. Note that the
> `-r` flag is provided to avoid this behavior, but it is only available on
> some platforms, and hence is inadmissable here.
> 
> Hence, our solution is to check if the results of `git diff` are empty, and
> only call `mesos-style.py` if not.
> 
> 
> Diffs
> -
> 
>   support/hooks/post-rewrite 7df1e0f29c6ce940a364c0b1d312251c6160e5e3 
>   support/hooks/pre-commit ca9e9810aca921734be5224e3ef71fe7ff4aa03d 
> 
> Diff: https://reviews.apache.org/r/40195/diff/
> 
> 
> Testing
> ---
> 
> Branches tested manually.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 40951: CMake: Added missing source files to src/CMakeLists.txt.

[Diana Arroyo]

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

(Updated Dec. 9, 2015, 11:35 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Changed the summary to match the commit message.


Summary (updated)
-

CMake: Added missing source files to src/CMakeLists.txt.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 

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


Testing
---

Tested to make sure library builds successfully.


Thanks,

Diana Arroyo

Re: Review Request 40056: Make hook execution order deterministic.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十二月 9, 2015, 5:25 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40056/
> ---
> 
> (Updated 十二月 9, 2015, 5:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Kapil Arya, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3485
> https://issues.apache.org/jira/browse/MESOS-3485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make hook execution order deterministic.
> 
> 
> Diffs
> -
> 
>   src/hook/manager.cpp efbb02561afbabcc451aafdcf204b1fe478f2677 
> 
> Diff: https://reviews.apache.org/r/40056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40382: Windows: Added threadsafe `strerror_r` implementation.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 10, 2015, 1:06 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40382/
> ---
> 
> (Updated Dec. 10, 2015, 1:06 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4107
> https://issues.apache.org/jira/browse/MESOS-4107
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added threadsafe `strerror_r` implementaiton.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp 
> 71689089a7818e3465e7e51becc43c7836b67240 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/40382/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 40382: Windows: Added threadsafe `strerror_r` implementation.

[Alex Clemmer]

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

(Updated Dec. 10, 2015, 1:06 a.m.)


Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

Windows: Added threadsafe `strerror_r` implementaiton.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp 
71689089a7818e3465e7e51becc43c7836b67240 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
1a7037d64afeedc340258c92067e95d1d3caa027 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 41156: Employed a better macro in tests, cleaned up formatting.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41156]

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

Error:
 2015-12-10 01:55:11 URL:https://reviews.apache.org/r/41156/diff/raw/ 
[14423/14423] -> "41156.patch" [1]
error: patch failed: src/tests/authorization_tests.cpp:390
error: src/tests/authorization_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Dec. 9, 2015, 11:36 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41156/
> ---
> 
> (Updated Dec. 9, 2015, 11:36 p.m.)
> 
> 
> Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced all instances of `AWAIT_EXPECT_EQ({true|false}, ...)` with more 
> appropriate `AWAIT_EXPECT_{TRUE|FALSE}(...)`. Also wrapped typenames in 
> backticks in "authorization_tests.cpp".
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 32beeea3584f093bcac24e0c160235de0e3abb28 
>   src/tests/group_tests.cpp 9db6162dae0a2657e12f4e99334f9cab65d14e9a 
> 
> Diff: https://reviews.apache.org/r/41156/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*AuthorizationTest*:*GroupTest*" make check -j7` on Mac OS 
> 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Neil Conway]

> On Dec. 8, 2015, 2:55 p.m., Alexander Rukletsov wrote:
> > src/tests/role_tests.cpp, lines 83-84
> > 
> >
> > `MesosTest` exposes the `defaultAgentResourcesString` constant. I think 
> > you can get rid of these lines in order not to distract the user and make 
> > sure `unreserved` is contained in `defaultAgentResourcesString` later in 
> > the code:
> > ```
> > 
> > EXPECT_TRUE(Resources::parse(defaultAgentResourcesString).get().contains(unreserved));
> > ```

I looked at doing this, but I didn't feel like it was an improvement: I think 
it is more readable for the test case to be explicit about the resources we 
expect to be available on the agent.


- Neil


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


On Dec. 9, 2015, 5:53 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 9, 2015, 5:53 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/tests/reservation_tests.cpp eccbb8f8a02b65b26f34e020e736afe0445a6d0d 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 41113: Added `IsolatorRecoveryInfo` message as the sole parameter to `Isolator::recover()`.

[Greg Mann]

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

Review request for mesos, Adam B, Ben Mahler, Jie Yu, and Neil Conway.


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


Repository: mesos


Description
---

This patch adds the message `IsolatorRecoveryInfo` and makes it the sole 
parameter of `Isolator::recover()`, in order to facilitate the future addition 
of parameters without breaking the interface.

In addition to the two existing parameters of `Isolator::recover()`, a third 
member was added to `IsolatorRecoveryInfo`: the agent's `work_dir`. This is 
useful for the network storage DVD isolator 
(https://github.com/emccode/mesos-module-dvdi) in particular.


Diffs
-

  include/mesos/slave/isolator.hpp 95a2933988ea7c9b9404df5e12031f134712b2b5 
  include/mesos/slave/isolator.proto d2032adf9336119ed8e1ff3c813d657d70331b67 
  src/common/protobuf_utils.hpp 7280c9fe36726df6b02ff468c7bd5ecedf5f5023 
  src/common/protobuf_utils.cpp 6e1eb0b8465809d1da5dac1cd29b692b9fa6ff66 
  src/slave/containerizer/mesos/containerizer.cpp 
6dad2e858b68cf47e048d49d34af4fa4cb3b6841 
  src/slave/containerizer/mesos/isolator.hpp 
937f253656d36ed10b47ceeb0b6101f212e65586 
  src/slave/containerizer/mesos/isolator.cpp 
493b5dd26cf0e8f986381a502cfa6d1dde6573d4 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
123b9ed3ccaebcd5da24fc62ff7a92d4a81ed760 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
3b95e195ad704f163c245175390d9a26bde7e17c 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
09952369c72d3c6322ae7a1c73cd68226d452ad2 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
2ddb9f4adbb879682cd39966ab974cf3fa32209c 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
5eaf49f1f35c93ad4465adb6c9c9cf57b3a2c6ee 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
b7ba00bc495001380f01737e46e8671ffe1c2ef7 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
b8d47e8250a892fa333a0a966a0f38fe1f2816f2 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
8442e9c30612fa04f34130b9f967cb1414880ca6 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
c3544aa313cbb185efb03bba59961cdf2b616a37 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
00ff84b6cd0aa29fa5a7918d7f88d480af8752ca 
  src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
2e457015a0348a457581edf493877b71fab17090 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
361ed6561bd5e2f75d026922def01f42b43d61c2 
  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
c2d1455249618f9cd2e17dc2244b184d52b32eaf 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
d65c1593b44f4b21237581147e57e441ebf3160d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 
  src/slave/containerizer/mesos/isolators/posix.hpp 
7e1ebc2fada5a5e291e84c7044bdba9a71f4b42c 
  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
31808c1e8199fbf2cea36c273860fdbf0a2388f8 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
d971db09083faad08f3cf18c25a79245321d1d9a 
  src/tests/containerizer/isolator.hpp e4101b188560bd857ea104f61f52f27c880e7731 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
fe679354d04d68b68e168cd8c4eab23898f6532f 

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


Testing
---

`sudo bin/mesos-tests.sh` was run on OSX and Ubuntu in order to test both the 
posix and linux isolator code. Only expected test failures were observed. Some 
of the failures on Ubuntu are `SlaveRecoveryTest`s, which is a bit 
disconcerting, but this issue is tracked in MESOS-4025, and seems to be due to 
artifacts left behind by previous tests. If I do:

`GTEST_FILTER="" make check`
`sudo GTEST_FILTER="SlaveRecoveryTest*" bin/mesos-tests.sh`

then all the `SlaveRecoveryTest`s pass.


Thanks,

Greg Mann

Re: Review Request 40905: [libprocess]: Made `MethodNotAllowed` response compliant to RFC 2616.

[Alexander Rukletsov]

> On Dec. 3, 2015, 5:10 p.m., Anand Mazumdar wrote:
> > 3rdparty/libprocess/include/process/http.hpp, line 582
> > 
> >
> > hmmm , why do we need another constructor with no response body ?
> > 
> > We have already seen previous bugs from end-users about us not setting 
> > `Content-Type` for some error `Response` types like `BadRequest`. Hence, 
> > empty body responses should be avoided at best unless I am missing 
> > something ?

I followed the pattern we use for other response types. I think we should clean 
it up at once in a separate patch.


- Alexander


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


On Dec. 3, 2015, 2:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40905/
> ---
> 
> (Updated Dec. 3, 2015, 2:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4056
> https://issues.apache.org/jira/browse/MESOS-4056
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to RFC 2616 
> (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.7), 
> `MethodNotAllowed` response must include an 'Allow' header containing a list 
> of valid methods.
> 
> This is a libprocess change only.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
> 
> Diff: https://reviews.apache.org/r/40905/diff/
> 
> 
> Testing
> ---
> 
> This a libprocess change only. The code does not compile without the 
> follow-up [mesos] patch https://reviews.apache.org/r/40913/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 41159: Corrected a comment in reservation endpoint tests.

[Alexander Rukletsov]

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

Review request for mesos, Greg Mann and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
c3833d0949df42c9f8dadf3c6e6b0b49e6cbdce9 

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


Testing
---

None.


Thanks,

Alexander Rukletsov

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 十二月 9, 2015, 11:33 p.m.)


Review request for mesos and Klaus Ma.


Repository: mesos


Description
---

WIP: Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 9, 2015, 11:15 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41158/
> ---
> 
> (Updated Dec. 9, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: MESOS-4105
> https://issues.apache.org/jira/browse/MESOS-4105
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We noticed that in some cases we delivered some corrupt packets to 
> applications running in our containers. This is clearly wrong. 
> 
> Here is what happens:
> 
> 1) We receive a corrupt packet externally
> 2) The hardware driver is able to checksum it and notices it has a bad 
> checksum
> 3) The driver delivers this packet anyway to wait for TCP layer to checksum 
> it again and then drop it
> 4) This packet is moved to a veth interface because it is for a container
> 5) Both sides of the veth pair have RX checksum offloading by default
> 6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
> device has rx checksum offloading
> 7) Packet is moved into the container TCP/IP stack
> 8) TCP layer is not going to checksum it since it is not necessary
> 9) The packet gets delivered to application layer
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
> 
> Diff: https://reviews.apache.org/r/41158/diff/
> 
> 
> Testing
> ---
> 
> 1) Turn rx checksum off manually and the bug is gone
> 2) Test this patch and verify rx checksum is turned off as expected.
> 3) I don't see any noticable performance issue after turning this off
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40375, 40339, 40529, 40532, 40632]

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

Error:
 + : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ cd /home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-amd64_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-i386_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release.gpg 
/var/lib/apt/lists/lock 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-i386_Packages
 /var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release.gpg 
/var/lib/apt/lists/partial 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-i386_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_i18n_Translation-en
 /va
 
r/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_source_Sources
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_Release
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_InRelease 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_i18n_Translation-en
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiver
 se_source_Sources 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_InRelease
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-amd64_Packages
 /var/lib/apt/lists/u
 s.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-i386_Packages 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_i18n_Translation-en
 /var/lib/apt/lists
 /us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_source_Sources 

Re: Review Request 41090: CMake: Added FindCurl.cmake script to locate curl library.

[Diana Arroyo]

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

(Updated Dec. 10, 2015, 1:10 a.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Addresses latest review comments.


Summary (updated)
-

CMake: Added FindCurl.cmake script to locate curl library.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/slave/cmake/FindCurl.cmake PRE-CREATION 

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


Testing
---

Tested on Ubuntu and OSX.


Thanks,

Diana Arroyo

Re: Review Request 41090: CMake: Added FindCurl.cmake script to locate curl library.

[Alex Clemmer]

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

Ship it!


Ship It!

- Alex Clemmer


On Dec. 10, 2015, 1:10 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41090/
> ---
> 
> (Updated Dec. 10, 2015, 1:10 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/FindCurl.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41090/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Ian Downes]

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

Ship it!



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 3572)


s/kernel/the kernel/



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 3574)


s/could/to


- Ian Downes


On Dec. 9, 2015, 3:15 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41158/
> ---
> 
> (Updated Dec. 9, 2015, 3:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: MESOS-4105
> https://issues.apache.org/jira/browse/MESOS-4105
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We noticed that in some cases we delivered some corrupt packets to 
> applications running in our containers. This is clearly wrong. 
> 
> Here is what happens:
> 
> 1) We receive a corrupt packet externally
> 2) The hardware driver is able to checksum it and notices it has a bad 
> checksum
> 3) The driver delivers this packet anyway to wait for TCP layer to checksum 
> it again and then drop it
> 4) This packet is moved to a veth interface because it is for a container
> 5) Both sides of the veth pair have RX checksum offloading by default
> 6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
> device has rx checksum offloading
> 7) Packet is moved into the container TCP/IP stack
> 8) TCP layer is not going to checksum it since it is not necessary
> 9) The packet gets delivered to application layer
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
> 
> Diff: https://reviews.apache.org/r/41158/diff/
> 
> 
> Testing
> ---
> 
> 1) Turn rx checksum off manually and the bug is gone
> 2) Test this patch and verify rx checksum is turned off as expected.
> 3) I don't see any noticable performance issue after turning this off
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 41156: Cleaned up formatting and employed a better macro in authz tests.

[Alexander Rukletsov]

> On Dec. 9, 2015, 10:58 p.m., Neil Conway wrote:
> > There are a few more instances of the `AWAIT_EXPECT_EQ(true, ...)` pattern 
> > in group_tests.cpp -- want to fix those as well?

I was looking at "authorization_tests.cpp", but... why not? Will do!


- Alexander


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


On Dec. 9, 2015, 10:53 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41156/
> ---
> 
> (Updated Dec. 9, 2015, 10:53 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 32beeea3584f093bcac24e0c160235de0e3abb28 
> 
> Diff: https://reviews.apache.org/r/41156/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*AuthorizationTest*" make check -j7` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41159: Corrected a comment in reservation endpoint tests.

[Neil Conway]

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

Ship it!


Ship It!

- Neil Conway


On Dec. 9, 2015, 11:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41159/
> ---
> 
> (Updated Dec. 9, 2015, 11:22 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> c3833d0949df42c9f8dadf3c6e6b0b49e6cbdce9 
> 
> Diff: https://reviews.apache.org/r/41159/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40906: Replaced `BadRequest` with `MethodNotAllowed` for all HTTP requests with unsupported methods.

[Alexander Rukletsov]

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

(Updated Dec. 10, 2015, 12:13 a.m.)


Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Joris Van 
Remoortere.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40913: Made `MethodNotAllowed` response compliant to RFC 2616.

[Alexander Rukletsov]

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

(Updated Dec. 10, 2015, 12:12 a.m.)


Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Joris Van 
Remoortere.


Changes
---

Rebased.


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


Repository: mesos


Description
---

According to RFC 2616 
(http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.7), 
`MethodNotAllowed` response must include an 'Allow' header containing a list of 
valid methods.

This updates `MethodNotAllowed` c-tor invocations in Mesos codebase.


Diffs (updated)
-

  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
  src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
  src/tests/executor_http_api_tests.cpp 
1be657c64d9ead14f1e2b41e35fd6ca04a0a56a4 
  src/tests/scheduler_http_api_tests.cpp 
4f52309ff50a5b56cf20f2c5cfddd9c10b2b75d9 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.

[Alex Clemmer]

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

(Updated Dec. 10, 2015, 1:14 a.m.)


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


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


Repository: mesos


Description
---

Windows: Implemented `os::mkdtemp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
a8c35c086ecae21701f6a720f25231c1b0d4e329 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 41178: Fixed a message dropping bug in the health checker.

[Ben Mahler]

> On Dec. 10, 2015, 2:10 a.m., Neil Conway wrote:
> > src/tests/health_check_tests.cpp, line 633
> > 
> >
> > Comment needs updating.

Thanks for catching this!


- Ben


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


On Dec. 10, 2015, 2:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41178/
> ---
> 
> (Updated Dec. 10, 2015, 2:01 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Timothy Chen.
> 
> 
> Bugs: MESOS-1613 and MESOS-4106
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Much like in the command executor, we need to sleep after we send
> the final message in the health checker. Otherwise, we may exit
> before libprocess is able to finish sending the message over the
> local network.
> 
> This led to the following issues:
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Diffs
> -
> 
>   src/health-check/main.cpp 83ee38cd853325b3adc7cb6bc2d1d67b343037f5 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/41178/diff/
> 
> 
> Testing
> ---
> 
> Running the `HealthCheckTest.DISABLED_ConsecutiveFailures` test in repetition 
> on a machine loaded with many `openssl speed` commands in the background 
> reproduces the flakiness. After this patch it is no longer flaky in this 
> setup.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41096: CMake: Added LFLAGs need for linux cmake build

[Diana Arroyo]

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

(Updated Dec. 10, 2015, 3:16 a.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Added AGENT target definition.


Summary (updated)
-

CMake: Added LFLAGs need for linux cmake build


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
a9cd9902567cef4c7ab4125463a68e19907db0b4 

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


Testing
---

Tested on Ubuntu and OSX.


Thanks,

Diana Arroyo

Re: Review Request 41188: Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages to HTTP endpoints

[Avinash sridharan]

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

(Updated Dec. 10, 2015, 3:36 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Providing JSON bindings to so that mesos modules can expose DiscoveryInfo 
protobuf messages to HTTP endpoints


Diffs
-

  src/common/http.hpp 6a7a14ce8f1b9e0a85bc88bcf32bd80ea56bb7be 
  src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
  src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 

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


Testing (updated)
---

make check

I haven't written explicit unit test cases for the JSON serialization for the 
new protobuf messages.


Thanks,

Avinash sridharan

Re: Review Request 41188: Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages to HTTP endpoints

[Avinash sridharan]

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

(Updated Dec. 10, 2015, 3:36 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Providing JSON bindings to so that mesos modules can expose DiscoveryInfo 
protobuf messages to HTTP endpoints


Diffs
-

  src/common/http.hpp 6a7a14ce8f1b9e0a85bc88bcf32bd80ea56bb7be 
  src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
  src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 

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


Testing (updated)
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41188: Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages to HTTP endpoints

[Anand Mazumdar]

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


1. We badly need a unit test for this. :-)
2. If I understand correctly, the reason you did not use the automated protobuf 
to JSON converters and had to use `model(...)` is that `labels` is populated in 
a custom way in the `state.json` endpoint and you wanted to preserve that ?


src/common/http.hpp (line 84)


Based on my comment in an earlier review , do we need this ?



src/common/http.cpp (line 158)


Missing period at the end.



src/common/http.cpp (line 159)


Do we need this ?



src/common/http.cpp (line 195)


Missing period at the end.



src/slave/http.cpp (line 96)


Missing Period


- Anand Mazumdar


On Dec. 10, 2015, 3:36 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41188/
> ---
> 
> (Updated Dec. 10, 2015, 3:36 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Providing JSON bindings to so that mesos modules can expose DiscoveryInfo 
> protobuf messages to HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 6a7a14ce8f1b9e0a85bc88bcf32bd80ea56bb7be 
>   src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
>   src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
> 
> Diff: https://reviews.apache.org/r/41188/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41159: Corrected a comment in reservation endpoint tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41159]

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

- Mesos ReviewBot


On Dec. 9, 2015, 11:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41159/
> ---
> 
> (Updated Dec. 9, 2015, 11:22 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> c3833d0949df42c9f8dadf3c6e6b0b49e6cbdce9 
> 
> Diff: https://reviews.apache.org/r/41159/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40903: Ported approximated Option CPU resource number comparison to v1 and improved the check expression for this.

[Klaus Ma]

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

Ship it!


Ship It!

- Klaus Ma


On Dec. 3, 2015, 7:09 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40903/
> ---
> 
> (Updated Dec. 3, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Ben Mahler, 
> Greg Mann, Klaus Ma, Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied the changes from common/resources.cpp in r40767 to v1/reources.cpp, 
> also addressing BenM's post-review. Thanks to @greggomann for spotting this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/40903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 41182: Exposed task health flag through the state endpoint on master and slave.

[Ben Mahler]

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

Ship it!


I shortened some of the comments, but looks great, thanks!


src/tests/health_check_tests.cpp (line 278)


Do we need this comment given you have the ones above these blocks?


- Ben Mahler


On Dec. 10, 2015, 2:50 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41182/
> ---
> 
> (Updated Dec. 10, 2015, 2:50 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4015
> https://issues.apache.org/jira/browse/MESOS-4015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed task health flag through the state endpoint on master and slave.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 586d1c833513836ae0d4041c9f94c6332b25fdd5 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/41182/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41092: Added CMake file for agent executable build.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40951, 41090, 41092]

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

Error:
 2015-12-10 03:23:34 URL:https://reviews.apache.org/r/41092/diff/raw/ 
[1967/1967] -> "41092.patch" [1]
41092.patch:45: trailing whitespace.
# ADD LINKER FLAGS 
41092.patch:48: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Successfully applied: Added CMake file for agent executable build.

Added CMake file for agent executable build.


Review: https://reviews.apache.org/r/41092
src/slave/CMakeLists.txt:39: trailing whitespace.
+# ADD LINKER FLAGS 
src/slave/CMakeLists.txt:42: new blank line at EOF.
Failed to commit patch

- Mesos ReviewBot


On Dec. 9, 2015, 3:43 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> ---
> 
> (Updated Dec. 9, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CMake file for agent executable build.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41092/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40906: Replaced `BadRequest` with `MethodNotAllowed` for all HTTP requests with unsupported methods.

[Anand Mazumdar]

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

Ship it!


Ship It!

- Anand Mazumdar


On Dec. 10, 2015, 12:13 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40906/
> ---
> 
> (Updated Dec. 10, 2015, 12:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4056
> https://issues.apache.org/jira/browse/MESOS-4056
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
> 
> Diff: https://reviews.apache.org/r/40906/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41178: Fixed a message dropping bug in the health checker.

[Artem Harutyunyan]

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

Ship it!


Ship It!

- Artem Harutyunyan


On Dec. 9, 2015, 6:01 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41178/
> ---
> 
> (Updated Dec. 9, 2015, 6:01 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Timothy Chen.
> 
> 
> Bugs: MESOS-1613 and MESOS-4106
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Much like in the command executor, we need to sleep after we send
> the final message in the health checker. Otherwise, we may exit
> before libprocess is able to finish sending the message over the
> local network.
> 
> This led to the following issues:
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Diffs
> -
> 
>   src/health-check/main.cpp 83ee38cd853325b3adc7cb6bc2d1d67b343037f5 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/41178/diff/
> 
> 
> Testing
> ---
> 
> Running the `HealthCheckTest.DISABLED_ConsecutiveFailures` test in repetition 
> on a machine loaded with many `openssl speed` commands in the background 
> reproduces the flakiness. After this patch it is no longer flaky in this 
> setup.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp (lines 27 - 29)


Let's move this into ``



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 1 - 
13)


new license header format.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 21)


Maybe we can drop the `Try` specific part of this comment.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 43)


Full words if not part of the API.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 47)


new line.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 61 - 
63)


let's clarify this as per offline discussion.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 77)


You can drop the "as far as I can tell"



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 82 - 
88)


indentation.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 86)


I think we put a space between the old c-style cast and the value. Can you 
check this?


- Joris Van Remoortere


On Dec. 10, 2015, 2:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> ---
> 
> (Updated Dec. 10, 2015, 2:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `WindowsError` to parallel `ErrnoError`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> fdd33512c8d8752093f72f597a7d647eb5e3c285 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39583/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 41178: Fixed a message dropping bug in the health checker.

[Ben Mahler]

> On Dec. 10, 2015, 2:35 a.m., Artem Harutyunyan wrote:
> > src/health-check/main.cpp, line 120
> > 
> >
> > Do we need to create a JIRA for eventually get rid of the hack?

Good idea, I filed MESOS-4111 and will reference it in a TODO. Will also add a 
reference in the command executor sleep.


- Ben


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


On Dec. 10, 2015, 2:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41178/
> ---
> 
> (Updated Dec. 10, 2015, 2:01 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Timothy Chen.
> 
> 
> Bugs: MESOS-1613 and MESOS-4106
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Much like in the command executor, we need to sleep after we send
> the final message in the health checker. Otherwise, we may exit
> before libprocess is able to finish sending the message over the
> local network.
> 
> This led to the following issues:
> https://issues.apache.org/jira/browse/MESOS-1613
> https://issues.apache.org/jira/browse/MESOS-4106
> 
> 
> Diffs
> -
> 
>   src/health-check/main.cpp 83ee38cd853325b3adc7cb6bc2d1d67b343037f5 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/41178/diff/
> 
> 
> Testing
> ---
> 
> Running the `HealthCheckTest.DISABLED_ConsecutiveFailures` test in repetition 
> on a machine loaded with many `openssl speed` commands in the background 
> reproduces the flakiness. After this patch it is no longer flaky in this 
> setup.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

[Avinash sridharan]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Adding labels field to Port information, for service discovery to associate 
arbitrary tags by applications to ports for a given task.


Diffs
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 

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


Testing
---

make check


Thanks,

Avinash sridharan

Review Request 41182: Exposed task health flag through the state endpoint on master and slave.

[Artem Harutyunyan]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Exposed task health flag through the state endpoint on master and slave.


Diffs
-

  src/common/http.cpp 586d1c833513836ae0d4041c9f94c6332b25fdd5 
  src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 

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


Testing
---

make check


Thanks,

Artem Harutyunyan

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Joris Van Remoortere]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (lines 1 - 
13)


header format change.



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


`#include `
`#include `
`#include `



3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp (lines 1 - 13)


header format change.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 1 
- 13)


header format change.



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


indentation.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 58)


`because folder guaranteed`
can you fix the grammar here?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 79 
- 80)


indentation



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 90 
- 91)


indentation.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 93 
- 94)


let's keep the else on the same line as the previous closing brace.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 
100 - 101)


indentation.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 109)


Can you explain / comment why here (as well as on L139) we use 
`ErrnoError`, yet elsewhere we use `WindowsError`?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 
110 - 111)


indentation.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 
133 - 134)


indentation.


- Joris Van Remoortere


On Nov. 16, 2015, 9:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Nov. 16, 2015, 9:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 40905: [libprocess]: Made `MethodNotAllowed` response compliant to RFC 2616.

[Anand Mazumdar]

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

Ship it!


LGTM

- Anand Mazumdar


On Dec. 10, 2015, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40905/
> ---
> 
> (Updated Dec. 10, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4056
> https://issues.apache.org/jira/browse/MESOS-4056
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to RFC 2616 
> (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.7), 
> `MethodNotAllowed` response must include an 'Allow' header containing a list 
> of valid methods.
> 
> This is a libprocess change only.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 3f33b46527e07ed73621e07e9c5d40594e32f6a9 
> 
> Diff: https://reviews.apache.org/r/40905/diff/
> 
> 
> Testing
> ---
> 
> This a libprocess change only. The code does not compile without the 
> follow-up [mesos] patch https://reviews.apache.org/r/40913/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 41185: CMake: Updated LFLAG for dl library to defined label

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Updated LFLAG for dl library to defined label


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
9893d741cd7c611dc65eba76be03e06dac618132 

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


Testing
---


Thanks,

Diana Arroyo

Review Request 41188: Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages to HTTP endpoints

[Avinash sridharan]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Providing JSON bindings to so that mesos modules can expose DiscoveryInfo 
protobuf messages to HTTP endpoints


Diffs
-

  src/common/http.hpp 6a7a14ce8f1b9e0a85bc88bcf32bd80ea56bb7be 
  src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
  src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 

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


Testing
---


Thanks,

Avinash sridharan

Re: Review Request 41187: Adding labels field to Port information, for service discovery to associate arbitrary tags by applications to ports for a given task.

[Anand Mazumdar]

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


Can we also update the V1 protobufs since this would be external user-facing ?


include/mesos/mesos.proto (line 1568)


Why `Parameters` and not just `Labels` ?

Also why not name the field as `labels` as is being done in other places in 
the file ?


- Anand Mazumdar


On Dec. 10, 2015, 3:32 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41187/
> ---
> 
> (Updated Dec. 10, 2015, 3:32 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3962
> https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding labels field to Port information, for service discovery to associate 
> arbitrary tags by applications to ports for a given task.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
> 
> Diff: https://reviews.apache.org/r/41187/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40429: Report executor exit to framework schedulers.

[Vinod Kone]

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


Few things you need to do before this can get committed

1) Send an email to the mailing list announcing this change. I hope none of the 
existing schedulers crash when they receive this callback, but we want to make 
sure.
2) Update the upgrade and changelog docs.
3) Add a NOTE to the executorLost() (and even slaveLost()) method in the 
C++/Java/Python interfaces that this is not reliably delivered.

Another, easier option of course is to not do this change in the scheduler 
driver and live with the fact that this event is only delivered to HTTP 
schedulers and not driver based schedulers.


src/sched/sched.cpp (lines 219 - 223)


pull this up to #209.



src/sched/sched.cpp (lines 221 - 222)


flip the order of these two.



src/sched/sched.cpp (line 1053)


move this upto #1009.



src/sched/sched.cpp (line 1075)


remove "!"



src/tests/scheduler_event_call_tests.cpp (line 615)


Can you also add/update a test that uses scheduler driver to ensure that 
this callback is called Perhaps, 
MasterSlaveReconciliationTest.SlaveReregisterTerminatedExecutor ?

I would imagine you would need to update a lot more tests that don't expect 
this callback but now get this. It's likely that GMOCK only throws a warning 
but doesn't error out. You can see the warnings if you run the tests *without* 
verbose mode.


- Vinod Kone


On Nov. 18, 2015, 9:50 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> ---
> 
> (Updated Nov. 18, 2015, 9:50 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Report executor exit to framework schedulers. This is a MVP to start the work 
> of notifying scheduler on scheduler refresh.
> 
> Next step would be sending this message reliabily, and/or splitting 
> Event::FAILURE for slave failure and executor termination.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp a6faf92ff99cd79c3817684581862fecd1608048 
>   src/tests/scheduler_event_call_tests.cpp 
> 39f67a8243db8073d1c9c92c7aeb71854143131d 
> 
> Diff: https://reviews.apache.org/r/40429/diff/
> 
> 
> Testing
> ---
> 
> Modified test for SchedulerDriverEventTest.Failure, which verifies that 
> MockScheduler::executorLost is invoked.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>