Re: Review Request 42040: Added Quota Operator Documentation.

2016-01-08 Thread Guangya Liu

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



docs/quota.md (line 20)


s/a cluster-wide dynamic reservation/a cluster-wide dynamic reservation 
without resource preemption

MESOS-1607 is introducing oversubscription for reservations, so the 
reservation resources can be preempted based on workload. Can we clarify it a 
bit here as above?



docs/quota.md (lines 33 - 34)


But we did have plan to introduce maximal limit in MESOS-3858 , can we 
calarify here?



docs/quota.md (lines 63 - 64)


s/quota set request/quota set request issued by operator



docs/quota.md (line 70)


s/to even/even ?



docs/quota.md (line 76)


s/scalar resources/unreserved and non-revocable scalar resourcess



docs/quota.md (line 153)


s/quota set until/quota set?



docs/quota.md (line 175)


Do we need to mention that the 80% is not configurable now?



docs/quota.md (line 177)


also not configurable now



docs/quota.md (line 227)


Add a link to implicit role?



docs/quota.md (lines 306 - 310)


1) what about adding "update Quota" here and remove it from L189
2) no Quota Limit but only guarantee

Another point is that shall we put quota design document link here to 
naviagte someone there if they want to get some design detail for Quota?


- Guangya Liu


On 一月 8, 2016, 10:26 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42040/
> ---
> 
> (Updated 一月 8, 2016, 10:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-3877
> https://issues.apache.org/jira/browse/MESOS-3877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Operator Documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md 6f0f4b9cb9d0da1f9960ebe7f36ce186c1317535 
>   docs/quota.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42040/diff/
> 
> 
> Testing
> ---
> 
> Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



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

2016-01-08 Thread Guangya Liu


> On 十二月 30, 2015, 12:42 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1334
> > 
> >
> > In this check, it seems allocator send offer when both !allocation & 
> > allocation are enough.
> 
> Joseph Wu wrote:
> Looks like quota might have stepped on the toes of oversubscription.  
> I'll double-check with Joris/AlexR.
> 
> Guangya Liu wrote:
> Not quite clear about the problem, can you elaborate? Joseph?
> 
> Klaus Ma wrote:
> @Joseph, the Quota only handle `unreserved.nonRevocable` resources; so 
> `USAGE_SLACK` are not included in Quota.
> 
> Joseph Wu wrote:
> Right.  The quota code may prevent the allocation of USAGE_SLACK.  When 
> quota is met, USAGE_SLACK will not be allocated (if the agent has 
> non-revocable available resources).
> 
> With your change, ALLOCATION_SLACK is allocated, even if quota is met.  
> This isn't your problem originally, but we'd want to address both types of 
> slack (or leave a TODO and address neither of them).

As non revocable resources are not counted for quota, so I think that we should 
allocate them in DRF stage without considering quota. The current patch is only 
handling allocation slack, I will add some TODO comments here to clarify that 
usage slack also needs to be considered here.


- Guangya


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


On 一月 8, 2016, 6:55 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated 一月 8, 2016, 6:55 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled oversubscribed resources for reservations in allocator.
> 
> There are three patches handling the allocator part:
> 1) This patch handles send offer, add slave and quota integration test.
> 2) https://reviews.apache.org/r/41847/ Handles the case of update slave.
> 3) https://reviews.apache.org/r/41791/ Handles the case of dynamic 
> reservations.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 42086: Updated and refactored Master::accept for Offers with InverseOffers.

2016-01-08 Thread Guangya Liu

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



src/master/master.cpp (lines 3054 - 3059)


Move this under L3050 to aggregate all offer validation operations.



src/master/validation.cpp (lines 678 - 709)


I saw that there is no test cases for offer validation, do you want to add 
inverseOffer test here?


- Guangya Liu


On 一月 9, 2016, 1:58 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42086/
> ---
> 
> (Updated 一月 9, 2016, 1:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4301
> https://issues.apache.org/jira/browse/MESOS-4301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds `validation::offer::inverse::validate` to validate inverse offers along 
> the same lines as `validation::offer::validate`, except `SlaveId` is not 
> validated for inverse offers.
> 
> Fixes and refactors `Master::accept` to allow `ACCEPT` calls that contain 
> both offers and inverse offers.
> Also tweaks `Master::accept` to not print a misleading log line "ACCEPT call 
> used invalid offers ..." when the call only includes inverse offers.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
> 
> Diff: https://reviews.apache.org/r/42086/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> # Ran the following and checked for blank output:
> bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null 
> |  grep "ACCEPT call used invalid offers"
> 
> # Check new test for flakiness:
> bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" 
> --gtest_repeat=1500 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Review Request 42100: Updated the jenkins build script to copy out xml testing reports.

2016-01-08 Thread Shuai Lin

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Updated the jenkins build script to copy out xml testing reports.


Diffs
-

  support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 

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


Testing
---


Thanks,

Shuai Lin



Re: Review Request 41820: Pulled out provisioner from linux filesystem isolator.

2016-01-08 Thread Gilbert Song

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

(Updated Jan. 8, 2016, 6:14 p.m.)


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


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


Repository: mesos


Description
---

Pulled out provisioner from linux filesystem isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
89d18624b08f2c953b9fa21663aeda694d4aac12 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
a29f9d184a0d1088577b3168a12bc8c06493a477 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
5bb85034c22caef64054c1629f6fd55d227e48b1 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 42086: Updated and refactored Master::accept for Offers with InverseOffers.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 5:58 p.m.)


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


Changes
---

Re-implemented the top of `Master::accept` based on what we (Joris/BenM/I) 
discussed offline.  This splits apart offers and inverse offers into separate 
arrays for validation and processing.


Summary (updated)
-

Updated and refactored Master::accept for Offers with InverseOffers.


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


Repository: mesos


Description (updated)
---

Adds `validation::offer::inverse::validate` to validate inverse offers along 
the same lines as `validation::offer::validate`, except `SlaveId` is not 
validated for inverse offers.

Fixes and refactors `Master::accept` to allow `ACCEPT` calls that contain both 
offers and inverse offers.
Also tweaks `Master::accept` to not print a misleading log line "ACCEPT call 
used invalid offers ..." when the call only includes inverse offers.


Diffs (updated)
-

  include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
  src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 

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


Testing
---

make check

# Ran the following and checked for blank output:
bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null |  
grep "ACCEPT call used invalid offers"

# Check new test for flakiness:
bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" --gtest_repeat=1500 
--gtest_break_on_failure


Thanks,

Joseph Wu



Re: Review Request 42086: Updated master's Offer validation to handle InverseOffers.

2016-01-08 Thread Joseph Wu


> On Jan. 8, 2016, 4:09 p.m., Klaus Ma wrote:
> > src/master/validation.cpp, line 597
> > 
> >
> > There're several similar code: 
> > if (condition) {
> > return Error();
> > }
> > 
> > Is it possible to introduce a macro, e.g.
> > RETURN_ERROR_IF(condition, "the error message of error");
> > 
> > With that macro, the code will be
> > 
> > InverseOffer* inverseOffer = ...
> > 
> > RETURN_ERROR_IF(inverseOffer == NULL, "Offer " + stringify(offerId) 
> > + " is no longer valid");
> > 
> > offerFrameworkId = inverseOffer->framework_id();

That certainly might be a nice convenience macro, but out of scope for this 
review.  If you'd like to, you can push a review that adds the macro (along 
with some example refactoring) and we can discuss it there.

I'll drop this issue for now.


- Joseph


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


On Jan. 8, 2016, 3:25 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42086/
> ---
> 
> (Updated Jan. 8, 2016, 3:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4301
> https://issues.apache.org/jira/browse/MESOS-4301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes `validation::offer::validate` to not error when a valid inverse offer 
> is validated.
> 
> Tweaks `Master::accept` to not print a misleading log line "ACCEPT call used 
> invalid offers ..." when the call only includes inverse offers.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
> 
> Diff: https://reviews.apache.org/r/42086/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> # Ran the following and checked for blank output:
> bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null 
> |  grep "ACCEPT call used invalid offers"
> 
> # Check new test for flakiness:
> bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" 
> --gtest_repeat=1500 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 40975: Document that libprocess ignores SIGPIPE.

2016-01-08 Thread Mesos ReviewBot

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


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 Jan. 8, 2016, 9:19 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40975/
> ---
> 
> (Updated Jan. 8, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document that libprocess ignores SIGPIPE.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/40975/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Review Request 42096: Fixed race in persistent volume tests.

2016-01-08 Thread Greg Mann

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed race in persistent volume tests.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
0a9a2bbd3b3a9c1bacf361750fae7a1970a622b7 

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


Testing
---

I was able to reproduce this failure by doing
`MESOS_VERBOSE=1 
GTEST_FILTER="PersistentVolumeTest.BadACLNoPrincipal:PersistentVolumeTest.BadACLDropCreateAndDestroy"
 bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1`
on Ubuntu 14.04. Unfortunately, the tests in question were relying on the 
`suppressOffers` and `reviveOffers` calls being reliably completed in a 
specified order, which is not the case. Inserting `Clock::settle()` after the 
appropriate `suppressOffers` calls allows those messages to be processed before 
`driver1` calls `reviveOffers`, ensuring that `driver1` can receive the 
resulting offers.

After applying the patch, I ran the above command on both Ubuntu 14.04 and OSX 
and it was successful.


Thanks,

Greg Mann



Re: Review Request 42084: Fixed header order.

2016-01-08 Thread Klaus Ma

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

Ship it!


Ship It!

- Klaus Ma


On Jan. 9, 2016, 4:08 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42084/
> ---
> 
> (Updated Jan. 9, 2016, 4:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rearranged headers to pull out the `#include ` into a separate 
> group.
> Pulled libprocess headers in front of stout headers in a few files.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/linux/ns.hpp 244a811b299c29b1dcd6652bd26e861e04df3f54 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
>   src/slave/containerizer/mesos/provisioner/appc/spec.cpp 
> 324cdfec3766da4a8e324378a6e413477fa2b5d9 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> b74c760e90b12f5bf45b6e626e2b661f841e5a8d 
>   src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 
> 
> Diff: https://reviews.apache.org/r/42084/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 37703: Add docker exec command.

2016-01-08 Thread Timothy Chen

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


I just ran the CheckExec test and it failed for me, can you try it yourself and 
see if you can repro?

- Timothy Chen


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



Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

2016-01-08 Thread Yi Sun

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

Ship it!


Ship It!


3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 55)


So the entry is always pointing to the allocated memory when success.


- Yi Sun


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `stout/os/windows/ls.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
> 5b6fba13ce215af5801fd0867f6e774e100689ca 
> 
> Diff: https://reviews.apache.org/r/39802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

2016-01-08 Thread Joris Van Remoortere

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

Ship it!


Regarding the snake_case, my comment was regarding the function and variable 
names, eg. `pathSize` -> `path_size`


3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 61 - 62)


I don't see the check for pathSize wrt. MAX_PATH.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 193 - 199)


`free` is meant to be safe for `NULL/nullptr`.
Why not follow this pattern?
asserting `directory != NULL` seems like it might surprise people?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 204)


why assert vs. false with errno?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 214)


are `null-terminated` right?
Same for the commeint in `_reentrantAdvanceDirStream`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 226)


same question as above.


- Joris Van Remoortere


On Dec. 23, 2015, 6:44 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Dec. 23, 2015, 6:44 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> d46e262e0fd1c2de36f3bf19d8bd693c23bf58cd 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 42093: Added proper handling of `None()` in the `CfsFilter`.

2016-01-08 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 8, 2016, 11:42 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42093/
> ---
> 
> (Updated Jan. 8, 2016, 11:42 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proper handling of `None()` in the `CfsFilter`. Previously, this test 
> assumed that `cgroups::hierarchy("cpu")` would either return `Some()` or an 
> `Error()`. However, the call may return `None()`, and this must be handled 
> properly.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 9d05630661bf6f20dbb492a27811933f65f3d14e 
> 
> Diff: https://reviews.apache.org/r/42093/diff/
> 
> 
> Testing
> ---
> 
> `sudo GTEST_FILTER="*CFS_*" bin/mesos-tests.sh` on Ubuntu 14.04, first with 
> the 'cpu' hierarchy mounted, then without the hierarchy mounted. When 
> properly mounted, the tests will run. When unmounted, the correct error 
> message is displayed corresponding to a `cgroups::hierarchy("cpu")` return 
> value of `None()`, and the tests are filtered out.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42084: Fixed header order.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42082, 42083, 42084]

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

- Mesos ReviewBot


On Jan. 8, 2016, 8:08 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42084/
> ---
> 
> (Updated Jan. 8, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rearranged headers to pull out the `#include ` into a separate 
> group.
> Pulled libprocess headers in front of stout headers in a few files.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/linux/ns.hpp 244a811b299c29b1dcd6652bd26e861e04df3f54 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
>   src/slave/containerizer/mesos/provisioner/appc/spec.cpp 
> 324cdfec3766da4a8e324378a6e413477fa2b5d9 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> b74c760e90b12f5bf45b6e626e2b661f841e5a8d 
>   src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 
> 
> Diff: https://reviews.apache.org/r/42084/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41962: Logger Module: Add tests for module recovery after agent failover.

2016-01-08 Thread Jojy Varghese


> On Jan. 6, 2016, 7:32 p.m., Jojy Varghese wrote:
> > src/tests/container_logger_tests.cpp, line 158
> > 
> >
> > Can we have this as :
> > 
> > ``` 
> > Shared docker(new MockDocker(...)); 
> > 
> > ```
> 
> Joseph Wu wrote:
> The trade-off (style-wise) is that later in the test, we'd need to do:
> ```
> EXPECT_CALL((MockDocker *) docker.get(), ps(_, _))
>   .WillOnce(Return(containers));
> ```
> 
> And I'm not really thrilled about that cast.

Ah i see. usually c++ style guides frown upon this style as its considered 
exception unsafe. using `make_shared` or `shared_ptr(new ...) ` is considered 
better as they guarantee exception safety. I guess for Mesos it doesnt matter.


- Jojy


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


On Jan. 9, 2016, 12:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41962/
> ---
> 
> (Updated Jan. 9, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Artem Harutyunyan, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds two heavily-mocked tests for the Mesos containerizer and Docker 
> containerizer.  Each checks that `ContainerLogger::recover` is called during 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/41962/diff/
> 
> 
> Testing
> ---
> 
> make (OSX & Centos7)
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41962: Logger Module: Add tests for module recovery after agent failover.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 4:24 p.m.)


Review request for mesos, Benjamin Hindman, Gilbert Song, Artem Harutyunyan, 
Jie Yu, and Timothy Chen.


Changes
---

Add check for `sandboxDirectory` in the mock tests.


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


Repository: mesos


Description
---

Adds two heavily-mocked tests for the Mesos containerizer and Docker 
containerizer.  Each checks that `ContainerLogger::recover` is called during 
`Containerizer::recover`.


Diffs (updated)
-

  src/tests/container_logger_tests.cpp c6b2e597517c74a55649287dc5ae5a3115f9a640 
  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 

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


Testing
---

make (OSX & Centos7)

Tests are run in the next review.


Thanks,

Joseph Wu



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Adam B


> On Jan. 6, 2016, 2:22 a.m., Joerg Schad wrote:
> > include/mesos/mesos.proto, line 482
> > 
> >
> > Let us doublecheck we actually need a deprecation cycle here: The 
> > critical scenario would be in my opinion if a new slaveInfo (i.e. without) 
> > reaches an old master trying to access this value.

We can address removing SlaveInfo.checkpoint in a separate patch. Although it 
has been forced to true for a few(?) releases already, SlaveInfo is still part 
of the API for various hooks/decorators as well as the executor 
[re]registered() APIs. Hooks must be compiled against the current version, so 
they'll update safely. Unfortunately, older executors may still be compiled 
against the old protobuf, which defaults 'checkpoint' to false if the field is 
missing. Perhaps we should change the default value to 'true' during the 
deprecation cycle? I wonder if any executors even care to look at the 
SlaveInfo.checkpoint field when they have MESOS_CHECKPOINT which should 
describe whether the _framework_ checkpoints.
Also, as you mention, the slave reports its entire SlaveInfo to the master, 
which doesn't act on SlaveInfo.checkpoint itself, but surfaces it in `/state` 
and `/slaves`, so external scripts could theoretically consume the field.
I bet with an email to dev@/user@ we could clear this up and get (lazy) 
consensus to remove the field from SlaveInfo, without a deprecation cycle.


- Adam


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


On Jan. 8, 2016, 4:46 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41649/
> ---
> 
> (Updated Jan. 8, 2016, 4:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2317
> https://issues.apache.org/jira/browse/MESOS-2317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slave checkpointing logic after deprecation cycle.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
>   src/examples/docker_no_executor_framework.cpp 
> 7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
>   src/examples/java/TestFramework.java 
> cbcdeedb0da22f82e274d088eb15a4128fd920fe 
>   src/examples/python/test_framework.py 
> 6af6d22f54c0ad702917d09cf658e70f7032d793 
>   src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
>   src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
>   src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/41649/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 41963: Logger Module: Implement ContainerLogger recovery.

2016-01-08 Thread Jojy Varghese


> On Jan. 6, 2016, 7:21 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/docker.cpp, line 701
> > 
> >
> > Do you need all the variables on the stack to be passed to the capture? 
> > Maybe just executorInfo ?
> 
> Joseph Wu wrote:
> The `[=]` should only be capturing `executorInfo`.

I believe that `[=]` directive is to `copy` all the variables on stack(and 
`this` to the capture. Its recommended to avoid this style and pick what you 
really need in captures (intent and not accident).


- Jojy


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


On Jan. 6, 2016, 2:19 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41963/
> ---
> 
> (Updated Jan. 6, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a call to `ContainerLogger::recover` inside each containerizer's 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
> 
> Diff: https://reviews.apache.org/r/41963/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX & Centos7)
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT*" (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 42086: Updated master's Offer validation to handle InverseOffers.

2016-01-08 Thread Klaus Ma

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



src/master/validation.cpp (line 597)


There're several similar code: 
if (condition) {
return Error();
}

Is it possible to introduce a macro, e.g.
RETURN_ERROR_IF(condition, "the error message of error");

With that macro, the code will be

InverseOffer* inverseOffer = ...

RETURN_ERROR_IF(inverseOffer == NULL, "Offer " + stringify(offerId) + " 
is no longer valid");

offerFrameworkId = inverseOffer->framework_id();


- Klaus Ma


On Jan. 9, 2016, 7:25 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42086/
> ---
> 
> (Updated Jan. 9, 2016, 7:25 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4301
> https://issues.apache.org/jira/browse/MESOS-4301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes `validation::offer::validate` to not error when a valid inverse offer 
> is validated.
> 
> Tweaks `Master::accept` to not print a misleading log line "ACCEPT call used 
> invalid offers ..." when the call only includes inverse offers.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
> 
> Diff: https://reviews.apache.org/r/42086/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> # Ran the following and checked for blank output:
> bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null 
> |  grep "ACCEPT call used invalid offers"
> 
> # Check new test for flakiness:
> bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" 
> --gtest_repeat=1500 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Adam B

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

Ship it!


Besides these two minor suggestions, I think this is commit-worthy.


src/master/allocator/mesos/hierarchical.hpp (line 284)


The hierarchicalDRF allocator shouldn't care about the framework's 
checkpointing state anymore, now that it's not comparing it against the slave's 
checkpoint variable to filter.



src/master/master.cpp (line 1110)


s/checkpointing//


- Adam B


On Jan. 8, 2016, 4:46 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41649/
> ---
> 
> (Updated Jan. 8, 2016, 4:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2317
> https://issues.apache.org/jira/browse/MESOS-2317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slave checkpointing logic after deprecation cycle.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
>   src/examples/docker_no_executor_framework.cpp 
> 7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
>   src/examples/java/TestFramework.java 
> cbcdeedb0da22f82e274d088eb15a4128fd920fe 
>   src/examples/python/test_framework.py 
> 6af6d22f54c0ad702917d09cf658e70f7032d793 
>   src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
>   src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
>   src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/41649/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42093: Added proper handling of `None()` in the `CfsFilter`.

2016-01-08 Thread Greg Mann

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

(Updated Jan. 8, 2016, 11:41 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Summary (updated)
-

Added proper handling of `None()` in the `CfsFilter`.


Repository: mesos


Description (updated)
---

Added proper handling of `None()` in the `CfsFilter`. Previously, this test 
assumed that `cgroups::hierarchy("cpu")` would either return `Some()` or an 
`Error()`. However, the call may return `None()`, and this must be handled 
properly.


Diffs
-

  src/tests/environment.cpp 9d05630661bf6f20dbb492a27811933f65f3d14e 

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


Testing
---

`sudo GTEST_FILTER="*CFS_*" bin/mesos-tests.sh` on Ubuntu 14.04, first with the 
'cpu' hierarchy mounted, then without the hierarchy mounted. When properly 
mounted, the tests will run. When unmounted, the correct error message is 
displayed corresponding to a `cgroups::hierarchy("cpu")` return value of 
`None()`, and the tests are filtered out.


Thanks,

Greg Mann



Review Request 42093: Added proper handling of 'None()' in the 'CfsFilter'.

2016-01-08 Thread Greg Mann

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

Review request for mesos, Ben Mahler and Timothy Chen.


Repository: mesos


Description
---

Added proper handling of `None()` in the 'CfsFilter'. Previously, this test 
assumed that `cgroups::hierarchy("cpu")` would either return `Some()` or an 
`Error()`. However, the call may return `None()`, and this must be handled 
properly.


Diffs
-

  src/tests/environment.cpp 9d05630661bf6f20dbb492a27811933f65f3d14e 

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


Testing
---

`sudo GTEST_FILTER="*CFS_*" bin/mesos-tests.sh` on Ubuntu 14.04, first with the 
'cpu' hierarchy mounted, then without the hierarchy mounted. When properly 
mounted, the tests will run. When unmounted, the correct error message is 
displayed corresponding to a `cgroups::hierarchy("cpu")` return value of 
`None()`, and the tests are filtered out.


Thanks,

Greg Mann



Review Request 42086: Updated master's Offer validation to handle InverseOffers.

2016-01-08 Thread Joseph Wu

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

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


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


Repository: mesos


Description
---

Changes `validation::offer::validate` to not error when a valid inverse offer 
is validated.

Tweaks `Master::accept` to not print a misleading log line "ACCEPT call used 
invalid offers ..." when the call only includes inverse offers.


Diffs
-

  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 

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


Testing
---

make check

# Ran the following and checked for blank output:
bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null |  
grep "ACCEPT call used invalid offers"

# Check new test for flakiness:
bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" --gtest_repeat=1500 
--gtest_break_on_failure


Thanks,

Joseph Wu



Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41002, 41003, 41004, 41061, 4, 41166, 41167, 41169, 
41560, 41294, 41370, 41378, 41779, 41780, 41781, 41782, 41783]

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

- Mesos ReviewBot


On Jan. 8, 2016, 8:06 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Jan. 8, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a non-default ContainerLogger that constrains total log size by rotating 
> logs (i.e. renaming the head log file).
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/rotate.hpp PRE-CREATION 
>   src/slave/container_loggers/rotate.cpp PRE-CREATION 
>   src/slave/container_loggers/rotating.hpp PRE-CREATION 
>   src/slave/container_loggers/rotating.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Review Request 42092: Add test for accept/decline offers and inverse offers.

2016-01-08 Thread Joseph Wu

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

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


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


Repository: mesos


Description
---

This tests `Call::ACCEPT` and `Call::DECLINE` that contain `OfferID` 's for 
both offers and inverse offers.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
2da6a2d4b786d9c5f64be8b5ab95f70ef3d98f92 

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


Testing
---

bin/mesos-tests.sh 
--gtest_filter="*MasterMaintenanceTest.OffersAndInverseOffers*" --verbose

This test currently fails with the following verbose logs:
```
I0108 14:54:43.211351 287354880 master.cpp:5360] Sending 1 offers to framework 
568b7754-9b13-42ef-8054-0a1e55a2a5a4- (default)
I0108 14:54:43.211797 287354880 master.cpp:5450] Sending 1 inverse offers to 
framework 568b7754-9b13-42ef-8054-0a1e55a2a5a4- (default)
I0108 14:54:43.236057 285208576 http.cpp:315] HTTP POST for 
/master/api/v1/scheduler from 10.0.79.8:62572
W0108 14:54:43.236501 285208576 master.cpp:3104] ACCEPT call used invalid 
offers '[ 568b7754-9b13-42ef-8054-0a1e55a2a5a4-O3, 
568b7754-9b13-42ef-8054-0a1e55a2a5a4-O4 ]': Offer 
568b7754-9b13-42ef-8054-0a1e55a2a5a4-O4 is no longer valid
I0108 14:54:43.236636 285208576 master.cpp:4838] Sending status update 
TASK_LOST) for task e8b41312-82fa-4886-b119-782761e73dd6 of framework 
568b7754-9b13-42ef-8054-0a1e55a2a5a4- 'Task launched with invalid offers: 
Offer 568b7754-9b13-42ef-8054-0a1e55a2a5a4-O4 is no longer valid'
../../src/tests/master_maintenance_tests.cpp:1567: Failure
Value of: event.get().update().status().state()
  Actual: TASK_LOST
Expected: v1::TASK_RUNNING
Which is: TASK_RUNNING
```


Thanks,

Joseph Wu



Re: Review Request 41962: Logger Module: Add tests for module recovery after agent failover.

2016-01-08 Thread Joseph Wu


> On Jan. 8, 2016, 1:29 p.m., Timothy Chen wrote:
> > src/tests/container_logger_tests.cpp, line 222
> > 
> >
> > Do we need to check that the correct container id is passed?

It wouldn't hurt.  I'll add it to both tests.

The second argument here is a the string `sandboxDirectory`, which is expected 
to be:
```
  slave::paths::getExecutorRunPath(
  flags.work_dir,
  slaveState.id,
  frameworkState.id,
  executorId,
  containerId)
```


- Joseph


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


On Jan. 5, 2016, 6:19 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41962/
> ---
> 
> (Updated Jan. 5, 2016, 6:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Artem Harutyunyan, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds two heavily-mocked tests for the Mesos containerizer and Docker 
> containerizer.  Each checks that `ContainerLogger::recover` is called during 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41962/diff/
> 
> 
> Testing
> ---
> 
> make (OSX & Centos7)
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Adam B

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


Looks pretty good to me except for the potential bug in devolve (although I 
don't think we even use SlaveInfo.checkpoint for anything anymore). I still 
want to do a pass over the code to see if there's anything else we ought to 
remove.


src/internal/devolve.cpp 


BUG? Wouldn't you still need to `set_checkpoint(true)`, since the 
SlaveInfo.checkpoint protobuf defaults to false, and AgentInfo doesn't have 
'checkpoint'?



src/master/master.cpp (lines 1073 - 1075)


"The semantics when a registered slave gets disconnected are as follows for 
each framework running on that slave:"
Actually, this whole comment block probably makes more sense above the 
foreach(frameworkIds) loop, where that "Remove all non-checkpointing 
frameworks" comment lives now



src/master/master.cpp (line 1088)


Unnecessary comment?


- Adam B


On Jan. 8, 2016, 4:46 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41649/
> ---
> 
> (Updated Jan. 8, 2016, 4:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2317
> https://issues.apache.org/jira/browse/MESOS-2317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slave checkpointing logic after deprecation cycle.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
>   src/examples/docker_no_executor_framework.cpp 
> 7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
>   src/examples/java/TestFramework.java 
> cbcdeedb0da22f82e274d088eb15a4128fd920fe 
>   src/examples/python/test_framework.py 
> 6af6d22f54c0ad702917d09cf658e70f7032d793 
>   src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
>   src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
>   src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/41649/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 41461: Added SFINAE-friendly `result_of` to stout.

2016-01-08 Thread Michael Park

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

(Updated Jan. 8, 2016, 10:07 p.m.)


Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
Remoortere.


Changes
---

Addressed BenH's offline comments.


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


Repository: mesos


Description
---

VS 2015 won't support C++14 `std::result_of` SFINAE until Update 2, so 
`result_of` must be replaced with `decltype(invoke)`.

Here, we implement SFINAE `result_of` in `stout`.

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp PRE-CREATION 

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


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park



Re: Review Request 42053: Add flags to set size of completed task/framework history.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42053]

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

- Mesos ReviewBot


On Jan. 8, 2016, 5:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42053/
> ---
> 
> (Updated Jan. 8, 2016, 5:47 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3307
> https://issues.apache.org/jira/browse/MESOS-3307
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default size of the buffers used to hold the state of completed
> tasks/frameworks is very large. However, many frameworks don't care much
> about this information when requesting a master's state. Moreover, if a
> large number of frameworks request this state simultaneously, the
> master can quickly become overwhelmed because the process of generating
> this state both blocks the master and takes up a lot of cycles. By
> allowing the master to configure the size of the buffers used to hold
> this state, we give it the power to significantly reduce the amount of
> state it needs to maintain.
> 
> This change allows the master to limit the size of this state via
> command line flags.
> 
> This commit is based on a pull request generated by Felix Bechstein at:
> https://github.com/apache/mesos/pull/82
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840 
>   src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0 
>   src/master/master.cpp 145c5932610bd019eb090947569b608df6815c3a 
> 
> Diff: https://reviews.apache.org/r/42053/diff/
> 
> 
> Testing
> ---
> 
> On Darwin I launched a master with:
> 
> ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos 
> --max_completed_tasks_per_framework=2 --max_completed_frameworks=1
> 
> and a slave with:
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050
> 
> and then ran a bunch of instances of:
> 
> ./src/test-framework --master=127.0.0.1:5050
> 
> each of which runs 5 tasks to completion
> 
> I then ran:
> 
> curl http://localhost:5050/tasks
> 
> and verified that only 1 framework and 2 of its completed tasks were given 
> back to me in the json that was returned.  I repeated this for a number of 
> other configurations with max_completed_frameworks=0..2 and 
> max_completed_tasks_per_framework=0..5 and verified visually that the proper 
> number of tasks/frameworks were being returned by the /tasks endpoint.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 37075: Protobuf definitions instructing the fetcher cache about checksums and their validation.

2016-01-08 Thread Timothy Chen

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


Is this still relevant?

- Timothy Chen


On Aug. 5, 2015, 11:37 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37075/
> ---
> 
> (Updated Aug. 5, 2015, 11:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-3208
> https://issues.apache.org/jira/browse/MESOS-3208
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Protobuf definitions instructing the fetcher cache about checksums and their 
> validation. First in a series of patches implementing phase 1 of MESOS-2073 
> (see comments in the ticket). There are references to docs in this patch, but 
> updating docs will be in the last patch.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba 
> 
> Diff: https://reviews.apache.org/r/37075/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>



Re: Review Request 34140: AppC image store

2016-01-08 Thread Timothy Chen

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


Can we close this now? I don't think this is relevant anymore.

- Timothy Chen


On July 7, 2015, 7:43 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34140/
> ---
> 
> (Updated July 7, 2015, 7:43 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Images are fetched into the store (after discovery). Stored images are
> currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
> 
> Diff: https://reviews.apache.org/r/34140/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>



Re: Review Request 41810: Added credential to Docker image protobuf.

2016-01-08 Thread Jie Yu

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

Ship it!



include/mesos/mesos.proto (line 1318)


Would you please mention that it's not encrypted.  Operators should enable 
SSL to avoid leaking that information.


- Jie Yu


On Dec. 30, 2015, 11:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41810/
> ---
> 
> (Updated Dec. 30, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added credential to Docker image protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
> 
> Diff: https://reviews.apache.org/r/41810/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 41956: Removed an unnecessary os::exists check in curl URI fetcher plugin.

2016-01-08 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 6, 2016, 1:31 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41956/
> ---
> 
> (Updated Jan. 6, 2016, 1:31 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an unnecessary os::exists check in curl URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/curl.cpp 0ff9833dc5ca53ddc137fd2013b4dc5420ae4eb1 
> 
> Diff: https://reviews.apache.org/r/41956/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41957: Style fix for curl URI fetcher plugin.

2016-01-08 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 6, 2016, 1:32 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41957/
> ---
> 
> (Updated Jan. 6, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Style fix for curl URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/curl.cpp 0ff9833dc5ca53ddc137fd2013b4dc5420ae4eb1 
> 
> Diff: https://reviews.apache.org/r/41957/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41960: Added the Docker URI fetcher plugin interface.

2016-01-08 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 6, 2016, 1:43 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41960/
> ---
> 
> (Updated Jan. 6, 2016, 1:43 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Vinod Kone.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the Docker URI fetcher plugin interface. See the motivation in the 
> ticket.
> 
> This patch just introduces the interface. The implementation will be added in 
> the subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 865926c5b46e42c5e29d3645a700c4ad20c1f11d 
>   src/uri/fetcher.hpp d5182a51809b901b5c22dc66cd48e0068caf89a0 
>   src/uri/fetcher.cpp ac13fbdc7399045d183cbdcc48dc5cf9969e8ad5 
>   src/uri/fetchers/docker.hpp PRE-CREATION 
>   src/uri/fetchers/docker.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41960/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41961: Added an HTTP decode response method.

2016-01-08 Thread Timothy Chen

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



3rdparty/libprocess/src/http.cpp (line 707)


Why don't we return the failure message?


- Timothy Chen


On Jan. 6, 2016, 1:44 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41961/
> ---
> 
> (Updated Jan. 6, 2016, 1:44 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an HTTP decode response method. This will be used to decode the 
> response returned from curl.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2 
> 
> Diff: https://reviews.apache.org/r/41961/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41961: Added an HTTP decode response method.

2016-01-08 Thread Timothy Chen

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



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


bmahler might have different say on this, as he was proposing that the 
comment style should stay the same with in the file for now. I don't know if 
that changed?



3rdparty/libprocess/src/http.cpp (line 714)


Should we also comment that we only return the first response even if there 
are multiples?


- Timothy Chen


On Jan. 6, 2016, 1:44 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41961/
> ---
> 
> (Updated Jan. 6, 2016, 1:44 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an HTTP decode response method. This will be used to decode the 
> response returned from curl.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2 
> 
> Diff: https://reviews.apache.org/r/41961/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41962: Logger Module: Add tests for module recovery after agent failover.

2016-01-08 Thread Timothy Chen

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



src/tests/container_logger_tests.cpp (line 222)


Do we need to check that the correct container id is passed?


- Timothy Chen


On Jan. 6, 2016, 2:19 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41962/
> ---
> 
> (Updated Jan. 6, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Artem Harutyunyan, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds two heavily-mocked tests for the Mesos containerizer and Docker 
> containerizer.  Each checks that `ContainerLogger::recover` is called during 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41962/diff/
> 
> 
> Testing
> ---
> 
> make (OSX & Centos7)
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 42011: Added gtest flags to generate xml report.

2016-01-08 Thread Ben Mahler

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

Ship it!


Thank you! This is very helpful to the project. We'll get this committed 
shortly.


support/docker_build.sh (line 70)


I'll add a period here.


- Ben Mahler


On Jan. 7, 2016, 1:55 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42011/
> ---
> 
> (Updated Jan. 7, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-4258
> https://issues.apache.org/jira/browse/MESOS-4258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added gtest flags to generate xml report.
> 
> 
> Diffs
> -
> 
>   Makefile.am d9604f48f194dc8272423410e9938e646f334a9c 
>   support/docker_build.sh 7882bd20e4b6ee28dc117086386de8faac995a3e 
> 
> Diff: https://reviews.apache.org/r/42011/diff/
> 
> 
> Testing
> ---
> 
> $ export GTEST_OUTPUT=xml:report.xml
> $ make check
> $ find . -name report.xml 
>   
>  
> ./3rdparty/libprocess/report.xml
> ./3rdparty/libprocess/3rdparty/report.xml
> ./src/report.xml
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 41429: Cleaned up the CfsFilter and clarified its logging message.

2016-01-08 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 8, 2016, 4:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> ---
> 
> (Updated Jan. 8, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to 
> @bmahler for noticing these issues!
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 
> 
> Diff: https://reviews.apache.org/r/41429/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run 
> correctly on Ubuntu, while the appropriate error message is displayed on 
> Debian and the "CFS_" tests are filtered out.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Timothy Chen

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



3rdparty/libprocess/README.md (line 257)


IMO there seems to be too much irrelevant code to make the point, can we 
collapse all the unnecessary code and just keep the ones that can illustrate 
the problem?


- Timothy Chen


On Jan. 8, 2016, 5:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42030/
> ---
> 
> (Updated Jan. 8, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4207
> https://issues.apache.org/jira/browse/MESOS-4207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added example of a `defer` bug to libprocess README.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/42030/diff/
> 
> 
> Testing
> ---
> 
> Viewed the markdown in a github branch: 
> https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess
> 
> Note that in addition to the inclusion of this example, further changes to 
> the `defer` documentation preceding the example are currently being 
> considered: https://reviews.apache.org/r/41618/
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 40975: Document that libprocess ignores SIGPIPE.

2016-01-08 Thread James Peach

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

(Updated Jan. 8, 2016, 9:19 p.m.)


Review request for mesos and Ben Mahler.


Summary (updated)
-

Document that libprocess ignores SIGPIPE.


Repository: mesos


Description
---

Document that libprocess ignores SIGPIPE.


Diffs
-

  3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 

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


Testing
---

None.


Thanks,

James Peach



Re: Review Request 40975: Document that libprocess ignores SIGPIPE

2016-01-08 Thread Joseph Wu


> On Jan. 7, 2016, 10:32 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [40975]
> > 
> > Failed command: ./support/apply-review.sh -n -r 40975
> > 
> > Error:
> >  2016-01-07 18:32:55 URL:https://reviews.apache.org/r/40975/diff/raw/ 
> > [1164/1164] -> "40975.patch" [1]
> > No files to lint
> > 
> > Error: Commit message summary (the first line) must end in a period.
> 
> James Peach wrote:
> I don't know how to fix this. The subject is correct in the commit I am 
> pushing; I don't know how to convince the tooling of that.

`support/post-reviews.sh` doesn't overwrite the review board "Summary" (so if 
you've added the period there, ReviewBot will still complain).  You can just 
edit the field on the web UI.


- Joseph


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


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



Re: Review Request 40975: Document that libprocess ignores SIGPIPE

2016-01-08 Thread James Peach

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

(Updated Jan. 8, 2016, 8:58 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Added backticks around EPIPE.


Repository: mesos


Description
---

Document that libprocess ignores SIGPIPE.


Diffs (updated)
-

  3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 

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


Testing
---

None.


Thanks,

James Peach



Re: Review Request 40975: Document that libprocess ignores SIGPIPE

2016-01-08 Thread James Peach


> On Jan. 7, 2016, 6:32 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [40975]
> > 
> > Failed command: ./support/apply-review.sh -n -r 40975
> > 
> > Error:
> >  2016-01-07 18:32:55 URL:https://reviews.apache.org/r/40975/diff/raw/ 
> > [1164/1164] -> "40975.patch" [1]
> > No files to lint
> > 
> > Error: Commit message summary (the first line) must end in a period.

I don't know how to fix this. The subject is correct in the commit I am 
pushing; I don't know how to convince the tooling of that.


- James


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


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



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42030]

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

- Mesos ReviewBot


On Jan. 8, 2016, 5:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42030/
> ---
> 
> (Updated Jan. 8, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4207
> https://issues.apache.org/jira/browse/MESOS-4207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added example of a `defer` bug to libprocess README.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/42030/diff/
> 
> 
> Testing
> ---
> 
> Viewed the markdown in a github branch: 
> https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess
> 
> Note that in addition to the inclusion of this example, further changes to 
> the `defer` documentation preceding the example are currently being 
> considered: https://reviews.apache.org/r/41618/
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 42084: Fixed header order.

2016-01-08 Thread Joseph Wu

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Repository: mesos


Description
---

Rearranged headers to pull out the `#include ` into a separate 
group.
Pulled libprocess headers in front of stout headers in a few files.


Diffs
-

  src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
  src/linux/ns.hpp 244a811b299c29b1dcd6652bd26e861e04df3f54 
  src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
  src/slave/containerizer/mesos/provisioner/appc/spec.cpp 
324cdfec3766da4a8e324378a6e413477fa2b5d9 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
b74c760e90b12f5bf45b6e626e2b661f841e5a8d 
  src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
  src/tests/containerizer/provisioner_backend_tests.cpp 
25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 

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


Testing
---

make check


Thanks,

Joseph Wu



Review Request 42083: Fixed header order in libprocess.

2016-01-08 Thread Joseph Wu

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Repository: mesos


Description
---

Rearranged headers to pull out the `#include ` into a separate 
group.


Diffs
-

  3rdparty/libprocess/src/io.cpp 9530192e2c7afbfa4bd9aa2af2ff40f00ee505a9 
  3rdparty/libprocess/src/process.cpp 7fd278a3672f5f57e667520e91dbaa30eb16c3ed 
  3rdparty/libprocess/src/subprocess.cpp 
863523404b9ef1d2024c108f3e9e1c77c3916049 
  3rdparty/libprocess/src/tests/reap_tests.cpp 
0cbe9f139d59ac154695d84638c551fbe098cb10 

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


Testing
---

make check


Thanks,

Joseph Wu



Review Request 42082: Fixed header order in stout.

2016-01-08 Thread Joseph Wu

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Repository: mesos


Description
---

Rearranged headers to pull out the `#include ` into a separate 
group.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
ffeb2d721149f600f552021c127694183f1aa837 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
11557e3938d23ae253dd4d4eade85600123e0b22 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
293f82f6730551491504721e0af28e9537540db1 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99 

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


Testing
---

make check


Thanks,

Joseph Wu



Re: Review Request 41782: Logger Module: Wire up the rotating container logger module and test.

2016-01-08 Thread Joseph Wu


> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 111
> > 
> >
> > s/sandboxPath/sandboxDirectory/
> > 
> > ?

This one points to the sandbox module (the default one).  I'll rename to 
`sandboxLoggerPath`.


> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 135
> > 
> >
> > Interesting that you pulled it out as a constant in the previous 
> > review, but not here?

I was trying to stay consistent within the `module.cpp` file.


> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, lines 152-154
> > 
> >
> > Is this a dead flag?

This flag tells the logger module to print its `stderr` to the given file.  I 
added the while debugging, and kept it because it makes sense to have an option 
to *not* silence the logger's output.  

(The logger can't write errors to the log it's writing to, because the errors 
it reports are due to writing to the log.)


> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 125
> > 
> >
> > I'm confused by the name of the variable `truncatingPath` ... is this a 
> > remanant of the old code and should be `rotatingContainerLoggerPath`?

Oops.  Yes that's a remnant.


- Joseph


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


On Jan. 8, 2016, 12:06 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41782/
> ---
> 
> (Updated Jan. 8, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Creates a new binary "mesos-rotate-logger" for use by the non-default 
> "Rotating" ContainerLogger module.
> Adds the `RotatingContainerLogger` to the test module configuration.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8 
>   src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c 
>   src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 
> 
> Diff: https://reviews.apache.org/r/41782/diff/
> 
> 
> Testing
> ---
> 
> Note: Some of the files added to the makefile are created in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 12:06 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Fix module cleanup on failure in `prepare`.  Fix header order, some spacing.


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


Repository: mesos


Description
---

Adds a non-default ContainerLogger that constrains total log size by rotating 
logs (i.e. renaming the head log file).


Diffs (updated)
-

  src/slave/container_loggers/rotate.hpp PRE-CREATION 
  src/slave/container_loggers/rotate.cpp PRE-CREATION 
  src/slave/container_loggers/rotating.hpp PRE-CREATION 
  src/slave/container_loggers/rotating.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joseph Wu



Re: Review Request 41781: Logger Module: Add test for the rotating container logger module.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 12:06 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Fixed header order, some comments, and some variables names.


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


Repository: mesos


Description
---

This test loads a non-default ContainerLogger module that rotates logs (i.e. 
renaming the head log file) and constrains total log size.


Diffs (updated)
-

  src/tests/container_logger_tests.cpp c6b2e597517c74a55649287dc5ae5a3115f9a640 

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


Testing
---

This test is run later in the review chain.


Thanks,

Joseph Wu



Re: Review Request 41782: Logger Module: Wire up the rotating container logger module and test.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 12:06 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Fixed some spacing, comments, and variable names.


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


Repository: mesos


Description
---

Creates a new binary "mesos-rotate-logger" for use by the non-default 
"Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.


Diffs (updated)
-

  src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8 
  src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c 
  src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 

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


Testing
---

Note: Some of the files added to the makefile are created in the next review.


Thanks,

Joseph Wu



Re: Review Request 41781: Logger Module: Add test for the rotating container logger module.

2016-01-08 Thread Joseph Wu


> On Jan. 7, 2016, 4:12 p.m., Benjamin Hindman wrote:
> > src/tests/container_logger_tests.cpp, lines 28-30
> > 
> >
> > How come these aren't pulled out since they are three levels?

Looks like we're a bit inconsistent.  I'll fix this here and submit a separate 
set of patches to tweak this in other places.
https://reviews.apache.org/r/42082/
https://reviews.apache.org/r/42083/
https://reviews.apache.org/r/42084/


- Joseph


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


On Jan. 8, 2016, 12:06 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41781/
> ---
> 
> (Updated Jan. 8, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test loads a non-default ContainerLogger module that rotates logs (i.e. 
> renaming the head log file) and constrains total log size.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
> 
> Diff: https://reviews.apache.org/r/41781/diff/
> 
> 
> Testing
> ---
> 
> This test is run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41429: Cleaned up the CfsFilter and clarified its logging message.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41429]

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

- Mesos ReviewBot


On Jan. 8, 2016, 4:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> ---
> 
> (Updated Jan. 8, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to 
> @bmahler for noticing these issues!
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 
> 
> Diff: https://reviews.apache.org/r/41429/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run 
> correctly on Ubuntu, while the appropriate error message is displayed on 
> Debian and the "CFS_" tests are filtered out.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42059: Updated ContainerLogger to use Subprocess::IO type.

2016-01-08 Thread Joseph Wu

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

(Updated Jan. 8, 2016, 10:28 a.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Missed a period in the summary.


Summary (updated)
-

Updated ContainerLogger to use Subprocess::IO type.


Repository: mesos


Description
---

Update ContainerLogger to use Subprocess::IO type


Diffs
-

  include/mesos/slave/container_logger.hpp 
a2362070ead0afcef1e6c2ca784083ddb01ba51a 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 

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


Testing
---

make check


Thanks,

Joseph Wu



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

2016-01-08 Thread Joseph Wu


> On Dec. 30, 2015, 4:42 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1334
> > 
> >
> > In this check, it seems allocator send offer when both !allocation & 
> > allocation are enough.
> 
> Joseph Wu wrote:
> Looks like quota might have stepped on the toes of oversubscription.  
> I'll double-check with Joris/AlexR.
> 
> Guangya Liu wrote:
> Not quite clear about the problem, can you elaborate? Joseph?
> 
> Klaus Ma wrote:
> @Joseph, the Quota only handle `unreserved.nonRevocable` resources; so 
> `USAGE_SLACK` are not included in Quota.

Right.  The quota code may prevent the allocation of USAGE_SLACK.  When quota 
is met, USAGE_SLACK will not be allocated (if the agent has non-revocable 
available resources).

With your change, ALLOCATION_SLACK is allocated, even if quota is met.  This 
isn't your problem originally, but we'd want to address both types of slack (or 
leave a TODO and address neither of them).


- Joseph


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


On Jan. 7, 2016, 10:55 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Jan. 7, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled oversubscribed resources for reservations in allocator.
> 
> There are three patches handling the allocator part:
> 1) This patch handles send offer, add slave and quota integration test.
> 2) https://reviews.apache.org/r/41847/ Handles the case of update slave.
> 3) https://reviews.apache.org/r/41791/ Handles the case of dynamic 
> reservations.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 41567: Clarified a comment that occurs in several test cases.

2016-01-08 Thread Neil Conway

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

(Updated Jan. 8, 2016, 6:22 p.m.)


Review request for mesos and Adam B.


Changes
---

Rebase, remove parent patch dependency.


Repository: mesos


Description
---

Clarified a comment that occurs in several test cases.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 
  src/tests/reservation_tests.cpp 69ddf906f2041bba84d7e1a20f90ee18817b4e64 
  src/tests/role_tests.cpp 91a6d27158a21ae11dd310dd1f55922dc147e6f3 

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


Testing
---


Thanks,

Neil Conway



Re: Review Request 41819: Added ContainerConfig to all isolators.

2016-01-08 Thread Gilbert Song


> On Jan. 7, 2016, 4:16 p.m., Jie Yu wrote:
> >

Fixed at `41816`.


- Gilbert


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


On Jan. 6, 2016, 3:41 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41819/
> ---
> 
> (Updated Jan. 6, 2016, 3:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ContainerConfig to all isolators.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/isolator.cpp 
> 493b5dd26cf0e8f986381a502cfa6d1dde6573d4 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
> 123b9ed3ccaebcd5da24fc62ff7a92d4a81ed760 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 3b95e195ad704f163c245175390d9a26bde7e17c 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
> 09952369c72d3c6322ae7a1c73cd68226d452ad2 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> 2ddb9f4adbb879682cd39966ab974cf3fa32209c 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 5eaf49f1f35c93ad4465adb6c9c9cf57b3a2c6ee 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 4d82c2b2f231c59cbb600869a0f2b716c1e55f5e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> a29f9d184a0d1088577b3168a12bc8c06493a477 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> c3544aa313cbb185efb03bba59961cdf2b616a37 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 00ff84b6cd0aa29fa5a7918d7f88d480af8752ca 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
> 2e457015a0348a457581edf493877b71fab17090 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 361ed6561bd5e2f75d026922def01f42b43d61c2 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> c2d1455249618f9cd2e17dc2244b184d52b32eaf 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> d65c1593b44f4b21237581147e57e441ebf3160d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 22a8428427b758bae4a0518356d7933c4110cd9f 
>   src/slave/containerizer/mesos/isolators/posix.hpp 
> 7e1ebc2fada5a5e291e84c7044bdba9a71f4b42c 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> 31808c1e8199fbf2cea36c273860fdbf0a2388f8 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 248c34adb63907911d89bed5b1519682a852bb2d 
> 
> Diff: https://reviews.apache.org/r/41819/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 41817: Changed slave isolator interface and updated correspondings.

2016-01-08 Thread Gilbert Song

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

(Updated Jan. 8, 2016, 10:18 a.m.)


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


Changes
---

Shorten commit message.


Summary (updated)
-

Changed slave isolator interface and updated correspondings.


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


Repository: mesos


Description (updated)
---

Changed slave isolator interface and updated correspondings.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 95a2933988ea7c9b9404df5e12031f134712b2b5 
  src/slave/containerizer/mesos/isolator.hpp 
937f253656d36ed10b47ceeb0b6101f212e65586 
  src/slave/containerizer/mesos/isolator.cpp 
493b5dd26cf0e8f986381a502cfa6d1dde6573d4 
  src/tests/containerizer/isolator.hpp e4101b188560bd857ea104f61f52f27c880e7731 
  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
95b493c6f479eef52ee0c9a44ac40254ed76ebae 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41814: Unified Container: Created ContainerConfig protobuf.

2016-01-08 Thread Gilbert Song

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

(Updated Jan. 8, 2016, 10:17 a.m.)


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


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


Repository: mesos


Description
---

Unified Container: Created ContainerConfig protobuf.


Diffs (updated)
-

  include/mesos/slave/isolator.proto d2032adf9336119ed8e1ff3c813d657d70331b67 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41816: Generated ContainerConfig in mesos containerizer.

2016-01-08 Thread Gilbert Song

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

(Updated Jan. 8, 2016, 10:17 a.m.)


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


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


Repository: mesos


Description
---

Generated ContainerConfig in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
89d18624b08f2c953b9fa21663aeda694d4aac12 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41567: Clarified a comment that occurs in several test cases.

2016-01-08 Thread Neil Conway


> On Jan. 8, 2016, 9:08 a.m., Adam B wrote:
> > Neil, do you want to rebase this or discard it, since you've discarded the 
> > parent patch?

Will rebase! Sorry, didn't realize it was a child patch.


- Neil


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


On Dec. 18, 2015, 9:30 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41567/
> ---
> 
> (Updated Dec. 18, 2015, 9:30 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified a comment that occurs in several test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 
>   src/tests/reservation_tests.cpp f429472e6b93a5d6d8fe6a5f7d5b94fc331f7295 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41567/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 42040: Added Quota Operator Documentation.

2016-01-08 Thread Neil Conway


> On Jan. 7, 2016, 11:55 p.m., Neil Conway wrote:
> > docs/quota.md, line 226
> > 
> >
> > Fix this link -- probably just link to roles.md. Although we don't 
> > really call the feature "implicit roles" in the user-facing docs (we just 
> > talk about whether a "role whitelist" has been configured), so maybe we can 
> > just remove this.
> 
> Alexander Rukletsov wrote:
> Though it indeed seems weird to mention random features in this user doc, 
> I would love to mention implicit roles somewhere here. The motivation is that 
> it's a new (read: relatively unknown) feature, which is highly related to 
> quota. I would like to make sure an operator reading this doc is aware of the 
> feature and can use it appropriately. The best solution would be to add a 
> `NOTE` or a sentence and remove it in ~6 month, when implicit roles become 
> standard. Unfortunately, we do not support remove `TODO`s in user docs : )

Maybe we can rephrase this to talk in terms of whether a "role whitelist" has 
been configured?


> On Jan. 7, 2016, 11:55 p.m., Neil Conway wrote:
> > docs/quota.md, line 61
> > 
> >
> > These kinds of implementation details belong at the bottom of the 
> > document, I think -- it is more important to tell the user/operator how to 
> > define quota before we worry about allocator details.
> > 
> > We could also remove a lot of this information -- the specific steps we 
> > take to implement a set/remove quota request are not an important thing to 
> > document (and might change over time).
> 
> Alexander Rukletsov wrote:
> Our intention was *not* to provide implementation details, but to mention 
> some of those, which are important and affect cluster behaviour. There are 3 
> things: capacity heuristic, rescinding offers, allocator logic (quota first, 
> fair share second). I do think we should explain operators how these things 
> work so that they don't break their clusters.
> 
> However, if the section feels like an "implementation details" section, 
> we didn't achieve our goal. The reason we decided to briefly mention other 
> stages is in order not to be misleading and leave an impression, that there 
> is nothing else.
> 
> Do you think leaving only these 3 sections and adding a sentence that 
> those are most important stages in quota processing is a good compromise?

I think we could improve things in two ways:

1. Move the "How does it work?" section to the end of the document. Although 
this is useful material, it is fairly advanced; a lot of users will want to 
just know how to configure/examine quota.
2. Remove some of the implementation details. For example, the fact that we 
authenticate, parse, validate, and authorize a quota request is not important 
for the information we want to convey to operators about how quotas behave.


- Neil


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


On Jan. 8, 2016, 10:26 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42040/
> ---
> 
> (Updated Jan. 8, 2016, 10:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-3877
> https://issues.apache.org/jira/browse/MESOS-3877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Operator Documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md 6f0f4b9cb9d0da1f9960ebe7f36ce186c1317535 
>   docs/quota.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42040/diff/
> 
> 
> Testing
> ---
> 
> Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42040: Added Quota Operator Documentation.

2016-01-08 Thread Neil Conway

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



docs/home.md (line 42)


Typo



docs/quota.md (line 109)


"quota of 1000 CPUs"



docs/quota.md (line 117)


I think "non-static cluster resources" is not quite accurate here. We care 
about whether the total quota exceeds the "total non-statically-reserved 
cluster resources", or something like that (we need to say "static 
reservation", not "static cluster resource").



docs/quota.md (line 124)


"configured quotas"



docs/quota.md (line 126)


"The force flag".

Period after "this check". I'd also rephrase slightly: "For example, this 
flag can be useful when the operator would like to configure a quota that 
exceeds the current cluster capacity, but they know that additional cluster 
resources will be added shortly."



docs/quota.md (line 170)


"are detected"



docs/quota.md (line 175)


"reregister"



docs/quota.md (line 176)


add "or" at the end of the first bullet point


- Neil Conway


On Jan. 8, 2016, 10:26 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42040/
> ---
> 
> (Updated Jan. 8, 2016, 10:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-3877
> https://issues.apache.org/jira/browse/MESOS-3877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Operator Documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md 6f0f4b9cb9d0da1f9960ebe7f36ce186c1317535 
>   docs/quota.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42040/diff/
> 
> 
> Testing
> ---
> 
> Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42053: Add flags to set size of completed task/framework history.

2016-01-08 Thread Kevin Klues

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

(Updated Jan. 8, 2016, 5:47 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

This change doesn't resolve any issues and simply fixes a merge conflict due to 
added code via a rebase.


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


Repository: mesos


Description
---

The default size of the buffers used to hold the state of completed
tasks/frameworks is very large. However, many frameworks don't care much
about this information when requesting a master's state. Moreover, if a
large number of frameworks request this state simultaneously, the
master can quickly become overwhelmed because the process of generating
this state both blocks the master and takes up a lot of cycles. By
allowing the master to configure the size of the buffers used to hold
this state, we give it the power to significantly reduce the amount of
state it needs to maintain.

This change allows the master to limit the size of this state via
command line flags.

This commit is based on a pull request generated by Felix Bechstein at:
https://github.com/apache/mesos/pull/82


Diffs (updated)
-

  src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840 
  src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
  src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0 
  src/master/master.cpp 145c5932610bd019eb090947569b608df6815c3a 

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


Testing
---

On Darwin I launched a master with:

./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos 
--max_completed_tasks_per_framework=2 --max_completed_frameworks=1

and a slave with:

./bin/mesos-slave.sh --master=127.0.0.1:5050

and then ran a bunch of instances of:

./src/test-framework --master=127.0.0.1:5050

each of which runs 5 tasks to completion

I then ran:

curl http://localhost:5050/tasks

and verified that only 1 framework and 2 of its completed tasks were given back 
to me in the json that was returned.  I repeated this for a number of other 
configurations with max_completed_frameworks=0..2 and 
max_completed_tasks_per_framework=0..5 and verified visually that the proper 
number of tasks/frameworks were being returned by the /tasks endpoint.


Thanks,

Kevin Klues



Re: Review Request 42027: WIP: Changes HTTP responses from Unauthorized (401) to Forbidden (403).

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043, 42027]

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

- Mesos ReviewBot


On Jan. 8, 2016, 1:48 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42027/
> ---
> 
> (Updated Jan. 8, 2016, 1:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4305
> https://issues.apache.org/jira/browse/MESOS-4305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check MESOS-4305 as to why this change was necesary.
> 
> Changes in the documentation of the endpoints still lacking.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 
>   src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> f0cce190abc90f0fae84d6c3db20e8215c2d8132 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/scheduler_http_api_tests.cpp 
> 4d23a5a8368e0ed126469fa4a90a889b339ad004 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/42027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Neil Conway


> On Jan. 7, 2016, 11:21 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/README.md, line 323
> > 
> >
> > Do we really have to look at all the code paths within `doHttpGet`? By 
> > the time `getBlob()` returns to the caller (i.e., by the time anyone might 
> > satisfy the future in question), `doHttpGet` must have been called and 
> > returned already.
> 
> Greg Mann wrote:
> Correct me if I'm wrong, but the execution context of the callbacks 
> registered on this future will be determined by the code path followed in 
> `doHttpGet`. So if we want to confirm that execution remains within this 
> process' context, we need to walk through those paths and confirm that they 
> stay within the process.

Makes sense. Can we clarify that what we care about is the behavior of 
callbacks on the future registered in `doHttpGet`, not the behavior of 
`doHttpGet` itself?


- Neil


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


On Jan. 8, 2016, 5:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42030/
> ---
> 
> (Updated Jan. 8, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4207
> https://issues.apache.org/jira/browse/MESOS-4207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added example of a `defer` bug to libprocess README.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/42030/diff/
> 
> 
> Testing
> ---
> 
> Viewed the markdown in a github branch: 
> https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess
> 
> Note that in addition to the inclusion of this example, further changes to 
> the `defer` documentation preceding the example are currently being 
> considered: https://reviews.apache.org/r/41618/
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Greg Mann

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

(Updated Jan. 8, 2016, 5:01 p.m.)


Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added example of a `defer` bug to libprocess README.


Diffs (updated)
-

  3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 

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


Testing
---

Viewed the markdown in a github branch: 
https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess

Note that in addition to the inclusion of this example, further changes to the 
`defer` documentation preceding the example are currently being considered: 
https://reviews.apache.org/r/41618/


Thanks,

Greg Mann



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Greg Mann


> On Jan. 7, 2016, 11:21 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/README.md, line 323
> > 
> >
> > Do we really have to look at all the code paths within `doHttpGet`? By 
> > the time `getBlob()` returns to the caller (i.e., by the time anyone might 
> > satisfy the future in question), `doHttpGet` must have been called and 
> > returned already.

Correct me if I'm wrong, but the execution context of the callbacks registered 
on this future will be determined by the code path followed in `doHttpGet`. So 
if we want to confirm that execution remains within this process' context, we 
need to walk through those paths and confirm that they stay within the process.


- Greg


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


On Jan. 8, 2016, 4:55 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42030/
> ---
> 
> (Updated Jan. 8, 2016, 4:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4207
> https://issues.apache.org/jira/browse/MESOS-4207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added example of a `defer` bug to libprocess README.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/42030/diff/
> 
> 
> Testing
> ---
> 
> Viewed the markdown in a github branch: 
> https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess
> 
> Note that in addition to the inclusion of this example, further changes to 
> the `defer` documentation preceding the example are currently being 
> considered: https://reviews.apache.org/r/41618/
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42030: Added example of a `defer` bug to libprocess README.

2016-01-08 Thread Greg Mann

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

(Updated Jan. 8, 2016, 4:55 p.m.)


Review request for mesos, Ben Mahler, Jojy Varghese, Neil Conway, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added example of a `defer` bug to libprocess README.


Diffs (updated)
-

  3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 

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


Testing
---

Viewed the markdown in a github branch: 
https://github.com/mesosphere/mesos/tree/defer_example/3rdparty/libprocess

Note that in addition to the inclusion of this example, further changes to the 
`defer` documentation preceding the example are currently being considered: 
https://reviews.apache.org/r/41618/


Thanks,

Greg Mann



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

2016-01-08 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [41096, 41185, 41108]

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

Error:
 2016-01-08 16:43:43 URL:https://reviews.apache.org/r/41108/diff/raw/ 
[4052/4052] -> "41108.patch" [1]
error: patch failed: src/slave/cmake/FindCurl.cmake:31
error: src/slave/cmake/FindCurl.cmake: patch does not apply

- Mesos ReviewBot


On Jan. 8, 2016, 3:29 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41108/
> ---
> 
> (Updated Jan. 8, 2016, 3:29 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/FindCurl.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake 
> fbdfdaa27fbd8c7429861eea5baf401a221f748b 
> 
> Diff: https://reviews.apache.org/r/41108/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> Tested if and else path of new logic added to FindCurl.cmake.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>



Re: Review Request 41429: Cleaned up the CfsFilter and clarified its logging message.

2016-01-08 Thread Greg Mann

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

(Updated Jan. 8, 2016, 4:24 p.m.)


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


Changes
---

Added relevant stout includes and capitalization.


Repository: mesos


Description
---

Cleaned up the CfsFilter and clarified its logging message. Many thanks to 
@bmahler for noticing these issues!


Diffs (updated)
-

  src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 

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


Testing
---

`sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run 
correctly on Ubuntu, while the appropriate error message is displayed on Debian 
and the "CFS_" tests are filtered out.


Thanks,

Greg Mann



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41649]

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

- Mesos ReviewBot


On Jan. 8, 2016, 12:46 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41649/
> ---
> 
> (Updated Jan. 8, 2016, 12:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2317
> https://issues.apache.org/jira/browse/MESOS-2317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slave checkpointing logic after deprecation cycle.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
>   src/examples/docker_no_executor_framework.cpp 
> 7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
>   src/examples/java/TestFramework.java 
> cbcdeedb0da22f82e274d088eb15a4128fd920fe 
>   src/examples/python/test_framework.py 
> 6af6d22f54c0ad702917d09cf658e70f7032d793 
>   src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
>   src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
>   src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/41649/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42027: WIP: Changes HTTP responses from Unauthorized (401) to Forbidden (403).

2016-01-08 Thread Jan Schlicht

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

Ship it!


This does not only change 401 to 403 but also removes the "Mesos master" realm 
from 401 responses -- which should be in line with the behavior expected from 
the RFC-2617 (is it?). Hence the ticket should be renamed accordingly or these 
changes should be extracted to another RR. Changing the realm will probably 
resolve tickets `MESOS-3933` & `MESOS-4197`.

- Jan Schlicht


On Jan. 8, 2016, 2:48 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42027/
> ---
> 
> (Updated Jan. 8, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4305
> https://issues.apache.org/jira/browse/MESOS-4305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check MESOS-4305 as to why this change was necesary.
> 
> Changes in the documentation of the endpoints still lacking.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 
>   src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> f0cce190abc90f0fae84d6c3db20e8215c2d8132 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/scheduler_http_api_tests.cpp 
> 4d23a5a8368e0ed126469fa4a90a889b339ad004 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/42027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



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

2016-01-08 Thread Diana Arroyo

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

(Updated Jan. 8, 2016, 3:29 p.m.)


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


Changes
---

Actually included the code changes that reflect the comments this time.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

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

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


Testing
---

Tested on Ubuntu and OSX.
Tested if and else path of new logic added to FindCurl.cmake.


Thanks,

Diana Arroyo



Re: Review Request 41876: Used List constructor from std::initializer_list.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41875, 41876]

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

- Mesos ReviewBot


On Jan. 8, 2016, 11:29 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41876/
> ---
> 
> (Updated Jan. 8, 2016, 11:29 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-4273
> https://issues.apache.org/jira/browse/MESOS-4273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used List constructor from std::initializer_list.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 084475ac4ef58c60da67a8d50ac031a5591eb335 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> 274598e63d28150aaa0ad06fa2d85e41e525c153 
> 
> Diff: https://reviews.apache.org/r/41876/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2016-01-08 Thread Yongqiao Wang

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

(Updated Jan. 8, 2016, 3 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Addressed the comments of Adam.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
5ee3c7afadd131802c93febbb6b4dbad069c2d81 
  include/mesos/authorizer/authorizer.proto 
84727e66dd14be9a25705ab1141e92eee72fae50 
  src/CMakeLists.txt 6ae44c76def124bbd1ccd4e6ad510c2fd0bfda5e 
  src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8 
  src/authorizer/local/authorizer.hpp 586f0da19c050e75e20902c376627c8f0b4bf272 
  src/authorizer/local/authorizer.cpp c1db9c2131ea8fbf835278203a240f108c6372c5 
  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
  src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
  src/master/weights_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 

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


Testing
---

Make & Make check successfully!

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/Users/yqwyq/tmp/mesos-master  
>> /tmp/mesos-master.log 2>&1 &)
$ curl -d 
weights="[{\"weight\":1.0,\"role\":\"role1\"},{\"weight\":8.0,\"role\":\"role2\"}]"
 -X PUT http://localhost:5050/weights
$ curl http://localhost:5050/roles
{
"roles": [
{
"frameworks": [ ], 
"name": "*", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role1", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role2", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 8
}
]
}


Thanks,

Yongqiao Wang



Re: Review Request 41597: Extending allocator interface to support dynamic weights.

2016-01-08 Thread Yongqiao Wang

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

(Updated Jan. 8, 2016, 2:59 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add the interface in allocator to support updating weight
at runtime, and the allocator is invoked to allocate the
resources based on the updated weights later.


Diffs (updated)
-

  include/mesos/master/allocator.hpp fcebcab71c50a725ca4e635c03c29eed2a406bc3 
  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

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


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2016-01-08 Thread Yongqiao Wang


> On Jan. 8, 2016, 10:36 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, lines 97-100
> > 
> >
> > Why the ostringstream here?

In order to convert double to string.


> On Jan. 8, 2016, 10:36 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, line 53
> > 
> >
> > Why did we choose PUT instead of POST. Everything else uses POST.

In Restful API, POST is always use to set/add new entry, and PUT is always use 
to do update. In Mesos, any role will always has a default weight (1.0), and 
this endpoint is used to update weight (change 1.0 to other) rather than 
add/set weight. so we choose PUT.


> On Jan. 8, 2016, 10:36 a.m., Adam B wrote:
> > src/master/master.hpp, lines 1037-1039
> > 
> >
> > Just wondering if we really want the authorize() interface to take 
> > multiple roles, or just a single role at a time.

In our design, A weights update request is an atomic operation, if any one role 
in the specified roles is invaild, then fail the request, so use multiple roles 
is better.


- Yongqiao


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


On Jan. 6, 2016, 2:15 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Jan. 6, 2016, 2:15 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> f61613948b7b5c5c2118f1782a0c5f8ff7e8e057 
>   include/mesos/authorizer/authorizer.proto 
> 7b729e19484d92be48bbde4dff2c833a4109936e 
>   src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
>   src/Makefile.am e08e86724abaa3023f0483aa222354c95d4d3817 
>   src/authorizer/local/authorizer.hpp 
> 1563c11709c2612350354690b50ceb33d2720f98 
>   src/authorizer/local/authorizer.cpp 
> 1d135fb6906c7050a788cbac9ca2d8164ff064ef 
>   src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110 
>   src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/Users/yqwyq/tmp/mesos-master  
> >> /tmp/mesos-master.log 2>&1 &)
> $ curl -d 
> weights="[{\"weight\":1.0,\"role\":\"role1\"},{\"weight\":8.0,\"role\":\"role2\"}]"
>  -X PUT http://localhost:5050/weights
> $ curl http://localhost:5050/roles
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 8
> }
> ]
> }
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>



Re: Review Request 41936: Required role in set quota request explicitly.

2016-01-08 Thread Joerg Schad

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

Ship it!


Ship It!

- Joerg Schad


On Jan. 5, 2016, 6:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41936/
> ---
> 
> (Updated Jan. 5, 2016, 6:08 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4058
> https://issues.apache.org/jira/browse/MESOS-4058
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A set quota request must provide a role, which now must be passed as
> a top-level field in the request JSON and not in `Resource` objects.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498 
>   src/tests/master_quota_tests.cpp 2f1bc3ae6a370e466f7cea9b597f51d7eccb1b33 
>   src/tests/role_tests.cpp 91a6d27158a21ae11dd310dd1f55922dc147e6f3 
> 
> Diff: https://reviews.apache.org/r/41936/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 41949: Replaced `QuotaInfo` with `Quota` in allocator.

2016-01-08 Thread Joerg Schad

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

Ship it!


Ship It!

- Joerg Schad


On Jan. 5, 2016, 11:43 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41949/
> ---
> 
> (Updated Jan. 5, 2016, 11:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3979
> https://issues.apache.org/jira/browse/MESOS-3979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
>   src/master/allocator/mesos/allocator.hpp 
> 50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498 
>   src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
>   src/tests/master_quota_tests.cpp 2f1bc3ae6a370e466f7cea9b597f51d7eccb1b33 
> 
> Diff: https://reviews.apache.org/r/41949/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 41856: Added helper functions to get allocation slack.

2016-01-08 Thread Klaus Ma

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

(Updated Jan. 8, 2016, 10:10 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Jian Qiu.


Changes
---

Address comments


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


Repository: mesos


Description
---

Helper functions to get allocation slack


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---


Thanks,

Klaus Ma



Re: Review Request 41857: Got evictable executors.

2016-01-08 Thread Klaus Ma


> On Jan. 8, 2016, 4:36 p.m., Jian Qiu wrote:
> > src/slave/slave.cpp, line 3967
> > 
> >
> > this check may not be necessary because resources cannot be negative?

Yes; it won't be negative, but it'll return first resources instead of empty.


- Klaus


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


On Jan. 8, 2016, 2:54 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41857/
> ---
> 
> (Updated Jan. 8, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris 
> Van Remoortere, Joseph Wu, and Jian Qiu.
> 
> 
> Bugs: MESOS-3892
> https://issues.apache.org/jira/browse/MESOS-3892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> get evictable executors
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/41857/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 42027: WIP: Changes HTTP responses from Unauthorized (401) to Forbidden (403).

2016-01-08 Thread Alexander Rojas

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

(Updated Jan. 8, 2016, 2:48 p.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan 
Schlicht, and Till Toenshoff.


Changes
---

Changes after Jan's review.


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


Repository: mesos


Description
---

Check MESOS-4305 as to why this change was necesary.

Changes in the documentation of the endpoints still lacking.


Diffs (updated)
-

  docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a 
  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
  src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 
  src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a 
  src/tests/persistent_volume_endpoints_tests.cpp 
f0cce190abc90f0fae84d6c3db20e8215c2d8132 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/scheduler_http_api_tests.cpp 
4d23a5a8368e0ed126469fa4a90a889b339ad004 
  src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 39781: Update ModuleTest to not assume dynamic dlopen search.

2016-01-08 Thread Benjamin Bannier


> On Nov. 30, 2015, 9:51 a.m., Benjamin Bannier wrote:
> > src/tests/module_tests.cpp, line 123
> > 
> >
> > None of these functions need to be (mutating!) member functions if you 
> > simply inject `libraryDirectory`; please consider converting them to free 
> > functions, e.g. implemented in an anon namespace in this translation unit.
> 
> James Peach wrote:
> What exactly do you mean by "inject"? Pass the ```libraryDirectory``` as 
> a parameter to these free functions? Or use some Google Test or Mock 
> injection technique?
> 
> I'd prefer not to clutter this change with additional refactorings if 
> that's OK.

I meant the former, but member functions are good enough too I guess. Since you 
changed these from `static` functions to methods could you please at least make 
them `const` so it is clear that they are not mutating instance data, though?


- Benjamin


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


On Dec. 16, 2015, 9:17 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39781/
> ---
> 
> (Updated Dec. 16, 2015, 9:17 p.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
> ---
> 
> Update ModuleTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
> 
> Diff: https://reviews.apache.org/r/39781/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 41593: Added `jsonify` function to stout.

2016-01-08 Thread Alexander Rukletsov


> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, lines 97-99
> > 
> >
> > Why this behaviour? I would expect if `set()` is not called, nothing is 
> > printed. If a field is optional, I can imagine some cases treating the 
> > absence of a value differently to `false`, effectively implementing tribool 
> > logic.
> > 
> > Or am I understanding the workflow wrong?
> 
> Michael Park wrote:
> This behavior is consistent with other writers such as `ArrayWriter` 
> where not calling `append` results in `[]`. The provided expectation is that 
> a writer will always write something. If a field is optional and therefore we 
> want to omit writing under certain conditions, the pattern is to not invoke 
> the `write` at all.
> 
> For exmaple,
> ```cpp
>  190   // Omit pid for http frameworks.
>  191   if (framework.pid.isSome()) {
>  192 writer->field("pid", string(framework.pid.get()));
>  193   }
> ```
> 
> Does this seem reasonable?

I see. First, almost no-one will use `BooleanWriter` directly, but rather 
indirectly via `JSON::ObjectWriter::field()`. Second the idea is once we create 
an object, we should add an entry in the JSON result. No entry — no object. 
Makes sense, since JSON does not allow empty values for fields AFAIK. Mind 
extending the example you have in the README with this approach for optional 
fields?


> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/Makefile.am, line 38
> > 
> >
> > Is formatting off or it's just review board?
> 
> Michael Park wrote:
> The formatting is off I think. I can't be sure with all of these tabs all 
> over the place.

Yeah, it's a bit odd. In makefiles we align using tabs.


- Alexander


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


On Jan. 8, 2016, 4:47 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> ---
> 
> (Updated Jan. 8, 2016, 4:47 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
>   3rdparty/libprocess/3rdparty/stout/README.md 
> a2a3dc6120b10c8fe82088b9f0ebecfa5642f945 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 98ea47794b3a7c99b3cbd2418ba6e36eb5951259 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 62ad461eb228b688f1ceac16cfb003561ed5a806 
>   3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> bf2a2b8a9f67c6a4cf66b156b9c14fae015a8af0 
> 
> Diff: https://reviews.apache.org/r/41593/diff/
> 
> 
> Testing
> ---
> 
> Added tests to ``
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 41429: Cleaned up the CfsFilter and clarified its logging message.

2016-01-08 Thread Till Toenshoff

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

Ship it!



src/tests/environment.cpp (line 129)


Include "stout/result.hpp" please.



src/tests/environment.cpp (line 132)


Include "stout/path.hpp" please.



src/tests/environment.cpp (line 135)


Include "stout/none.hpp" please.



src/tests/environment.cpp (line 166)


Include "stout/option.hpp" please.


- Till Toenshoff


On Jan. 7, 2016, 8 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41429/
> ---
> 
> (Updated Jan. 7, 2016, 8 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the CfsFilter and clarified its logging message. Many thanks to 
> @bmahler for noticing these issues!
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 
> 
> Diff: https://reviews.apache.org/r/41429/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both Debian 8.2 and Ubuntu 14.04. The "CFS_" tests run 
> correctly on Ubuntu, while the appropriate error message is displayed on 
> Debian and the "CFS_" tests are filtered out.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42069: Porting libprocess on ppc64le.

2016-01-08 Thread Mesos ReviewBot

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Jan. 8, 2016, 11:02 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42069/
> ---
> 
> (Updated Jan. 8, 2016, 11:02 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4312
> https://issues.apache.org/jira/browse/MESOS-4312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Porting libprocess on ppc64le.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/glog-0.3.3.patch 
> 76b8c0fe3b4615371e265bab713d62c896b7c3d6 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
>   3rdparty/libprocess/3rdparty/protobuf-2.5.0.patch PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42069/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX 10.10.5, Ubuntu 14.04.3 LTS ppc64le)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 41791: Updated allocation slack for dynamic reserve.

2016-01-08 Thread Guangya Liu

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

(Updated 一月 8, 2016, 12:50 p.m.)


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


Summary (updated)
-

Updated allocation slack for dynamic reserve.


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


Repository: mesos


Description (updated)
---

Update allocation slack resources if reserve some
new stateless reserved resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Joerg Schad

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

(Updated Jan. 8, 2016, 12:46 p.m.)


Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.


Changes
---

Removed blank line.


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


Repository: mesos


Description
---

Removed slave checkpointing logic after deprecation cycle.


Diffs (updated)
-

  include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
  src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
  src/examples/docker_no_executor_framework.cpp 
7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
  src/examples/java/TestFramework.java cbcdeedb0da22f82e274d088eb15a4128fd920fe 
  src/examples/python/test_framework.py 
6af6d22f54c0ad702917d09cf658e70f7032d793 
  src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
  src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
  src/slave/containerizer/containerizer.hpp 
6964d136818ea9904fa35cd778eb9ef19e2c64fc 
  src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
  src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
  src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
  src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 

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


Testing
---

make check


Thanks,

Joerg Schad



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

2016-01-08 Thread Guangya Liu

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

(Updated 一月 8, 2016, 12:41 p.m.)


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


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


Repository: mesos


Description
---

Added a flag to master to enable oversubscription for reservations.


Diffs (updated)
-

  include/mesos/master/allocator.hpp fcebcab71c50a725ca4e635c03c29eed2a406bc3 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/master/flags.hpp d923b1b0444d7e9023f1db4cbc4f7d4b84c20ff5 
  src/master/flags.cpp 88909590ff421421659e6faac7f3444bdc57b630 
  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a 
  src/tests/persistent_volume_endpoints_tests.cpp 
f0cce190abc90f0fae84d6c3db20e8215c2d8132 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/reservation_tests.cpp 69ddf906f2041bba84d7e1a20f90ee18817b4e64 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 

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


Testing
---

Ubuntu 14.04
make
make check


Thanks,

Guangya Liu



Re: Review Request 41597: Extending allocator interface to support dynamic weights.

2016-01-08 Thread Yongqiao Wang

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

(Updated Jan. 8, 2016, 12:40 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add the interface in allocator to support updating weight
at runtime, and the allocator is invoked to allocate the
resources based on the updated weights later.


Diffs (updated)
-

  include/mesos/master/allocator.hpp fcebcab71c50a725ca4e635c03c29eed2a406bc3 
  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

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


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-08 Thread Till Toenshoff


> On Dec. 17, 2015, 3:42 a.m., Till Toenshoff wrote:
> > Ship It!

Sorry for the delay -- this patch now needs a rebase.


- Till


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


On Dec. 17, 2015, 3:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Dec. 17, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-dir' instead of simply assuming that the local .git is the proper
> directory.
> 
> 
> Diffs
> -
> 
>   bootstrap 89d986fd95dc16bbb79623ef92e3b14a2e7009f9 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 41649: Removed slave checkpointing logic after deprecation cycle.

2016-01-08 Thread Till Toenshoff

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

Ship it!


lgtm!


src/master/master.cpp (line 1076)


Intentional blank?



src/slave/containerizer/containerizer.hpp (line 138)


So this was an error as the checkpoint flag is  passed on from the 
frameworkInfo as discussed.


- Till Toenshoff


On Jan. 7, 2016, 5:13 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41649/
> ---
> 
> (Updated Jan. 7, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2317
> https://issues.apache.org/jira/browse/MESOS-2317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slave checkpointing logic after deprecation cycle.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   src/cli/execute.cpp a2b610f32da3deb9d3df99c225d22a425e03cdba 
>   src/examples/docker_no_executor_framework.cpp 
> 7b3f0581f75105addb388dc15c4a0390ae4e0e0e 
>   src/examples/java/TestFramework.java 
> cbcdeedb0da22f82e274d088eb15a4128fd920fe 
>   src/examples/python/test_framework.py 
> 6af6d22f54c0ad702917d09cf658e70f7032d793 
>   src/internal/devolve.cpp 6c38403d908954112e07030c80e8701b84178678 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
>   src/slave/slave.cpp 9d80c96d8e28085c7fa47ce21b9b055c0926d12c 
>   src/slave/state.cpp b912c554fc9cb5593f541a7803422c22dc03dffd 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/41649/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42068: Porting Mesos on ppc64le.

2016-01-08 Thread Mesos ReviewBot

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Jan. 8, 2016, 11:02 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42068/
> ---
> 
> (Updated Jan. 8, 2016, 11:02 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4312
> https://issues.apache.org/jira/browse/MESOS-4312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Porting Mesos on ppc64le.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb.patch ad8c19b9caa856ff85978ba832d48df11b3a83f0 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
>   src/linux/fs.cpp bfcf97186cd1b0696a9537c4a332083def6b462e 
> 
> Diff: https://reviews.apache.org/r/42068/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX 10.10.5, Ubuntu 14.04.3 LTS ppc64le)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 42040: Added Quota Operator Documentation.

2016-01-08 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42040]

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

- Mesos ReviewBot


On Jan. 8, 2016, 10:26 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42040/
> ---
> 
> (Updated Jan. 8, 2016, 10:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-3877
> https://issues.apache.org/jira/browse/MESOS-3877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Operator Documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md 6f0f4b9cb9d0da1f9960ebe7f36ce186c1317535 
>   docs/quota.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42040/diff/
> 
> 
> Testing
> ---
> 
> Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 41875: Provided List constructor from std::initializer_list.

2016-01-08 Thread Benjamin Bannier

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

(Updated Jan. 8, 2016, 12:38 p.m.)


Review request for mesos, Ben Mahler and Till Toenshoff.


Changes
---

Also added list_tests.cpp to stout's CMakeLists.txt.


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


Repository: mesos


Description
---

Provided List constructor from std::initializer_list.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/list.hpp 
864d8c9498d66f14ab6fc531965c12fb397b5fe5 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
62ad461eb228b688f1ceac16cfb003561ed5a806 
  3rdparty/libprocess/3rdparty/stout/tests/list_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X 10.10.5 and Debian 8)


Thanks,

Benjamin Bannier



Re: Review Request 41876: Used List constructor from std::initializer_list.

2016-01-08 Thread Benjamin Bannier

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

(Updated Jan. 8, 2016, 12:29 p.m.)


Review request for mesos, Ben Mahler and Till Toenshoff.


Changes
---

Added a new test source to stout-tests.


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


Repository: mesos


Description
---

Used List constructor from std::initializer_list.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
084475ac4ef58c60da67a8d50ac031a5591eb335 
  3rdparty/libprocess/src/tests/timeseries_tests.cpp 
274598e63d28150aaa0ad06fa2d85e41e525c153 

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


Testing
---


Thanks,

Benjamin Bannier



  1   2   >