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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40954]

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

- Mesos ReviewBot


On Dec. 4, 2015, 5:44 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40954/
> ---
> 
> (Updated Dec. 4, 2015, 5:44 a.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 40211: Add mesos provisioner doc.

[Timothy Chen]

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

(Updated Dec. 4, 2015, 9:26 a.m.)


Review request for mesos, Jie Yu and Jojy Varghese.


Repository: mesos


Description
---

Add mesos provisioner doc.


Diffs (updated)
-

  docs/mesos-provisioner.md PRE-CREATION 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40961, 40821, 39450]

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

- Mesos ReviewBot


On Dec. 4, 2015, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Dec. 4, 2015, 10:09 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3984
> https://issues.apache.org/jira/browse/MESOS-3984
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> a45b3dd0a8237fdd080536cb9ab600e71ad939d3 
> 
> Diff: https://reviews.apache.org/r/39450/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Bernd Mathiske]

> On Dec. 3, 2015, 7:33 p.m., Ben Mahler wrote:
> > 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

Makes sense, this will be better.


- Bernd


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


On Nov. 19, 2015, 1: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, 1: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 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. 4, 2015, 9:38 a.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  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 39450: Quota: Added hierarchical allocator-specific tests.

[Alexander Rukletsov]

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

(Updated Dec. 4, 2015, 9:37 a.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
a45b3dd0a8237fdd080536cb9ab600e71ad939d3 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40945: Made HDFS::copyToLocal asynchronous.

[Bernd Mathiske]

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



src/hdfs/hdfs.cpp (line 302)


By closure-capturing values from above we could print out more specifics 
about what subprocess could not be reaped here.


- Bernd Mathiske


On Dec. 3, 2015, 5:11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40945/
> ---
> 
> (Updated Dec. 3, 2015, 5:11 p.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::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 40806: Made HDFS::exists asynchronous.

[Bernd Mathiske]

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

Ship it!



src/hdfs/hdfs.cpp (line 143)


Maybe we can get some extra info about what subprocess failed to be reaped 
by capturing some values from above in the closure?


- Bernd Mathiske


On Dec. 3, 2015, 4:45 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40806/
> ---
> 
> (Updated Dec. 3, 2015, 4:45 p.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 40211: Add mesos provisioner doc.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40211]

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

- Mesos ReviewBot


On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> ---
> 
> (Updated Dec. 4, 2015, 9:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add mesos provisioner doc.
> 
> 
> Diffs
> -
> 
>   docs/mesos-provisioner.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40211/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 40964: Quota: Ensured resources in `QuotaInfo` do not have non-default role.

[Alexander Rukletsov]

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

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


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/quota/quota.proto 5529ab1aa27c15f7047916644a2dd4a90b2e73cb 
  src/master/quota.cpp e75abfc5bd39438483f5d65a2e62b57e7e8c8cf5 
  src/master/quota_handler.cpp 76de7c44c2cbd16c5f76b1dc0a94f567e037bffe 
  src/tests/master_quota_tests.cpp 7aa116fcf459fb05e832a22bbcd5d576529317c1 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Alexander Rukletsov]

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

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


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
a45b3dd0a8237fdd080536cb9ab600e71ad939d3 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40821: Quota: Ensured the sorter for quota'ed roles does not contain revocable resources.

[Alexander Rukletsov]

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

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


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
388ed1775b03cef3f5ea41e2ee17a752e834527d 
  src/master/allocator/mesos/hierarchical.cpp 
6d82ae7be28ad0dba35941114bafb05b7540ebd1 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Review Request 40961: 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/40961/
---

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

Addendum to b9762afeb851662fc6122e177abf1c1a4f4921ae.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
6d82ae7be28ad0dba35941114bafb05b7540ebd1 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40942: Made HDFS::rm asynchronous.

[Bernd Mathiske]

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

Ship it!



src/hdfs/hdfs.cpp (line 234)


Here we could include more info about the subprocess.


- Bernd Mathiske


On Dec. 3, 2015, 4:45 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40942/
> ---
> 
> (Updated Dec. 3, 2015, 4:45 p.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::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 40943: Made HDFS::copyFromLocal asynchronous.

[Bernd Mathiske]

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

Ship it!



src/hdfs/hdfs.cpp (line 270)


Can we include more info about the subprocess here, please?


- Bernd Mathiske


On Dec. 3, 2015, 4:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40943/
> ---
> 
> (Updated Dec. 3, 2015, 4:57 p.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::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 40964: Quota: Ensured resources in `QuotaInfo` do not have non-default role.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40964]

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

- Mesos ReviewBot


On Dec. 4, 2015, 11:53 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40964/
> ---
> 
> (Updated Dec. 4, 2015, 11:53 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3965
> https://issues.apache.org/jira/browse/MESOS-3965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.proto 5529ab1aa27c15f7047916644a2dd4a90b2e73cb 
>   src/master/quota.cpp e75abfc5bd39438483f5d65a2e62b57e7e8c8cf5 
>   src/master/quota_handler.cpp 76de7c44c2cbd16c5f76b1dc0a94f567e037bffe 
>   src/tests/master_quota_tests.cpp 7aa116fcf459fb05e832a22bbcd5d576529317c1 
> 
> Diff: https://reviews.apache.org/r/40964/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40348: [4/4] Quota Authorization: Documented quota authorization.

[Jan Schlicht]

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

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


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Quota: Documented quota authorization.


Diffs (updated)
-

  docs/authorization.md f5ed75fcd0785fde38058917354fcf6d668dcccb 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40346: [2/4] Quota Authorization: Implemented authorization of quota requests in the authorizer.

[Jan Schlicht]

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

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


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Addressed issues and rebased.


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


Repository: mesos


Description
---

Quota: Implemented authorization of quota requests in the authorizer.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
  src/authorizer/local/authorizer.hpp 965658b53d21734a2dbf3713d02d44d728fd6cca 
  src/authorizer/local/authorizer.cpp 1ac6a41d20454515a3ef2b84e296494a4592b3ea 
  src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
  src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39288: Quota: Added authentication of quota requests.

[Jan Schlicht]

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

(Updated Dec. 4, 2015, 2:51 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Address issues.


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


Repository: mesos


Description
---

Added authentication of quota requests.


Diffs (updated)
-

  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/quota_handler.cpp 76de7c44c2cbd16c5f76b1dc0a94f567e037bffe 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Jan Schlicht]

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

(Updated Dec. 4, 2015, 2:53 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Addressed issues and rebased.


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


Repository: mesos


Description
---

Quota: Added "SetQuota" message to ACL protobuf.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
74fcc86d3c92cb3aa27e45b647b1653705b3201c 

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


Testing
---

make check


Thanks,

Jan Schlicht

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

[Joris Van Remoortere]

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

Ship it!


Addressed Joseph's comments, verified with Joseph & Neil before committing.

- Joris Van Remoortere


On Dec. 4, 2015, 12:17 a.m., Neil Conway wrote:
> 
> ---
> 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.
> 
> 
> 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
> 
>

Review Request 40975: Document that libprocess ignores SIGPIPE

[James Peach]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Document that libprocess ignores SIGPIPE.


Diffs
-

  3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 

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


Testing
---

None.


Thanks,

James Peach

Re: Review Request 40821: Quota: Ensured the sorter for quota'ed roles does not contain revocable resources.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 4, 2015, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40821/
> ---
> 
> (Updated Dec. 4, 2015, 10:09 a.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
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 388ed1775b03cef3f5ea41e2ee17a752e834527d 
>   src/master/allocator/mesos/hierarchical.cpp 
> 6d82ae7be28ad0dba35941114bafb05b7540ebd1 
> 
> Diff: https://reviews.apache.org/r/40821/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40961: 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/40961/#review108992
---

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 4, 2015, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40961/
> ---
> 
> (Updated Dec. 4, 2015, 10:09 a.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
> ---
> 
> Addendum to b9762afeb851662fc6122e177abf1c1a4f4921ae.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 6d82ae7be28ad0dba35941114bafb05b7540ebd1 
> 
> Diff: https://reviews.apache.org/r/40961/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40985: Fixed the flaky PersistentVolumeTest.SlaveRecovery test.

[Vinod Kone]

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

Ship it!



src/tests/persistent_volume_tests.cpp (line 680)


you can kill this.


- Vinod Kone


On Dec. 4, 2015, 8:27 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40985/
> ---
> 
> (Updated Dec. 4, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3907
> https://issues.apache.org/jira/browse/MESOS-3907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the flaky PersistentVolumeTest.SlaveRecovery test.
> 
> We need to wait for the executor to re-register before advance the clock. 
> Otherwise, the timeout event might be handled first and result in task being 
> lost.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 0d162ca60f85d19421d1b4e6dbb1ceb0b31fd150 
> 
> Diff: https://reviews.apache.org/r/40985/diff/
> 
> 
> Testing
> ---
> 
> bin/mesos-test.sh --gtest_filter=PersistentVolumeTest.SlaveRecovery 
> --gtest_repeat=100
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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

[Gilbert Song]

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

(Updated Dec. 4, 2015, 2:56 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 (updated)
-

  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 40964: Quota: Ensured resources in `QuotaInfo` do not have non-default role.

[Joris Van Remoortere]

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

Ship it!


Fixed in-line


src/master/quota_handler.cpp (line 97)


Let's not restrict this function to only `Quota request`s


- Joris Van Remoortere


On Dec. 4, 2015, 11:53 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40964/
> ---
> 
> (Updated Dec. 4, 2015, 11:53 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3965
> https://issues.apache.org/jira/browse/MESOS-3965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.proto 5529ab1aa27c15f7047916644a2dd4a90b2e73cb 
>   src/master/quota.cpp e75abfc5bd39438483f5d65a2e62b57e7e8c8cf5 
>   src/master/quota_handler.cpp 76de7c44c2cbd16c5f76b1dc0a94f567e037bffe 
>   src/tests/master_quota_tests.cpp 7aa116fcf459fb05e832a22bbcd5d576529317c1 
> 
> Diff: https://reviews.apache.org/r/40964/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Joris Van Remoortere]

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

Ship it!


Touched up some comments before committing.


src/tests/hierarchical_allocator_tests.cpp (line 183)


static?


- Joris Van Remoortere


On Dec. 4, 2015, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Dec. 4, 2015, 10:09 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3984
> https://issues.apache.org/jira/browse/MESOS-3984
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> a45b3dd0a8237fdd080536cb9ab600e71ad939d3 
> 
> Diff: https://reviews.apache.org/r/39450/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Diana Arroyo]

> On Dec. 4, 2015, 9:48 p.m., Joseph Wu wrote:
> > Diana, can you add `hausdorff`, `kaysoky`, `hartem`, and `jvanremoortere` 
> > to this review (and future CMake reviews)?
> > 
> > ---
> > 
> > Ran `cmake .. && make` on OSX.  Hit this error:
> > ```
> > mesos/src/slave/containerizer/mesos/linux_launcher.cpp:20:10: fatal error: 
> > 'linux/sched.h' file not found
> > #include 
> > ```
> > 
> > Note that `src/Makefile.am` does this:
> > ```
> > if OS_LINUX
> > libmesos_no_3rdparty_la_SOURCES += $(MESOS_LINUX_FILES)
> > else
> > EXTRA_DIST += $(MESOS_LINUX_FILES)
> > endif
> > ```
> > And `linux_launcher.cpp` is in `MESOS_LINUX_FILES`.

Reference: "can you add hausdorff, kaysoky, hartem, and jvanremoortere to this 
review (and future CMake reviews)?":  Done.
Reference: "fatal error: 'linux/sched.h' file not found": working...


- Diana


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


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

Review Request 40995: Added test cases for role behavior.

[Neil Conway]

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

Review request for mesos, Adam B and Greg Mann.


Repository: mesos


Description
---

Added test cases for role behavior.


Diffs
-

  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
  src/tests/role_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Neil Conway

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40978]

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

- Mesos ReviewBot


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

Review Request 40985: Fixed the flaky PersistentVolumeTest.SlaveRecovery test.

[Jie Yu]

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

Fixed the flaky PersistentVolumeTest.SlaveRecovery test.

We need to wait for the executor to re-register before advance the clock. 
Otherwise, the timeout event might be handled first and result in task being 
lost.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
0d162ca60f85d19421d1b4e6dbb1ceb0b31fd150 

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


Testing
---

bin/mesos-test.sh --gtest_filter=PersistentVolumeTest.SlaveRecovery 
--gtest_repeat=100


Thanks,

Jie Yu

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


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. 4, 2015, 7:02 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38878/
> ---
> 
> (Updated Dec. 4, 2015, 7:02 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 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 40056: Make hook execution order deterministic.

[Niklas Nielsen]

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


Hi Haosdent!

I apologize the tardy reply. The patch looks good but needs rebasing.
Also, have you thought of a way to test this?

With a test (maybe by just ensuring the existing ordering of the test modules 
are indeed run in order), we should be able to land this. Kapil, any concerns 
on this?

- Niklas Nielsen


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

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On Dec. 4, 2015, 1:53 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40345/
> ---
> 
> (Updated Dec. 4, 2015, 1:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added "SetQuota" message to ACL protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 74fcc86d3c92cb3aa27e45b647b1653705b3201c 
> 
> Diff: https://reviews.apache.org/r/40345/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 40987: Fix apply-reviews.py to work for first-time contributors.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Vinod 
Kone.


Repository: mesos


Description
---

`apply-reviews.py` does `git commit --author 'NAME'`.  This only works for 
contributors that have contributed before.

`apply-review.sh` does `git commit --author 'NAME '` which works for 
first-time contributors too.


Diffs
-

  support/apply-reviews.py f6c05be77bcf963c5aa6036a4660296d390bd475 

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


Testing
---

`support/apply-reviews.py -n -r 40951`

^ Should not complain with:
```
fatal: --author 'Diana Arroyo' is not 'Name ' and matches no existing 
author
```


Thanks,

Joseph Wu

Re: Review Request 40988: stout: Fixed comments for numify(), cleaned up some test code.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 4, 2015, 9:50 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40988/
> ---
> 
> (Updated Dec. 4, 2015, 9:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> stout: Fixed comments for numify(), cleaned up some test code.
> 
> Note that numify() handles negative numbers inconsistently, depending on 
> whether they are specified in hex or decimal (see test case). Can you guys 
> take a look at fixing?
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 322a89946befb0d7006124a08b415e2ef0a03e97 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 522fc3470eb3496f7dfe1eeb814b171bcff21f38 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> fc5a821bf3bb1416b67103d0ffd7ab5b88f45cca 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 7fa06a980b58040f68bf92d217c866f9e48a57d3 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7c0309c41ee5ad18bed30aa31a8361f11bca23a1 
> 
> Diff: https://reviews.apache.org/r/40988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 40988: stout: Fixed comments for numify(), cleaned up some test code.

[Neil Conway]

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

Review request for mesos, Jie Yu and Cong Wang.


Repository: mesos


Description
---

stout: Fixed comments for numify(), cleaned up some test code.

Note that numify() handles negative numbers inconsistently, depending on 
whether they are specified in hex or decimal (see test case). Can you guys take 
a look at fixing?


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
322a89946befb0d7006124a08b415e2ef0a03e97 
  3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
522fc3470eb3496f7dfe1eeb814b171bcff21f38 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
fc5a821bf3bb1416b67103d0ffd7ab5b88f45cca 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
7fa06a980b58040f68bf92d217c866f9e48a57d3 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7c0309c41ee5ad18bed30aa31a8361f11bca23a1 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 40346: [2/4] Quota Authorization: Implemented authorization of quota requests in the authorizer.

[Alexander Rukletsov]

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

Ship it!



include/mesos/authorizer/authorizer.hpp (line 175)


s/with/for


- Alexander Rukletsov


On Dec. 4, 2015, 2:34 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40346/
> ---
> 
> (Updated Dec. 4, 2015, 2:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Implemented authorization of quota requests in the authorizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
>   src/authorizer/local/authorizer.hpp 
> 965658b53d21734a2dbf3713d02d44d728fd6cca 
>   src/authorizer/local/authorizer.cpp 
> 1ac6a41d20454515a3ef2b84e296494a4592b3ea 
>   src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
>   src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 
> 
> Diff: https://reviews.apache.org/r/40346/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Joseph Wu]

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


Diana, can you add `hausdorff`, `kaysoky`, `hartem`, and `jvanremoortere` to 
this review (and future CMake reviews)?

---

Ran `cmake .. && make` on OSX.  Hit this error:
```
mesos/src/slave/containerizer/mesos/linux_launcher.cpp:20:10: fatal error: 
'linux/sched.h' file not found
#include 
```

Note that `src/Makefile.am` does this:
```
if OS_LINUX
libmesos_no_3rdparty_la_SOURCES += $(MESOS_LINUX_FILES)
else
EXTRA_DIST += $(MESOS_LINUX_FILES)
endif
```
And `linux_launcher.cpp` is in `MESOS_LINUX_FILES`.

- Joseph Wu


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

Re: Review Request 40953: Avoid accepting hex float literals

[Cong Wang]

> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 37
> > 
> >
> > Just noticed that this fails to parse negative hex numbers like `-0x2`. 
> > This seems overly restrictive to me.

If you have any use case for that, sure, please do make it better. But I don't 
have any.


> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 38
> > 
> >
> > Instead of this comment we should specify what *we* support (we are not 
> > writing a C++ lexer, but interpreting user input here).
> > 
> > My expectation for this would be to support what e.g., a `stringstream` 
> > can parse with `hex` or `hexfloat` (or `stoi`, `strtof`, ...).

Yeah we can add doc for this function. Or even better: fix boost::lexical_cast.


> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp, line 42
> > 
> >
> > As currently implemented we should test that mixed float & exponent 
> > forms also fail, e.g. `0xF.Ep-1`.
> > 
> > I do not agree that being so limited is needed.
> > 
> > Note that e.g., `strtof` would fail to parse `0x10.9` since it lacks 
> > the required exponent.

Ditto, if you need that case, please go for it.


- Cong


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


On Dec. 4, 2015, 5:23 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40953/
> ---
> 
> (Updated Dec. 4, 2015, 5:23 a.m.)
> 
> 
> 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 40953: Avoid accepting hex float literals

[Benjamin Bannier]

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


No idea why this suddenly had to be so expedited, but here still my comments as 
I believe that while this makes the test suite pass it is broken usagewise.


3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 37)


Just noticed that this fails to parse negative hex numbers like `-0x2`. 
This seems overly restrictive to me.



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 38)


Instead of this comment we should specify what *we* support (we are not 
writing a C++ lexer, but interpreting user input here).

My expectation for this would be to support what e.g., a `stringstream` can 
parse with `hex` or `hexfloat` (or `stoi`, `strtof`, ...).



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 42)


As currently implemented we should test that mixed float & exponent forms 
also fail, e.g. `0xF.Ep-1`.

I do not agree that being so limited is needed.

Note that e.g., `strtof` would fail to parse `0x10.9` since it lacks the 
required exponent.


- Benjamin Bannier


On Dec. 4, 2015, 5:23 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40953/
> ---
> 
> (Updated Dec. 4, 2015, 5:23 a.m.)
> 
> 
> 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 39782: Add a comment for os::libraries::setPaths.

[Niklas Nielsen]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp (line 321)


Cool! Mind moving this down on the line below and add a ':', so it becomes:

```
// Updates the value of LD_LIBRARY_PATH environment variable.
// NOTE: that setting this is only useful for child processes. Neither the 
Linux nor the OS X
// linkers dynamically update their search path after a process is launched.
```


- Niklas Nielsen


On Nov. 26, 2015, 6:06 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39782/
> ---
> 
> (Updated Nov. 26, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a comment for os::libraries::setPaths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
> 
> Diff: https://reviews.apache.org/r/39782/diff/
> 
> 
> Testing
> ---
> 
> No code changes.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 40660: Linked against executor PID's to ensure ordered message delivery

[Ben Mahler]

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

Ship it!


We may also want to link in the recovery path, but the agent <-> executor 
protocol is such that we don't need to in order to fix the issue.

- Ben Mahler


On Nov. 24, 2015, 6:25 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40660/
> ---
> 
> (Updated Nov. 24, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we did not `link` against the executor `PID` while 
> (re)-registering. This might lead to libprocess creating ephemeral sockets 
> everytime a `send(...)` was invoked. This was leading to races where messages 
> might appear on the Executor out of order. This change does a `link(...)` on 
> the executor PID to ensure ordered message delivery.
> 
> ---Not to be included in commit message---
> I am still not comfortable bringing back the reverted commit 
> https://reviews.apache.org/r/40107/ . I can see one more race condition even 
> with a `link(...)`. We can still have messages coming out of order when the 
> first socket fails after sending the first message when still in flight. A 
> new socket gets created when we send the second message now, which might 
> arrive earlier then the first message leading to a race. But, this is a 
> behavior that is heavily relied upon elsewhere in our code-base. Happy to be 
> proven wrong though and be convinced that we can bring back the reverted 
> commit now after this change.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/40660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40987: Fix apply-reviews.py to work for first-time contributors.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Dec. 4, 2015, 9:30 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40987/
> ---
> 
> (Updated Dec. 4, 2015, 9:30 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `apply-reviews.py` does `git commit --author 'NAME'`.  This only works for 
> contributors that have contributed before.
> 
> `apply-review.sh` does `git commit --author 'NAME '` which works for 
> first-time contributors too.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py f6c05be77bcf963c5aa6036a4660296d390bd475 
> 
> Diff: https://reviews.apache.org/r/40987/diff/
> 
> 
> Testing
> ---
> 
> `support/apply-reviews.py -n -r 40951`
> 
> ^ Should not complain with:
> ```
> fatal: --author 'Diana Arroyo' is not 'Name ' and matches no existing 
> author
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 3, 2015, 11:37 p.m., Greg Mann wrote:
> 
> ---
> 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.
> 
> 
> 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 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
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40944]

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

- Mesos ReviewBot


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

Re: Review Request 37540: Add perf event API

[Cong Wang]

> On Dec. 2, 2015, 6:13 p.m., Niklas Nielsen wrote:
> > Ping - Cong, is there anything you need to close the last issues? :)

I was blocked at the second to the last issue above and switched to something 
else, now I can think more about it. Thanks!


- Cong


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


On Sept. 30, 2015, 12:12 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 30, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 41000: Shortened allocation_interval in ReservationTests to avoid flakiness.

[Greg Mann]

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

Review request for mesos, Jie Yu and Michael Park.


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


Repository: mesos


Description
---

Shortened allocation_interval in ReservationTests to avoid flakiness.


Diffs
-

  src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 

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


Testing
---

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

While only ACLMultipleOperations is currently flaky, in this patch I shorten 
the `allocation_interval` for all tests in this file to avoid flakiness in the 
future as the tests are modified, instead promoting hard failures that will be 
easy to identify.


Thanks,

Greg Mann

Review Request 41001: Improved 'ReservationTest.ACLMultipleOperations'.

[Greg Mann]

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

Review request for mesos.


Repository: mesos


Description
---

Improved 'ReservationTest.ACLMultipleOperations'.


Diffs
-

  src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 

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


Testing
---

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

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


Thanks,

Greg Mann

Re: Review Request 40999: Fixed flakiness in ReservationTest.ACLMultipleOperations.

[Joseph Wu]

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



src/tests/reservation_tests.cpp (lines 1663 - 1664)


You can be more explicit in your expectations (about batch allocations) by 
using `Clock::advance(flags.allocation_interval)`.

Using the above will ensure that only one allocation cycle can be triggered.


- Joseph Wu


On Dec. 4, 2015, 4:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40999/
> ---
> 
> (Updated Dec. 4, 2015, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4067
> https://issues.apache.org/jira/browse/MESOS-4067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in ReservationTest.ACLMultipleOperations.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
> 
> Diff: https://reviews.apache.org/r/40999/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="ReservationTest.ACLMultipleOperations" bin/mesos-tests.sh 
> --gtest_repeat=2000 --gtest_break_on_failure`
> 
> This test does not fail often unless the `allocation_interval` is reset to a 
> shorter period (5ms, for example). The patch following this one implements 
> this change for all tests in `ReservationTest` to reduce their potential for 
> flakiness (https://reviews.apache.org/r/41000/).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38117]

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

- Mesos ReviewBot


On Dec. 4, 2015, 7:18 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Dec. 4, 2015, 7:18 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
>   include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 40998: Reinstated previously reverted commit around CHECK's for ordered delivery of messages in Command Executor.

[Anand Mazumdar]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

This change reinstates the previously reverted commit 
`b6d4b28a4c9ca717ad8be5bbc27e40c005fc51ad`
https://github.com/apache/mesos/commit/b6d4b28a4c9ca717ad8be5bbc27e40c005fc51ad#diff-3ddba917cc11631eb5ede254da408a27


Diffs
-

  src/launcher/executor.cpp f90ea01131e6fa28e42f0c00a317d66a49f81ffa 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 40999: Fixed flakiness in ReservationTest.ACLMultipleOperations.

[Greg Mann]

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

Review request for mesos, Jie Yu and Michael Park.


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


Repository: mesos


Description
---

Fixed flakiness in ReservationTest.ACLMultipleOperations.


Diffs
-

  src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 

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


Testing
---

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

This test does not fail often unless the `allocation_interval` is reset to a 
shorter period (5ms, for example). The patch following this one implements this 
change for all tests in `ReservationTest` to reduce their potential for 
flakiness (https://reviews.apache.org/r/41000/).


Thanks,

Greg Mann

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

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


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

Re: Review Request 39848: Validate revocable resources

[Vinod Kone]

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



src/slave/slave.cpp (line 1503)


I think at this point it's worthwhile to factor out the validations in this 
function into slave/validation.cpp file like we have done in 
master/validation.cpp. The validation should likely return an object 
(TaskStatus?) that contains the message and REASON to be used in the status 
update.

This would also help with unit testing the validations more easily.


- Vinod Kone


On Nov. 5, 2015, 7:36 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39848/
> ---
> 
> (Updated Nov. 5, 2015, 7:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2647
> https://issues.apache.org/jira/browse/MESOS-2647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validates if there are enough revocable resources when launching
> the task on slave.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ddeece4334b13cf8b5ab7d87012bbf946640025b 
> 
> Diff: https://reviews.apache.org/r/39848/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Niklas Nielsen]

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


looks great so far! We probably need a few tests to exercise the different 
scenarios (making sure that combinations of the thresholds exhibits the right 
behavior)


src/Makefile.am (line 1674)


Tab seems off - set tabstop to 8 :)



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


We usually limit the use of typedefs for simpler types (like a list of a 
concrete type). Let's expand the use here.



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


Let's add the QoS controller policy documentation here :) For example, why 
5 and 15 minute thresholds, but not 1 minute?



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


You don't have to use 'this->' for disambiguation here.



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


How will this behave? You return a future that will never be satisfied, no? 
Can you return the error instead?



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


Same as above.



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


Kill 'this->'



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


We usually don't know 'getXYZ', if you can infer the operation type from 
the name. For example, just calling it 'loadAvg()', which in fact should be 
'loadAverage()' to be pedantic :)



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


You evict everything by issuing corrections, what do you think about using 
that term?



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


How about calling them 'preemptionTargets' or 'evictionTargets' or simply 
'targets'?



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


Not only the executors disappear, the entire container is nuked. Maybe we 
should refer to executor+tasks or just, 'container'.



src/slave/qos_controllers/load.cpp (lines 157 - 162)


4 space indent per argument wrapping.



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


Two newlines between implementing functions here and rest of file



src/slave/qos_controllers/load.cpp (lines 176 - 177)


4 space indent on argument list wrapping :)



src/slave/qos_controllers/load.cpp (lines 181 - 184)


We usually limit this kind of single call wrappers. Doesn't change the 
abstraction/return type of os::loadavg().

I see it is for the test; can we create a c++11 lambda in place with 
`[](){return os::load();}` instead?

I'd like to get rid of the wrapper - it's a bit of a code smell :)



src/slave/qos_controllers/load.cpp (lines 190 - 195)


We can skip this for now and leave a NULL in the compatibility function 
pointer field.



src/slave/qos_controllers/load.cpp (lines 204 - 206)


stout has string parsing, take a look at `numify<>()` :)



src/slave/qos_controllers/load.cpp (lines 214 - 215)


Move the else line up, so it becomes: 

```
} else if (...
```



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


Should we throw a warning or error here instead of just a NULL pointer?



src/tests/oversubscription_tests.cpp (line 63)


kill this new line



src/tests/oversubscription_tests.cpp (line 171)


Think we should refer to the load function with ``'s. Try to see Greg 
Mann's proposal on the dev list :)



src/tests/oversubscription_tests.cpp (lines 199 - 201)


We need to do a scan for argument wrapping. Should be 4 space indent.



src/tests/oversubscription_tests.cpp (line 1157)


Instead of refering to BE, we standardized on 'recovable' I think. @Vinod: 
what do you think?



src/tests/oversubscription_tests.cpp (line 1170)


Doesn't this need to be 

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40838, 40884]

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

- Mesos ReviewBot


On Dec. 4, 2015, 10:56 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40884/
> ---
> 
> (Updated Dec. 4, 2015, 10:56 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 40995: Added test cases for role behavior.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40995]

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

- Mesos ReviewBot


On Dec. 4, 2015, 11:09 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 4, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Timothy Chen]

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



src/slave/containerizer/mesos/provisioner/docker/message.proto (line 68)


Let's add a TODO as discussed here, that we should make protobuf::parse 
support nested parsing in the future.


- Timothy Chen


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

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

[Diana Arroyo]

> On Dec. 4, 2015, 9:48 p.m., Joseph Wu wrote:
> > Diana, can you add `hausdorff`, `kaysoky`, `hartem`, and `jvanremoortere` 
> > to this review (and future CMake reviews)?
> > 
> > ---
> > 
> > Ran `cmake .. && make` on OSX.  Hit this error:
> > ```
> > mesos/src/slave/containerizer/mesos/linux_launcher.cpp:20:10: fatal error: 
> > 'linux/sched.h' file not found
> > #include 
> > ```
> > 
> > Note that `src/Makefile.am` does this:
> > ```
> > if OS_LINUX
> > libmesos_no_3rdparty_la_SOURCES += $(MESOS_LINUX_FILES)
> > else
> > EXTRA_DIST += $(MESOS_LINUX_FILES)
> > endif
> > ```
> > And `linux_launcher.cpp` is in `MESOS_LINUX_FILES`.
> 
> Diana Arroyo wrote:
> Reference: "can you add hausdorff, kaysoky, hartem, and jvanremoortere to 
> this review (and future CMake reviews)?":  Done.
> Reference: "fatal error: 'linux/sched.h' file not found": working...

Reference: "fatal error: 'linux/sched.h' file not found": Fixed.


- Diana


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


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

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

[Diana Arroyo]

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

(Updated Dec. 5, 2015, 3:35 a.m.)


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


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


Repository: mesos


Description
---

Initial set of source files missing for cmake agent binary.


Diffs (updated)
-

  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 

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


Testing
---

Tested to make sure library builds successfully.


Thanks,

Diana Arroyo

Re: Review Request 40998: Reinstated previously reverted commit around CHECK's for ordered delivery of messages in Command Executor.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40998]

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

- Mesos ReviewBot


On Dec. 5, 2015, 12:19 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40998/
> ---
> 
> (Updated Dec. 5, 2015, 12:19 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change reinstates the previously reverted commit 
> `b6d4b28a4c9ca717ad8be5bbc27e40c005fc51ad`
> https://github.com/apache/mesos/commit/b6d4b28a4c9ca717ad8be5bbc27e40c005fc51ad#diff-3ddba917cc11631eb5ede254da408a27
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f90ea01131e6fa28e42f0c00a317d66a49f81ffa 
> 
> Diff: https://reviews.apache.org/r/40998/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


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

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40951]

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

- Mesos ReviewBot


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

Re: Review Request 40975: Document that libprocess ignores SIGPIPE

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40975]

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

- Mesos ReviewBot


On Dec. 4, 2015, 5:33 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40975/
> ---
> 
> (Updated Dec. 4, 2015, 5:33 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document that libprocess ignores SIGPIPE.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 
> 
> Diff: https://reviews.apache.org/r/40975/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

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. 4, 2015, 10:44 a.m.)


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


Repository: mesos


Description
---

Fixed protobuf parse failure when pulling a docker image.


Diffs (updated)
-

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

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


Testing
---

make check(ubuntu14.04 + clang3.6)


Thanks,

Gilbert Song

Re: Review Request 40953: Avoid accepting hex float literals

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 4, 2015, 5:23 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40953/
> ---
> 
> (Updated Dec. 4, 2015, 5:23 a.m.)
> 
> 
> 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
> 
>

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

[Joseph Wu]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Artem Harutyunyan.


Repository: mesos


Description
---

This addresses the comment here: 
https://reviews.apache.org/r/40501/#comment168391


Diffs
-

  src/tests/fetcher_tests.cpp 069c9bc6bc54d5bc1275c55e329457651d3c7b71 

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


Testing
---

make check


Thanks,

Joseph Wu

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

[Joseph Wu]

> On Dec. 3, 2015, 7:33 p.m., Ben Mahler wrote:
> > 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
> 
> Bernd Mathiske wrote:
> Makes sense, this will be better.

I'll open a separate review to fix this, because this commit wasn't reverted.

See: https://reviews.apache.org/r/40978/


- Joseph


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


On Nov. 19, 2015, 1: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, 1: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 40954: Fix comments in TaskHealthStatus message proto.

[Joseph Wu]

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

Ship it!



src/messages/messages.proto (line 666)


Typo: s/on each receive. If/On each receive, if/

Suggestion: s/the health check program failed consecutively that/the number 
of consecutive failed health checks/



src/messages/messages.proto (line 667)


Suggestion: insert "the" before "`kill_task`".



src/messages/messages.proto (line 678)


Typo: s/failure/failures/


- Joseph Wu


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 40951: Initial set of source files missing for cmake agent binary.

[Diana Arroyo]

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

(Updated Dec. 4, 2015, 6:30 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Initial set of source files missing for cmake agent binary.


Diffs
-

  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 

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


Testing
---

Tested to make sure library builds successfully.


Thanks,

Diana Arroyo

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

[Jojy Varghese]

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



src/slave/containerizer/mesos/provisioner/docker/message.proto (line 68)


Might be easy to read/understand if we name the fields as 
rawV1CompatibilityString and the the actual parsed field as v1Compatibility to 
match their semantics.


- Jojy Varghese


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

Re: Review Request 38117: Export per container SNMP statistics

[Cong Wang]

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

(Updated Dec. 4, 2015, 7:18 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Fix doc


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
  include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 
  src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

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

[Anand Mazumdar]

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

(Updated Dec. 4, 2015, 7:02 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
---

Review comments from Vinod


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

  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 38117: Export per container SNMP statistics

[Cong Wang]

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

(Updated Dec. 4, 2015, 7:01 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and add doc


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
  include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 
  src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

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. 4, 2015, 11:01 a.m.)


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


Repository: mesos


Description
---

Fixed protobuf parse failure when pulling a docker image.


Diffs (updated)
-

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

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


Testing
---

make check(ubuntu14.04 + clang3.6)


Thanks,

Gilbert Song