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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

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

- Mesos ReviewBot


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

Review Request 40953: Avoid accepting hex float literals

[Cong Wang]

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

Review request for mesos, Benjamin Bannier, Ben Mahler, Jie Yu, and Michael 
Park.


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


Repository: mesos


Description
---

GCC, as an extension, accepts hex float literals even for C++, this is not 
allowed by standard C++. So we disallow it explicitly.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
2035653d578497e88ef465dc6cd49e2c0fc53366 
  3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
26fa7a4a06b36f9e7490e3274264f84b0369d412 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 40954: Fix comments in TaskHealthStatus message proto.

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Dec. 3, 2015, 9:44 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40954/
> ---
> 
> (Updated Dec. 3, 2015, 9:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix comments in TaskHealthStatus message proto.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 178d757a76f14829da6daab97ec678843717cf5a 
> 
> Diff: https://reviews.apache.org/r/40954/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39941: SIGPIPE is ignored in libprocess so stop handling it.

[Ben Mahler]

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

Ship it!


Looks great, thanks!


src/tests/main.cpp (line 19)


No longer needed?



src/tests/main.cpp (line 32)


No longer needed?


- Ben Mahler


On Nov. 4, 2015, 5:16 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39941/
> ---
> 
> (Updated Nov. 4, 2015, 5:16 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2079
> https://issues.apache.org/jira/browse/MESOS-2079
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SIGPIPE is ignored in libprocess so stop handling it.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp fb798670d9ac79c75ad39905614fbfe1ea25fba6 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
> 
> Diff: https://reviews.apache.org/r/39941/diff/
> 
> 
> Testing
> ---
> 
> make check.
> Run IOTest.Write in a loop on OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 40956: Reduced HealthCheckTest.CheckCommandTimeout test duration.

[Timothy Chen]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Reduced HealthCheckTest.CheckCommandTimeout test duration.


Diffs
-

  src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 40631: Move "using mesos::fetcher::FetcherInfo" into internal namespace in "fetcher.hpp"

[Klaus Ma]

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

(Updated Dec. 4, 2015, 3:11 p.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Michael Park.


Changes
---

Move `using` into .cpp files


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


Repository: mesos


Description
---

According to the google code style, the using should be used in internal 
namespace in header files. Grep the header files, only fetcher.hpp deserved a 
path.


> You may use a using-declaration anywhere in a .cc file (including in the 
> global namespace), and in functions, methods, classes, or within internal 
> namespaces in .h files.

>Do not use using-declarations in .h files except in explicitly marked 
>internal-only namespaces, because anything imported into a namespace in a .h 
>file becomes part of the public API exported by that file.

```
// OK in .cc files.
// Must be in a function, method, internal namespace, or
// class in .h files.
using ::foo::bar;
```


Diffs (updated)
-

  src/slave/containerizer/fetcher.hpp 78e7d14 
  src/slave/containerizer/fetcher.cpp 26df3d5 
  src/tests/fetcher_cache_tests.cpp fb0b3ba 
  src/tests/mesos.hpp 8d2d919 
  src/tests/mesos.cpp d42dab5 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 40501: Cleanup a leaked reference to a test process living in the stack.

[Ben Mahler]

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



src/tests/fetcher_tests.cpp (lines 285 - 289)


This makes spawning and termination asymmetric! :(

Please follow the approach done here:


https://github.com/apache/mesos/blob/0.26.0-rc3/3rdparty/libprocess/src/tests/http_tests.cpp#L64-L117


- Ben Mahler


On Nov. 19, 2015, 9:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40501/
> ---
> 
> (Updated Nov. 19, 2015, 9:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two of the fetcher tests will spawn a process which is stored in the stack 
> (i.e. local variable in the test).  `spawn` will store a pointer to the 
> process in libprocess's `ProcessManager`.  When the test finishes, the 
> process goes out of scope and is therefore lost.  However, the process is 
> **not** terminated.
> 
> Failing to terminate this process will lead to an infinite loop in 
> `~ProcessManager`, which is called in `process::finalize`.  In 
> `ProcessManager` 's destructor, we will loop and try to kill all processes.  
> The process spawned in the test will be running.  However, since the pointer 
> lives in the stack, the `ProcessManager` will be unable to find the process 
> and will thereby be stuck trying to kill a process it cannot find.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
> 
> Diff: https://reviews.apache.org/r/40501/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Additional testing:
> 
> Insert a `process::finalize` in `src/test/main.cpp`.  i.e.
> ```
>   // Replace `return RUN_ALL_TESTS();` with this:
>   int ret = RUN_ALL_TESTS();
>   process::finalize();
>   return ret;
> ```
> 
> Then `make check 
> GTEST_FILTER="*FetcherTest.OSNetUriTest*:*FetcherTest.OSNetUriSpaceTest*"`.
> The test program should not stall or segfault or abort in some weird way.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40730: Fixing MESOS-3552 by using CHECK_NEAR to avoid errors due to double precision errors.

[Bernd Mathiske]

> On Nov. 26, 2015, 6:43 a.m., Alexander Rukletsov wrote:
> > src/common/resources.cpp, line 35
> > 
> >
> > Is it fine to include files from "master/" in "common/*"?

Addressed in https://reviews.apache.org/r/40767/


- Bernd


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


On Nov. 25, 2015, 10:52 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40730/
> ---
> 
> (Updated Nov. 25, 2015, 10:52 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A version of this fix was already proposed by Mandeep Chadha (@mchadha) and 
> submitted to the reivew board https://reviews.apache.org/r/39056/ . So the 
> main credit should go to him for this fix.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp b4abf5405039d7d0a5028ccf034ad2e9623d064c 
> 
> Diff: https://reviews.apache.org/r/40730/diff/
> 
> 
> Testing
> ---
> 
> Added Mandeep's test case to GTEST https://reviews.apache.org/r/39056/ and 
> enabled an existing precision test (https://reviews.apache.org/r/40732/) to 
> test this fix.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40767: Take possible isNone() into account when comparing two Option CPU resource numbers.

[Bernd Mathiske]

> On Nov. 30, 2015, 9:10 a.m., Ben Mahler wrote:
> > src/common/resources.cpp, lines 963-965
> > 
> >
> > Do you know about Option's getOrElse? You can avoid these ternary 
> > expressions.
> > 
> > Also the ? needs a space in the second line here.
> 
> Bernd Mathiske wrote:
> Addressed in https://reviews.apache.org/r/40903/.

Good point! Thanks!


- Bernd


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


On Nov. 27, 2015, 5:44 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40767/
> ---
> 
> (Updated Nov. 27, 2015, 5:44 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Klaus Ma, 
> Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addendum to https://reviews.apache.org/r/40730. Not squeeky clean yet, but 
> good enough IMHO until we bring about MESOS-3997, which will change every 
> piece of code like this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5cf04ea93e7567107e6664ed56733b7b54dd321b 
> 
> Diff: https://reviews.apache.org/r/40767/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

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

[Alexander Rukletsov]

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

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.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
  src/slave/http.cpp eeebc79f59eb4deff12b1b8bdcd48b62d80f37fc 
  src/tests/executor_http_api_tests.cpp 
fe9df1f4d68babaf0960a3b689ffbe60704b8ad5 
  src/tests/scheduler_http_api_tests.cpp 
4f52309ff50a5b56cf20f2c5cfddd9c10b2b75d9 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

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

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 40909: Made unary c-tors for `Unauthorized` response explicit.

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On Dec. 3, 2015, 12:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40909/
> ---
> 
> (Updated Dec. 3, 2015, 12:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Ben Mahler, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
> 
> Diff: https://reviews.apache.org/r/40909/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Guangya Liu]

> On 十二月 3, 2015, 12:22 p.m., Guangya Liu wrote:
> > src/master/http.cpp, line 555
> > 
> >
> > I see that others using "Expecting a 'POST' request", this may be more 
> > clear
> > 
> > Also, is it a MUST to set {"POST"} as first parameter?
> > 
> > Ditto as following.

I should check 40905 first. I saw that you have updated the "MethodNotAllowed" 
there. Still want to know if we can update the message to "Expectint a 'POST' 
request, received a 'DELETE' reuqest"?


- Guangya


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


On 十二月 3, 2015, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40906/
> ---
> 
> (Updated 十二月 3, 2015, 11:23 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 40903: Ported approximated Option CPU resource number comparison to v1 and improved the check expression for this.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40767, 40903]

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

- Mesos ReviewBot


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 40906: Replaced `BadRequest` with `MethodNotAllowed` for all HTTP requests with unsupported methods.

[Guangya Liu]

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



src/master/http.cpp (line 555)


I see that others using "Expecting a 'POST' request", this may be more clear

Also, is it a MUST to set {"POST"} as first parameter?

Ditto as following.


- Guangya Liu


On 十二月 3, 2015, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40906/
> ---
> 
> (Updated 十二月 3, 2015, 11:23 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 40767: Take possible isNone() into account when comparing two Option CPU resource numbers.

[Bernd Mathiske]

> On Nov. 30, 2015, 9:10 a.m., Ben Mahler wrote:
> > src/common/resources.cpp, lines 963-965
> > 
> >
> > Do you know about Option's getOrElse? You can avoid these ternary 
> > expressions.
> > 
> > Also the ? needs a space in the second line here.

Addressed in https://reviews.apache.org/r/40903/.


- Bernd


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


On Nov. 27, 2015, 5:44 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40767/
> ---
> 
> (Updated Nov. 27, 2015, 5:44 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Klaus Ma, 
> Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addendum to https://reviews.apache.org/r/40730. Not squeeky clean yet, but 
> good enough IMHO until we bring about MESOS-3997, which will change every 
> piece of code like this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5cf04ea93e7567107e6664ed56733b7b54dd321b 
> 
> Diff: https://reviews.apache.org/r/40767/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

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

[Bernd Mathiske]

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

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 40445: Added linter for license headers in some file types.

[Benjamin Bannier]

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

(Updated Dec. 3, 2015, 9:29 a.m.)


Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.


Changes
---

Removed use of function-style `print` calls as per review comments.


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


Repository: mesos


Description
---

Review: https://reviews.apache.org/r/40445


Diffs (updated)
-

  support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 

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


Testing
---

Ran the a whole clean checkout through the linter with only one expected 
failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
license).


NOTE TO THE COMMITTER
-

Before committing this, it is probably a good idea to check the whole code base 
again and fix any new files which do not follow the current license style. The 
commits which originally fixed this were

* fa36917 (mesos),
* dc23756 (stout), and
* 3539b7a (libprocess).


Thanks,

Benjamin Bannier

Re: Review Request 40445: Added linter for license headers in some file types.

[Benjamin Bannier]

> On Dec. 2, 2015, 11:31 p.m., Michael Park wrote:
> > support/mesos-style.py, line 6
> > 
> >
> > Why the introduction of the `print` function? Can't we just use `print` 
> > and `.format`?
> 
> Benjamin Bannier wrote:
> This was some drive-by future proofing. 
> 
> In python-3.0 `print` can only be called as a function (`print("foo")`) 
> and the python-2.X syntax used around here (`print "foo"`) isn't available 
> anymore. The `print` function becomes available in python-2.6.0, but in the 
> shebang we pick whatever python interpreter is in `$PATH`. If we can drop 
> support for python-2.5 or earlier (not sure if it is still supported with 
> what is in here) we don't need to import it from `__future__`.
> 
> Michael Park wrote:
> Can we take this on as a separate ticket/task? We have a bunch of python 
> scripts, all of them seem to assume Python 2.

I filed MESOS-4054, and reverted to a `print "foo"` style.


- Benjamin


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


On Dec. 3, 2015, 9:29 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40445/
> ---
> 
> (Updated Dec. 3, 2015, 9:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/40445
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 
> 
> Diff: https://reviews.apache.org/r/40445/diff/
> 
> 
> Testing
> ---
> 
> Ran the a whole clean checkout through the linter with only one expected 
> failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
> license).
> 
> 
> NOTE TO THE COMMITTER
> -
> 
> Before committing this, it is probably a good idea to check the whole code 
> base again and fix any new files which do not follow the current license 
> style. The commits which originally fixed this were
> 
> * fa36917 (mesos),
> * dc23756 (stout), and
> * 3539b7a (libprocess).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 38878: Added test for the Subscribe->Subscribed workflow for the Executor HTTP API

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39569, 38874, 38875, 38876, 39297, 38877, 38878]

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

- Mesos ReviewBot


On Dec. 3, 2015, 6:48 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38878/
> ---
> 
> (Updated Dec. 3, 2015, 6:48 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a basic test to validate the implementation for 
> Subscribe->Subscribed workflow on the `api/v1/executor` endpoint on Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> fe9df1f4d68babaf0960a3b689ffbe60704b8ad5 
> 
> Diff: https://reviews.apache.org/r/38878/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add more agent recovery tests/executor reconnect tests in a 
> separate patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Timothy Chen]

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



src/slave/containerizer/docker.cpp (line 377)


Why are we adding in TaskInfo's environment variables for the executor?


- Timothy Chen


On Dec. 2, 2015, 10:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40884/
> ---
> 
> (Updated Dec. 2, 2015, 10:16 p.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
> -
> 
>   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 40445: Added linter for license headers in some file types.

[Benjamin Bannier]

> On Dec. 3, 2015, 3:51 a.m., Michael Park wrote:
> > support/mesos-style.py, lines 152-153
> > 
> >
> > Why not `print`?

Using `print "foo"`-style now (for local consistency).


- Benjamin


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


On Dec. 3, 2015, 9:29 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40445/
> ---
> 
> (Updated Dec. 3, 2015, 9:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/40445
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 
> 
> Diff: https://reviews.apache.org/r/40445/diff/
> 
> 
> Testing
> ---
> 
> Ran the a whole clean checkout through the linter with only one expected 
> failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
> license).
> 
> 
> NOTE TO THE COMMITTER
> -
> 
> Before committing this, it is probably a good idea to check the whole code 
> base again and fix any new files which do not follow the current license 
> style. The commits which originally fixed this were
> 
> * fa36917 (mesos),
> * dc23756 (stout), and
> * 3539b7a (libprocess).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40118: [1/7] Added 'principal' field to 'Resource.DiskInfo.Persistence'.

[Alexander Rukletsov]

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



include/mesos/mesos.proto (lines 629 - 631)


Let's leave a note why it's marked `optional`, which is inconsistent to 
`ReservationInfo.principal`. For symmetry, maybe it makes sense to add a `TODO` 
around `ReservationInfo.principal` that we may change it to optional as well


- Alexander Rukletsov


On Dec. 2, 2015, 9:42 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40118/
> ---
> 
> (Updated Dec. 2, 2015, 9:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3064
> https://issues.apache.org/jira/browse/MESOS-3064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'principal' field to 'Resource.DiskInfo.Persistence'.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 27971fea6935b82d7034397edaa7a37edb1f6f38 
>   include/mesos/v1/mesos.proto 9acefd55603a5a4f3f08a879a380ff927fd1e0dd 
> 
> Diff: https://reviews.apache.org/r/40118/diff/
> 
> 
> Testing
> ---
> 
> This is the first in a chain of 7 patches. `make check` was used to test 
> after all patches were applied.
> 
> Note that this chain of patches touches many of the same files as another 
> chain beginning with Review #39985 and ending with Review #39989, which is 
> currently in review as well. To avoid conflicts, the beginning of this chain 
> begins on top of Review #39989.
> 
> One additional patch with documentation is forthcoming.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40445: Added linter for license headers in some file types.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39590, 39591, 39592, 40445]

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

- Mesos ReviewBot


On Dec. 3, 2015, 9:29 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40445/
> ---
> 
> (Updated Dec. 3, 2015, 9:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/40445
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 
> 
> Diff: https://reviews.apache.org/r/40445/diff/
> 
> 
> Testing
> ---
> 
> Ran the a whole clean checkout through the linter with only one expected 
> failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
> license).
> 
> 
> NOTE TO THE COMMITTER
> -
> 
> Before committing this, it is probably a good idea to check the whole code 
> base again and fix any new files which do not follow the current license 
> style. The commits which originally fixed this were
> 
> * fa36917 (mesos),
> * dc23756 (stout), and
> * 3539b7a (libprocess).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40435: Fixed pointer alignment error in IP::create().

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Dec. 2, 2015, 10:28 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40435/
> ---
> 
> (Updated Dec. 2, 2015, 10:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Joris Van Remoortere, 
> Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3939
> https://issues.apache.org/jira/browse/MESOS-3939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous code took the address of a "struct sockaddr", and then cast the
> resulting pointer to "struct sockaddr_storage *". The alignment requirements 
> for
> "struct sockaddr_storage *" are more strict than for "struct sockaddr *", and
> hence this code produced undefined behavior per ubsan in GCC 5.2.
> 
> Along the way, tweak the code to use reinterpret_cast rather than a C-style
> cast, and not to unnecessarily cast-away constness.
> 
> MESOS-3939.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 3e506e1e94ee32e3a19870e1fd72d9968c5f8e67 
> 
> Diff: https://reviews.apache.org/r/40435/diff/
> 
> 
> Testing
> ---
> 
> "make check" on Linux/AMD64 + GCC 5.2 + ubsan; without fix, ubsan reports an 
> error. With fix, ubsan does not report (this) error.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40435: Fixed pointer alignment error in IP::create().

[Michael Park]

> On Dec. 3, 2015, 4:26 a.m., Michael Park wrote:
> > Could you also create a clean-up patch to get rid of the unnecessary 
> > `struct` disambiguators in this file?
> 
> Neil Conway wrote:
> I kinda like leaving "struct" as-is, because we're interfacing with 
> C-style APIs where "struct" is syntactically significant -- when reading C 
> code, I expect to see "struct sockaddr", not just "sockaddr". What do you 
> think?

Is your perspective that the definitions of these structs are in C headers and 
therefore should be used as if we were writing C?
My perspective is that it may be a C header but it still gets compiled __as__ 
C++ with a C++ compiler.
I feel like if we were to agree with your argument, we could carry it all the 
way to: "it makes more sense for us to keep the C-style cast rather than using 
C++'s `reinterpret_cast` since we're dealing with C API"?


- Michael


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


On Dec. 3, 2015, 6:28 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40435/
> ---
> 
> (Updated Dec. 3, 2015, 6:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Joris Van Remoortere, 
> Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3939
> https://issues.apache.org/jira/browse/MESOS-3939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous code took the address of a "struct sockaddr", and then cast the
> resulting pointer to "struct sockaddr_storage *". The alignment requirements 
> for
> "struct sockaddr_storage *" are more strict than for "struct sockaddr *", and
> hence this code produced undefined behavior per ubsan in GCC 5.2.
> 
> Along the way, tweak the code to use reinterpret_cast rather than a C-style
> cast, and not to unnecessarily cast-away constness.
> 
> MESOS-3939.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 3e506e1e94ee32e3a19870e1fd72d9968c5f8e67 
> 
> Diff: https://reviews.apache.org/r/40435/diff/
> 
> 
> Testing
> ---
> 
> "make check" on Linux/AMD64 + GCC 5.2 + ubsan; without fix, ubsan reports an 
> error. With fix, ubsan does not report (this) error.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 40954: Fix comments in TaskHealthStatus message proto.

[Timothy Chen]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Fix comments in TaskHealthStatus message proto.


Diffs
-

  src/messages/messages.proto 178d757a76f14829da6daab97ec678843717cf5a 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 40909: Made unary c-tors for `Unauthorized` response explicit.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 3, 2015, 11:42 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40909/
> ---
> 
> (Updated Dec. 3, 2015, 11:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Ben Mahler, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
> 
> Diff: https://reviews.apache.org/r/40909/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Guangya Liu]

> On Dec. 3, 2015, 12:24 p.m., Guangya Liu wrote:
> > src/master/http.cpp, line 385
> > 
> >
> > What about update to 
> > 
> > Expecting a 'POST' request, received a '" + request.method + "' request"
> 
> Alexander Rukletsov wrote:
> Do you think it adds extra information?
> 
> I think the message is consice and fits in one line. Also technically, 
> 'POST' is not a request, it's a method (or http verb).

I think that the POST, DELETE, GET, PUT can all be treated as HTTP request ;-)

Using "Expecting a 'POST' request, received a '" + request.method + "' 
request"" does not add extra information, but just make the message clear and 
official.


- Guangya


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


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

Re: Review Request 40551: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

> On Dec. 3, 2015, 1:40 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1026
> > 
> >
> > roleConsumedResources = roleConsumedResources.nonRevocable();

As you may know, `.nonRevocable()` filter landed rather recently, far later 
than this patch has been proposed. Right after, [a follow-up 
patch](https://reviews.apache.org/r/40821/) has been published for review and 
is part of the current chain. Applying `.nonRevocable()` filter required a more 
careful approach, otherwise we may blindly apply it to resources that do not 
contain revocable resources by default. Please review [that 
patch](https://reviews.apache.org/r/40821/) instead. Same for similar comments 
below.


> On Dec. 3, 2015, 1:40 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1054
> > 
> >
> > user nonRevocable()?

See above.


> On Dec. 3, 2015, 1:40 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1116
> > 
> >
> > use nonRevocable() directly?

See above.


- Alexander


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


On Nov. 30, 2015, 3:24 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40551/
> ---
> 
> (Updated Nov. 30, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota is satisfied in a separate loop over agents. Running total is 
> maintained as an exit criterion for the WDRF allocation stage.
> 
> Precursory version: https://reviews.apache.org/r/39401/
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40551/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40776: Fixed test flakiness in FetcherTest and FetcherCacheTest.

[Benjamin Bannier]

> On Dec. 3, 2015, 1:05 p.m., Bernd Mathiske wrote:
> > src/tests/fetcher_tests.cpp, line 543
> > 
> >
> > Since both ExtractNotExecutable and ExtractTar dio the same now one of 
> > these tests is redundant and can be deleted. However, we are then no longer 
> > testing if the extension .tgz is recognized. How about changing one of the 
> > tests to using that extension (with a comment why) yet using 
> > non-compressing packaging. Unpacking will still work.

Note that at least the current `os::tar` and since 2013-03-13 *always did 
compress with gzip* regardless of the extension.

To still touch the path matching the `.tar.gz` extension (sic) I restored the 
old filename in `FetcherTest.ExtractNotExecutable`, but still created an 
uncompressed file to work around the issue here.


- Benjamin


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


On Dec. 3, 2015, 2:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40776/
> ---
> 
> (Updated Dec. 3, 2015, 2:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3579
> https://issues.apache.org/jira/browse/MESOS-3579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GNU tar-1.28 and earlier can relatively randomly misidentify
> compressed files as uncompressed ones, and will then fail when
> unpacking. Avoid compressed tar files in some tests where we know we
> directly call the tar system command (in mesos-fetcher in this case).
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp ac5ad6acfcf60b7094ec45f64bed164b4dc3f8bd 
>   src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a 
> 
> Diff: https://reviews.apache.org/r/40776/diff/
> 
> 
> Testing
> ---
> 
> `make check` and verified test does not fail in at least 17500 iterations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40776: Fixed test flakiness in FetcherTest and FetcherCacheTest.

[Benjamin Bannier]

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

(Updated Dec. 3, 2015, 2:33 p.m.)


Review request for mesos, Bernd Mathiske and Till Toenshoff.


Changes
---

Added some variation in filename extension.

Also switched from `os::system` to `os::shell` so we can suppress benign stderr 
output from `tar` (`tar: Removing leading '/' from member names`).


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


Repository: mesos


Description
---

GNU tar-1.28 and earlier can relatively randomly misidentify
compressed files as uncompressed ones, and will then fail when
unpacking. Avoid compressed tar files in some tests where we know we
directly call the tar system command (in mesos-fetcher in this case).


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp ac5ad6acfcf60b7094ec45f64bed164b4dc3f8bd 
  src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a 

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


Testing
---

`make check` and verified test does not fail in at least 17500 iterations.


Thanks,

Benjamin Bannier

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

[Alexander Rukletsov]

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


Changes
---

Split into [libprocess] and [mesos] patches.


Summary (updated)
-

[libprocess]: Made `MethodNotAllowed` response compliant to RFC 2616.


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


Repository: mesos


Description (updated)
---

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

  3rdparty/libprocess/include/process/http.hpp 
c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 

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


Testing (updated)
---

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

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40905]

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

Error:
 2015-12-03 13:57:45 URL:https://reviews.apache.org/r/40905/diff/raw/ 
[3346/3346] -> "40905.patch" [1]
Successfully applied: Made `MethodNotAllowed` response compliant to RFC 2616.

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.


Review: https://reviews.apache.org/r/40905
Checking 5 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  src/master/http.cpp
  src/slave/http.cpp
  src/tests/executor_http_api_tests.cpp
  src/tests/scheduler_http_api_tests.cpp
libprocess:
  3rdparty/libprocess/include/process/http.hpp
Failed to commit patch

- Mesos ReviewBot


On Dec. 3, 2015, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40906/
> ---
> 
> (Updated Dec. 3, 2015, 11:23 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
> 
>

Review Request 40910: [stout]: Made license-headers doxygen-compatible.

[Benjamin Bannier]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

This commit adjusts license headers of C++ source files which were
added since dc23756.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
40629f4623891162d830e7addf341c2ebd657e8b 

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


Testing
---

`make check` under OS X 10.10.5 succeeded modulo MESOS-4045.


Thanks,

Benjamin Bannier

Re: Review Request 40551: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Guangya Liu]

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



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


roleConsumedResources = roleConsumedResources.nonRevocable();



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


user nonRevocable()?



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


I searched the mesos code and found that there is no reference for WDRF, 
shall we clarify this as weight DRF for one time?



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


use nonRevocable() directly?


- Guangya Liu


On 十一月 30, 2015, 3:24 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40551/
> ---
> 
> (Updated 十一月 30, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota is satisfied in a separate loop over agents. Running total is 
> maintained as an exit criterion for the WDRF allocation stage.
> 
> Precursory version: https://reviews.apache.org/r/39401/
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40551/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40776: Fixed test flakiness in FetcherTest and FetcherCacheTest.

[Bernd Mathiske]

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



src/tests/fetcher_tests.cpp (line 543)


Since both ExtractNotExecutable and ExtractTar dio the same now one of 
these tests is redundant and can be deleted. However, we are then no longer 
testing if the extension .tgz is recognized. How about changing one of the 
tests to using that extension (with a comment why) yet using non-compressing 
packaging. Unpacking will still work.


- Bernd Mathiske


On Nov. 28, 2015, 3:15 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40776/
> ---
> 
> (Updated Nov. 28, 2015, 3:15 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3579
> https://issues.apache.org/jira/browse/MESOS-3579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GNU tar-1.28 and earlier can relatively randomly misidentify
> compressed files as uncompressed ones, and will then fail when
> unpacking. Avoid compressed tar files in some tests where we know we
> directly call the tar system command (in mesos-fetcher in this case).
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp 7fa5e6a572a40dc1a531001fcee2a377d1c99309 
>   src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a 
> 
> Diff: https://reviews.apache.org/r/40776/diff/
> 
> 
> Testing
> ---
> 
> `make check` and verified test does not fail in at least 17500 iterations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Alexander Rukletsov]

> On Dec. 3, 2015, 12:24 p.m., Guangya Liu wrote:
> > src/master/http.cpp, line 385
> > 
> >
> > What about update to 
> > 
> > Expecting a 'POST' request, received a '" + request.method + "' request"

Do you think it adds extra information?

I think the message is consice and fits in one line. Also technically, 'POST' 
is not a request, it's a method (or http verb).


- Alexander


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


On Dec. 3, 2015, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40905/
> ---
> 
> (Updated Dec. 3, 2015, 11:23 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.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/slave/http.cpp eeebc79f59eb4deff12b1b8bdcd48b62d80f37fc 
>   src/tests/executor_http_api_tests.cpp 
> fe9df1f4d68babaf0960a3b689ffbe60704b8ad5 
>   src/tests/scheduler_http_api_tests.cpp 
> 4f52309ff50a5b56cf20f2c5cfddd9c10b2b75d9 
> 
> Diff: https://reviews.apache.org/r/40905/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40557: Add documentation about using terminate/wait on Processes when deallocating them.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Nov. 20, 2015, 11:52 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40557/
> ---
> 
> (Updated Nov. 20, 2015, 11:52 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on a discussion here: https://reviews.apache.org/r/40501/#review107290
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 
> 
> Diff: https://reviews.apache.org/r/40557/diff/
> 
> 
> Testing
> ---
> 
> Checked README.md rendering on GitHub.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

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 updates `MethodNotAllowed` c-tor invocations in Mesos codebase.


Diffs
-

  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
  src/slave/http.cpp eeebc79f59eb4deff12b1b8bdcd48b62d80f37fc 
  src/tests/executor_http_api_tests.cpp 
fe9df1f4d68babaf0960a3b689ffbe60704b8ad5 
  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 40776: Fixed test flakiness in FetcherTest and FetcherCacheTest.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Dec. 3, 2015, 6:33 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40776/
> ---
> 
> (Updated Dec. 3, 2015, 6:33 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3579
> https://issues.apache.org/jira/browse/MESOS-3579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GNU tar-1.28 and earlier can relatively randomly misidentify
> compressed files as uncompressed ones, and will then fail when
> unpacking. Avoid compressed tar files in some tests where we know we
> directly call the tar system command (in mesos-fetcher in this case).
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp ac5ad6acfcf60b7094ec45f64bed164b4dc3f8bd 
>   src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a 
> 
> Diff: https://reviews.apache.org/r/40776/diff/
> 
> 
> Testing
> ---
> 
> `make check` and verified test does not fail in at least 17500 iterations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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. 3, 2015, 2:35 p.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 40880: Fix flaky MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery test.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Dec. 2, 2015, 12:44 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40880/
> ---
> 
> (Updated Dec. 2, 2015, 12:44 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4047
> https://issues.apache.org/jira/browse/MESOS-4047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` has very similar 
> logic for restarting an agent, re-registering the executor, and even getting 
> `ResourceStatistics`.  But 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` is stable.
> 
> This patch updates the flaky test's wait-for-agent-recovery logic to match 
> the stable test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> e18b971c4df26c9b9c103ca73bdad4fd400d6c02 
> 
> Diff: https://reviews.apache.org/r/40880/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 14:
> `make check`
> `sudo bin/mesos-tests.sh 
> --gtest_filter="*MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> ^ Ran the above until satisfied.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 40911: Made license-headers doxygen-compatible.

[Benjamin Bannier]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

This commit adjusts license headers of C++ source files added since
fa36917.


Diffs
-

  src/linux/routing/handle.cpp 09589e9616f115df5e55b7fc813169cfdbbec8be 
  src/tests/containerizer/launcher.cpp 22b44baf6509d8cda780184721ef05320f40b10d 
  src/tests/master_quota_tests.cpp c9d78cffc30539ce2d29360b231fcaf9e5b592ea 
  src/tests/persistent_volume_endpoints_tests.cpp 
ac46806a712113170b349a969a9f1132723116f0 

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


Testing
---

`make check` under OS X 10.10.5 succeeded modulo MESOS-4045.


Thanks,

Benjamin Bannier

Re: Review Request 40445: Added linter for license headers in some file types.

[Benjamin Bannier]

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

(Updated Dec. 3, 2015, 1:06 p.m.)


Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.


Changes
---

Used `str.format` as suggested out-of-band by mcypark.

Also fixed a faulty regex to now actually display `cpplint` issues.


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


Repository: mesos


Description (updated)
---

This adds a style check for license headers in C, C++, and Proto files.


Diffs (updated)
-

  support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 

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


Testing (updated)
---

Ran the a whole clean checkout through the linter with only one expected 
failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
license).

`make check` under OS X 10.10.5 succeeded modulo MESOS-4045.


Thanks,

Benjamin Bannier

Re: Review Request 40909: Made unary c-tors for `Unauthorized` response explicit.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40909]

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

- Mesos ReviewBot


On Dec. 3, 2015, 11:42 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40909/
> ---
> 
> (Updated Dec. 3, 2015, 11:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Ben Mahler, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> c7d19a2086a8891d0198dce1c4d1c92bd99ac6e3 
> 
> Diff: https://reviews.apache.org/r/40909/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Alexander Rukletsov]

> On Dec. 3, 2015, 12:22 p.m., Guangya Liu wrote:
> > src/master/http.cpp, line 555
> > 
> >
> > I see that others using "Expecting a 'POST' request", this may be more 
> > clear
> > 
> > Also, is it a MUST to set {"POST"} as first parameter?
> > 
> > Ditto as following.
> 
> Guangya Liu wrote:
> I should check 40905 first. I saw that you have updated the 
> "MethodNotAllowed" there. Still want to know if we can update the message to 
> "Expectint a 'POST' request, received a 'DELETE' reuqest"?

Replied in r/40905/.


- Alexander


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


On Dec. 3, 2015, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40906/
> ---
> 
> (Updated Dec. 3, 2015, 11:23 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 40849: Fix flaky MemoryPressureMesosTests

[Bernd Mathiske]

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

Ship it!



src/tests/containerizer/memory_pressure_tests.cpp (line 140)


Consider moving this comment to line 143, before "break;". It makes more 
sense to me to talk about what you are not doing once you have actually reached 
the statement where you might do something.


- Bernd Mathiske


On Dec. 2, 2015, 11:25 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40849/
> ---
> 
> (Updated Dec. 2, 2015, 11:25 a.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 40672: Fixed flakey test: MasterMaintenanceTest.InverseOffersFilters.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Nov. 24, 2015, 9:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40672/
> ---
> 
> (Updated Nov. 24, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3916
> https://issues.apache.org/jira/browse/MESOS-3916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3916.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> ade05f932020013ced19c1573be756a029396fac 
> 
> Diff: https://reviews.apache.org/r/40672/diff/
> 
> 
> Testing
> ---
> 
> (1) mesos-tests --gtest_repeat=2000 on OSX
> (2) My Ubuntu VM seems to be in a weird state: this fixes the observed 
> flakiness, but there seem to be other weird networking issues that cause 
> messaging timeouts that seem to be unrelated. Till, Jan: would you guys mind 
> repro'ing that this fixes the flakiness for you?
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40435: Fixed pointer alignment error in IP::create().

[Neil Conway]

> On Dec. 3, 2015, 4:26 a.m., Michael Park wrote:
> > Could you also create a clean-up patch to get rid of the unnecessary 
> > `struct` disambiguators in this file?
> 
> Neil Conway wrote:
> I kinda like leaving "struct" as-is, because we're interfacing with 
> C-style APIs where "struct" is syntactically significant -- when reading C 
> code, I expect to see "struct sockaddr", not just "sockaddr". What do you 
> think?
> 
> Michael Park wrote:
> Is your perspective that the definitions of these structs are in C 
> headers and therefore should be used as if we were writing C?
> My perspective is that it may be a C header but it still gets compiled 
> __as__ C++ with a C++ compiler.
> I feel like if we were to agree with your argument, we could carry it all 
> the way to: "it makes more sense for us to keep the C-style cast rather than 
> using C++'s `reinterpret_cast` since we're dealing with C API"?

For situations in which there is a functional difference between C-style code 
and C++-style code (e.g., reinterpret_cast), I'd vote for doing the 
best/safest/optimal thing. For things like `struct` naming, since it is just a 
cosmetic difference, I'd probably lean towards matching what you see in 
idiomatic code that uses the underlying API -- which is typically `struct 
sockaddr` and so on, at least in my experience. I don't feel strongly about it 
though -- happy to send a RR if you'd like.


- Neil


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


On Dec. 3, 2015, 6:28 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40435/
> ---
> 
> (Updated Dec. 3, 2015, 6:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Joris Van Remoortere, 
> Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3939
> https://issues.apache.org/jira/browse/MESOS-3939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous code took the address of a "struct sockaddr", and then cast the
> resulting pointer to "struct sockaddr_storage *". The alignment requirements 
> for
> "struct sockaddr_storage *" are more strict than for "struct sockaddr *", and
> hence this code produced undefined behavior per ubsan in GCC 5.2.
> 
> Along the way, tweak the code to use reinterpret_cast rather than a C-style
> cast, and not to unnecessarily cast-away constness.
> 
> MESOS-3939.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 3e506e1e94ee32e3a19870e1fd72d9968c5f8e67 
> 
> Diff: https://reviews.apache.org/r/40435/diff/
> 
> 
> Testing
> ---
> 
> "make check" on Linux/AMD64 + GCC 5.2 + ubsan; without fix, ubsan reports an 
> error. With fix, ubsan does not report (this) error.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40873: RegistryClientTests: Created separate server to serve blobs.

[Jojy Varghese]

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

(Updated Dec. 3, 2015, 5:20 p.m.)


Review request for mesos and Timothy Chen.


Summary (updated)
-

RegistryClientTests: Created separate server to serve blobs.


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


Repository: mesos


Description (updated)
---

By having a separate blob server, it simulates the real world more closely. It 
also allows the test server to be in accept mode early.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
c63bf53fee40ef12536a16e11f4d5224c4e4278e 

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


Testing
---

make check (600 count).


Thanks,

Jojy Varghese

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/#review108845
---


LGTM, Just one minor suggestion + a query regarding why do we need the 
additional constructor without a `Response` body.


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 ?



3rdparty/libprocess/include/process/http.hpp (line 588)


s/std::vector/std::initializer_list

An initializer list is better suited for this constructor since we hardly 
need any `std::vector` operations. I am assuming `strings::join(...)` plays 
well with it too ?


- Anand Mazumdar


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

Re: Review Request 40332: Quota: Implemented recovery in hierarchical allocator.

[Alexander Rukletsov]

> On Nov. 23, 2015, 5:54 p.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 413-418
> > 
> >
> > Rather than doing the math here  (which I believe we're missing 
> > corresponding entries for in `removeSlave()`?) why not test whether 
> > `slaves.size() >= expectedAgentCount()` ?
> > 
> > I think it is more clear, and less error prone.
> > 
> > Let's log when this condition is met?
> 
> Alexander Rukletsov wrote:
> Let me address your concerns one by one : )
> 
> 1. I don't think we need corresponding entries in `removeSlave()` because 
> we do not track what agents reregistered, but rather a total number. If an 
> agent reregistered and then got lost, this should not influence the 
> allocation pause.
> 2. For the reason described in 1., `slaves.size() >= 
> expectedAgentCount()` seems not a good fit, because it counts removed agents 
> in.
> 3. Logging is a good idea.

If we have just a number for recovered agents, we cannot distinguish between 
“old” agents from the registry and “new” ones joined after recovery. Because we 
do not persist enough information to base logical decisions on, any accounting 
algorithm here will be crude. Hence we decided to pick Joris' suggestion. Doing 
`slaves.size() >= expectedAgents` at least expresses the intention.


- Alexander


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


On Nov. 30, 2015, 3:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40332/
> ---
> 
> (Updated Nov. 30, 2015, 3:32 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3981
> https://issues.apache.org/jira/browse/MESOS-3981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1cd8d16661568010901e74705375e7719cdfb8a0 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40332/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40880: Fix flaky MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40849, 40880]

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

- Mesos ReviewBot


On Dec. 3, 2015, 7:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40880/
> ---
> 
> (Updated Dec. 3, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4047
> https://issues.apache.org/jira/browse/MESOS-4047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` has very similar 
> logic for restarting an agent, re-registering the executor, and even getting 
> `ResourceStatistics`.  But 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` is stable.
> 
> This patch updates the flaky test's wait-for-agent-recovery logic to match 
> the stable test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> e18b971c4df26c9b9c103ca73bdad4fd400d6c02 
> 
> Diff: https://reviews.apache.org/r/40880/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 14:
> `make check`
> `sudo bin/mesos-tests.sh 
> --gtest_filter="*MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> ^ Ran the above until satisfied.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Alexander Rukletsov]

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

(Updated Dec. 3, 2015, 8:25 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Joris' comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
a8f65b72488505afd3677ecdeb9b821ea27af7e0 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40795: Quota: Properly initialized the sorter for quota'ed roles in the allocator.

[Alexander Rukletsov]

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

(Updated Dec. 3, 2015, 7:30 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
Joseph Wu, and Qian Zhang.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40551: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Joris Van Remoortere]

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

Ship it!


Fixed in-line.


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


new line.



src/master/allocator/mesos/hierarchical.cpp (lines 1100 - 1101)


Let's fold these together.



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


`const`?



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


Let's clarify that, in this case, we continue rather than "stop".



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


`are allocated or accounted for`


- Joris Van Remoortere


On Nov. 30, 2015, 3:24 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40551/
> ---
> 
> (Updated Nov. 30, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota is satisfied in a separate loop over agents. Running total is 
> maintained as an exit criterion for the WDRF allocation stage.
> 
> Precursory version: https://reviews.apache.org/r/39401/
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40551/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40795: Quota: Properly initialized the sorter for quota'ed roles in the allocator.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 3, 2015, 7:30 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40795/
> ---
> 
> (Updated Dec. 3, 2015, 7:30 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3981
> https://issues.apache.org/jira/browse/MESOS-3981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40795/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40849: Fix flaky MemoryPressureMesosTests

[Joseph Wu]

> On Dec. 3, 2015, 6:24 a.m., Bernd Mathiske wrote:
> > src/tests/containerizer/memory_pressure_tests.cpp, line 140
> > 
> >
> > Consider moving this comment to line 143, before "break;". It makes 
> > more sense to me to talk about what you are not doing once you have 
> > actually reached the statement where you might do something.

Done!  
Also improved the wording a bit :)


- Joseph


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


On Dec. 3, 2015, 11:01 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40849/
> ---
> 
> (Updated Dec. 3, 2015, 11:01 a.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 40849: Fix flaky MemoryPressureMesosTests

[Joseph Wu]

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

(Updated Dec. 3, 2015, 11:01 a.m.)


Review request for mesos, Bernd Mathiske, Greg Mann, Artem Harutyunyan, Jan 
Schlicht, and Till Toenshoff.


Changes
---

Moved and reworded a comment.


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

  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 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Joris Van Remoortere]

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

Ship it!



src/master/allocator/mesos/hierarchical.cpp (lines 1086 - 1097)


Let's simplify this to a basic for loop with a condition.
We can add an abstraction to stout to do something like filter later.


- Joris Van Remoortere


On Nov. 30, 2015, 5:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40797/
> ---
> 
> (Updated Nov. 30, 2015, 5:14 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40797/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40332: Quota: Implemented recovery in hierarchical allocator.

[Alexander Rukletsov]

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

(Updated Dec. 3, 2015, 8:07 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
Joseph Wu, and Qian Zhang.


Changes
---

Update recovery algorithm


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1cd8d16661568010901e74705375e7719cdfb8a0 
  src/master/allocator/mesos/hierarchical.cpp 
a8f65b72488505afd3677ecdeb9b821ea27af7e0 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40559: Added a wait() function to Subprocess.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 20, 2015, 9:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40559/
> ---
> 
> (Updated Nov. 20, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a wait() function to Subprocess. This is similar to 
> `Popen.communicate(None)` in python. See the example code below:
> 
> ```
> Try s = subprocess(...);
> if (s.isError()) {
>   ...
> }
> 
> return s.get().wait()
>   .then([](const Subprocess::Result& result) {
> if (result.status.isNone()) { ... }
> if (result.status.get() != 0) { ... }
> 
> handle(result.out.get());
> handle(result.err.get());
>   });
> ```
> 
> Relevant review: https://reviews.apache.org/r/37336/
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> f17816e813d5efce1d3bb1ff1e850eeda3ba 
>   3rdparty/libprocess/src/subprocess.cpp 
> e51f024e89594d84f1dfb7ee6f2e1d8fb33b4a08 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/40559/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Vinod Kone]

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

Ship it!


i'm assuming there are already tests that verify this change?


src/hdfs/hdfs.cpp (line 103)


maybe prepend a comma here?



src/hdfs/hdfs.cpp (line 106)


ditto. prepend a comma here?


- Vinod Kone


On Dec. 1, 2015, 1:21 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40806/
> ---
> 
> (Updated Dec. 1, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made HDFS::exists asynchronous.
> 
> This also fixed a bug in the orignal code: the original code never returns 
> false.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
>   src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
>   src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
> 
> Diff: https://reviews.apache.org/r/40806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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

[Greg Mann]

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

Ship it!


Ship It!

- Greg Mann


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 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Dec. 2, 2015, 10:41 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Dec. 2, 2015, 10:41 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
>   src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Alexander Rukletsov]

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

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


Review request for mesos and Joris Van Remoortere.


Changes
---

Removed unnecessary include.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
a8f65b72488505afd3677ecdeb9b821ea27af7e0 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

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

[Joseph Wu]

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


Looks good.  Just a few nits.


include/mesos/resources.hpp (line 156)


You should surround `!isPersistentVolume` in backticks.  We use backticks 
for code inside comments.

Same for all copy-pasted comments.



include/mesos/resources.hpp (line 235)


Note: You have an extra "s" (typo) at the end of `!persistentVolumes`.



src/tests/resources_tests.cpp (lines 1794 - 1798)


Can you remove some of the newlines here?

Also comment that PersistentVolumes are stateful resources.


- Joseph Wu


On Dec. 2, 2015, 8:35 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40529/
> ---
> 
> (Updated Dec. 2, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Bugs: MESOS-3955
> https://issues.apache.org/jira/browse/MESOS-3955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get stateless resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/40529/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Joseph Wu]

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


It would be a good idea to start writing tests for each of these changes.  You 
should probably do so in a new `master__test.cpp` file.

We have tests for master flags like this:
https://github.com/apache/mesos/blob/master/src/tests/master_tests.cpp#L1092-L1108

- Joseph Wu


On Dec. 2, 2015, 8:35 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40339/
> ---
> 
> (Updated Dec. 2, 2015, 8:35 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
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1cd8d16661568010901e74705375e7719cdfb8a0 
>   src/master/allocator/mesos/hierarchical.cpp 
> a8f65b72488505afd3677ecdeb9b821ea27af7e0 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> a45b3dd0a8237fdd080536cb9ab600e71ad939d3 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_quota_tests.cpp c9d78cffc30539ce2d29360b231fcaf9e5b592ea 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> ac46806a712113170b349a969a9f1132723116f0 
>   src/tests/reservation_endpoints_tests.cpp 
> 142b7c61e08804417df639551926bf7fe3da9680 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   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 40332: Quota: Implemented recovery in hierarchical allocator.

[Alexander Rukletsov]

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

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


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
Joseph Wu, and Qian Zhang.


Changes
---

Skipped recovery if there are no agents.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1cd8d16661568010901e74705375e7719cdfb8a0 
  src/master/allocator/mesos/hierarchical.cpp 
a8f65b72488505afd3677ecdeb9b821ea27af7e0 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40559: Added a wait() function to Subprocess.

[Jie Yu]

> On Dec. 3, 2015, 10:34 p.m., Vinod Kone wrote:
> > Ship It!

There's some controversy regarding this patch. I'll move this logic to hdfs.cpp 
as a helper. Discard this patch for now. Will send a new one shortly.


- Jie


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


On Nov. 20, 2015, 9:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40559/
> ---
> 
> (Updated Nov. 20, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a wait() function to Subprocess. This is similar to 
> `Popen.communicate(None)` in python. See the example code below:
> 
> ```
> Try s = subprocess(...);
> if (s.isError()) {
>   ...
> }
> 
> return s.get().wait()
>   .then([](const Subprocess::Result& result) {
> if (result.status.isNone()) { ... }
> if (result.status.get() != 0) { ... }
> 
> handle(result.out.get());
> handle(result.err.get());
>   });
> ```
> 
> Relevant review: https://reviews.apache.org/r/37336/
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> f17816e813d5efce1d3bb1ff1e850eeda3ba 
>   3rdparty/libprocess/src/subprocess.cpp 
> e51f024e89594d84f1dfb7ee6f2e1d8fb33b4a08 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/40559/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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

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

Review Request 40941: Added a helper for HDFS client to shell out commands.

[Jie Yu]

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

Review request for mesos, Ben Mahler and Vinod Kone.


Repository: mesos


Description
---

Added a helper for HDFS client to shell out commands.

Code is copied from https://reviews.apache.org/r/40559/.


Diffs
-

  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

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

(Updated Dec. 4, 2015, 12:09 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description
---

Made HDFS::exists asynchronous.

This also fixed a bug in the orignal code: the original code never returns 
false.


Diffs (updated)
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

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

(Updated Dec. 4, 2015, 12:20 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


Changes
---

Address review comments.


Repository: mesos


Description
---

Made HDFS::exists asynchronous.

This also fixed a bug in the orignal code: the original code never returns 
false.


Diffs (updated)
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40935: Fixed flakiness in MasterMaintenanceTest.InverseOffersFilters.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40935]

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 40118: [1/7] Added 'principal' field to 'Resource.DiskInfo.Persistence'.

[Greg Mann]

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

(Updated Dec. 3, 2015, 11:37 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Added comments.


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


Repository: mesos


Description
---

Added 'principal' field to 'Resource.DiskInfo.Persistence'.


Diffs (updated)
-

  include/mesos/mesos.proto 671365eb541418eeb60f029f8ef1d73abf6ef61d 
  include/mesos/v1/mesos.proto 9acefd55603a5a4f3f08a879a380ff927fd1e0dd 

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


Testing
---

This is the first in a chain of 7 patches. `make check` was used to test after 
all patches were applied.

Note that this chain of patches touches many of the same files as another chain 
beginning with Review #39985 and ending with Review #39989, which is currently 
in review as well. To avoid conflicts, the beginning of this chain begins on 
top of Review #39989.

One additional patch with documentation is forthcoming.


Thanks,

Greg Mann

Review Request 40935: Fixed flakiness in MasterMaintenanceTest.InverseOffersFilters.

[Neil Conway]

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

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


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


Repository: mesos


Description
---

There were two problems:

(1) After launching two tasks, we assumed that we would see TASK_RUNNING updates
for the tasks in the same order they were launched. This is not guaranteed,
so adjust the test to handle TASK_RUNNING updates in the order they are
received.

(2) The test used this pattern:

Mesos m;
Call c;

m.send(c);
Clock::settle();
// Trigger a new batch allocation that reflects the call
Clock::advance();

However, this is actually unsafe (see MESOS-3760): the send() call might not
have reached the master by the time `Clock::settle()` happens. This was
fixed by blocking using `FUTURE_DISPATCH` on the downstream logic in the
allocator that is invoked to handle the delivered event.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
00900561a1b8dd03a7a2f3d60a036b4beb920aa1 

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


Testing
---

./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
--gtest_repeat=2000 # on OSX
./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
--gtest_repeat=100  # on Ubuntu Wily (slow VM)


Thanks,

Neil Conway

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

> On Dec. 3, 2015, 10:42 p.m., Vinod Kone wrote:
> > i'm assuming there are already tests that verify this change?

just checked. Not every function. I'll add tests in the subsequent reviews.


- Jie


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


On Dec. 1, 2015, 1:21 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40806/
> ---
> 
> (Updated Dec. 1, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made HDFS::exists asynchronous.
> 
> This also fixed a bug in the orignal code: the original code never returns 
> false.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
>   src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
>   src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
> 
> Diff: https://reviews.apache.org/r/40806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40935: Fixed flakiness in MasterMaintenanceTest.InverseOffersFilters.

[Neil Conway]

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

(Updated Dec. 4, 2015, 12:17 a.m.)


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


Changes
---

Fix another instance of the `send(); Clock::settle();` pattern in the same test.


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


Repository: mesos


Description
---

There were two problems:

(1) After launching two tasks, we assumed that we would see TASK_RUNNING updates
for the tasks in the same order they were launched. This is not guaranteed,
so adjust the test to handle TASK_RUNNING updates in the order they are
received.

(2) The test used this pattern:

Mesos m;
Call c;

m.send(c);
Clock::settle();
// Trigger a new batch allocation that reflects the call
Clock::advance();

However, this is actually unsafe (see MESOS-3760): the send() call might not
have reached the master by the time `Clock::settle()` happens. This was
fixed by blocking using `FUTURE_DISPATCH` on the downstream logic in the
allocator that is invoked to handle the delivered event.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
00900561a1b8dd03a7a2f3d60a036b4beb920aa1 

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


Testing
---

./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
--gtest_repeat=2000 # on OSX
./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
--gtest_repeat=100  # on Ubuntu Wily (slow VM)


Thanks,

Neil Conway

Re: Review Request 40941: Added a helper for HDFS client to shell out commands.

[Vinod Kone]

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

Ship it!



src/hdfs/hdfs.cpp (line 52)


Add a TODO to eventually move this to subprocess?



src/hdfs/hdfs.cpp (lines 55 - 56)


instead of CHECKs why not return failed futures?


- Vinod Kone


On Dec. 4, 2015, 12:04 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40941/
> ---
> 
> (Updated Dec. 4, 2015, 12:04 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper for HDFS client to shell out commands.
> 
> Code is copied from https://reviews.apache.org/r/40559/.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
> 
> Diff: https://reviews.apache.org/r/40941/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

> On Dec. 1, 2015, 10:40 a.m., Bernd Mathiske wrote:
> > src/hdfs/hdfs.cpp, line 100
> > 
> >
> > This block seems to be reusable in a lot of situations. Any chance we 
> > could abstract it and make it part of class Subprocess?
> > 
> > In a separate ticket, I suppose. ==> Place a TODO here?

Due to some controversy in the Subprocess patch. I move the logic to hdfs.cpp 
as a helper and that slightly simplies the logics here.


- Jie


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


On Dec. 4, 2015, 12:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40806/
> ---
> 
> (Updated Dec. 4, 2015, 12:20 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made HDFS::exists asynchronous.
> 
> This also fixed a bug in the orignal code: the original code never returns 
> false.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
>   src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
>   src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
> 
> Diff: https://reviews.apache.org/r/40806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40935: Fixed flakiness in MasterMaintenanceTest.InverseOffersFilters.

[Neil Conway]

> On Dec. 4, 2015, 12:24 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [40935]
> > 
> > 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
  
/var/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_mult
 iverse_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/lis
 
ts/us.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/l
 
ists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_source_Sources
 

Review Request 40942: Made HDFS::rm asynchronous.

[Jie Yu]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Made HDFS::rm asynchronous.


Diffs
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

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

(Updated Dec. 4, 2015, 12:45 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Made HDFS::exists asynchronous.

This also fixed a bug in the orignal code: the original code never returns 
false.


Diffs
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40941: Added a helper for HDFS client to shell out commands.

[Jie Yu]

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

(Updated Dec. 4, 2015, 12:45 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Added a helper for HDFS client to shell out commands.

Code is copied from https://reviews.apache.org/r/40559/.


Diffs
-

  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 40943: Made HDFS::copyFromLocal asynchronous.

[Jie Yu]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Made HDFS::copyFromLocal asynchronous.


Diffs
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40873: RegistryClientTests: Created separate server to serve blobs.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40872, 40873]

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

- Mesos ReviewBot


On Dec. 3, 2015, 5:20 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40873/
> ---
> 
> (Updated Dec. 3, 2015, 5:20 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By having a separate blob server, it simulates the real world more closely. 
> It also allows the test server to be in accept mode early.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/40873/diff/
> 
> 
> Testing
> ---
> 
> make check (600 count).
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 40945: Made HDFS::copyToLocal asynchronous.

[Jie Yu]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Made HDFS::copyToLocal asynchronous.


Diffs
-

  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
  src/launcher/fetcher.cpp eafdda38adde871aa33224187cd9fb774faac8fb 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40935: Fixed flakiness in MasterMaintenanceTest.InverseOffersFilters.

[Joseph Wu]

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

Ship it!


Tested on Centos7.

There still appears to be some flakiness (in the first inverse offer filter) 
that requires further investigation.  But these changes make the test 
significantly more stable.
I vote for committing this patch, but keeping the JIRA open until we nail down 
the flakiness for good.


src/tests/master_maintenance_tests.cpp (lines 1485 - 1488)


It might be valuable to expect an update for each task.

Maybe check:
`updateStatus.task_id() == taskInfo1.task_id() || updateStatus.task_id() == 
taskInfo2.task_id()`
And between the two acknowledges, you could do:
`updateStatus.task_id() != event.get().update().status().taskid()`


Note: As far as I know, no other HTTP scheduler library test launches two 
tasks simultaneously.


- Joseph Wu


On Dec. 3, 2015, 4:17 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40935/
> ---
> 
> (Updated Dec. 3, 2015, 4:17 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-4059
> https://issues.apache.org/jira/browse/MESOS-4059
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There were two problems:
> 
> (1) After launching two tasks, we assumed that we would see TASK_RUNNING 
> updates
> for the tasks in the same order they were launched. This is not 
> guaranteed,
> so adjust the test to handle TASK_RUNNING updates in the order they are
> received.
> 
> (2) The test used this pattern:
> 
> Mesos m;
> Call c;
> 
> m.send(c);
> Clock::settle();
> // Trigger a new batch allocation that reflects the call
> Clock::advance();
> 
> However, this is actually unsafe (see MESOS-3760): the send() call might 
> not
> have reached the master by the time `Clock::settle()` happens. This was
> fixed by blocking using `FUTURE_DISPATCH` on the downstream logic in the
> allocator that is invoked to handle the delivered event.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 00900561a1b8dd03a7a2f3d60a036b4beb920aa1 
> 
> Diff: https://reviews.apache.org/r/40935/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
> --gtest_repeat=2000 # on OSX
> ./src/mesos-tests --gtest_filter="MasterMaintenanceTest.InverseOffersFilters" 
> --gtest_repeat=100  # on Ubuntu Wily (slow VM)
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40332: Quota: Implemented recovery in hierarchical allocator.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 3, 2015, 9:10 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40332/
> ---
> 
> (Updated Dec. 3, 2015, 9:10 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3981
> https://issues.apache.org/jira/browse/MESOS-3981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1cd8d16661568010901e74705375e7719cdfb8a0 
>   src/master/allocator/mesos/hierarchical.cpp 
> a8f65b72488505afd3677ecdeb9b821ea27af7e0 
> 
> Diff: https://reviews.apache.org/r/40332/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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. 3, 2015, 5:40 p.m.)


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


Changes
---

Add comments to protobuf field v1Compat.


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 39597: Add Newbie guide.

[Diana Arroyo]

> On Nov. 21, 2015, 1:05 a.m., Timothy Chen wrote:
> > Diana are you still able to finish the comments?

Hey Tim, Yes, I'll finish them up.


- Diana


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


On Oct. 30, 2015, 9 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39597/
> ---
> 
> (Updated Oct. 30, 2015, 9 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Vinod Kone.
> 
> 
> Bugs: MESOS-3224
> https://issues.apache.org/jira/browse/MESOS-3224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Newbie guide.
> 
> 
> Diffs
> -
> 
>   docs/NewbieQuickStart.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39597/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 40946: Made HDFS::du asynchrounous.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40941, 40806, 40942, 40943, 40945, 40946]

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

- Mesos ReviewBot


On Dec. 4, 2015, 1:34 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40946/
> ---
> 
> (Updated Dec. 4, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3951
> https://issues.apache.org/jira/browse/MESOS-3951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made HDFS::du asynchrounous.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
>   src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 
>   src/slave/containerizer/fetcher.cpp 
> 26df3d5c55dde9f11b497b4808316e33a3fe395c 
> 
> Diff: https://reviews.apache.org/r/40946/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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

Review request for mesos.


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 40910: [stout]: Made license-headers doxygen-compatible.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 3, 2015, 1:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40910/
> ---
> 
> (Updated Dec. 3, 2015, 1:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adjusts license headers of C++ source files which were
> added since dc23756.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 40629f4623891162d830e7addf341c2ebd657e8b 
> 
> Diff: https://reviews.apache.org/r/40910/diff/
> 
> 
> Testing
> ---
> 
> `make check` under OS X 10.10.5 succeeded modulo MESOS-4045.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40911: Made license-headers doxygen-compatible.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 3, 2015, 1:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40911/
> ---
> 
> (Updated Dec. 3, 2015, 1:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adjusts license headers of C++ source files added since
> fa36917.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/handle.cpp 09589e9616f115df5e55b7fc813169cfdbbec8be 
>   src/tests/containerizer/launcher.cpp 
> 22b44baf6509d8cda780184721ef05320f40b10d 
>   src/tests/master_quota_tests.cpp c9d78cffc30539ce2d29360b231fcaf9e5b592ea 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> ac46806a712113170b349a969a9f1132723116f0 
> 
> Diff: https://reviews.apache.org/r/40911/diff/
> 
> 
> Testing
> ---
> 
> `make check` under OS X 10.10.5 succeeded modulo MESOS-4045.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40445: Added linter for license headers in some file types.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 3, 2015, 1:06 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40445/
> ---
> 
> (Updated Dec. 3, 2015, 1:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Marco Massenzio, and Michael Park.
> 
> 
> Bugs: MESOS-3581
> https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a style check for license headers in C, C++, and Proto files.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 66b45692c3c04f68358b63d52e4d87934f241bd7 
> 
> Diff: https://reviews.apache.org/r/40445/diff/
> 
> 
> Testing
> ---
> 
> Ran the a whole clean checkout through the linter with only one expected 
> failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto` which lacks a 
> license).
> 
> `make check` under OS X 10.10.5 succeeded modulo MESOS-4045.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>