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

[Guangya Liu]

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

(Updated 十二月 30, 2015, 7:04 a.m.)


Review request for mesos, Klaus Ma and Jian Qiu.


Summary (updated)
-

WIP: Enabled oversubscribed resources for reservations in allocator.


Repository: mesos


Description
---

Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
7f900c4e024485704d79e57ae22407557598fe6c 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[Guangya Liu]

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

(Updated 十二月 30, 2015, 7:03 a.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 optimistic offers.


Diffs (updated)
-

  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d 
  src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
  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 2f1bc3ae6a370e466f7cea9b597f51d7eccb1b33 
  src/tests/persistent_volume_endpoints_tests.cpp 
c0feedee393b8475fd27b0af9344d306a392893e 
  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

Review Request 41772: WIP: Added helper function to flatten resources.

[Guangya Liu]

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

Review request for mesos and Klaus Ma.


Repository: mesos


Description
---

Added two new helper functions to flatten resources.
1) Flatten reserved resources.
2) Flatten allocation slack revocable resources.


Diffs
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[Guangya Liu]

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

(Updated 十二月 30, 2015, 7:02 a.m.)


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


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


Repository: mesos


Description
---

Added helper function to get stateless resources.


Diffs (updated)
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[Guangya Liu]

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

(Updated 十二月 30, 2015, 7:01 a.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-

WIP: Added helper functions to filter allocation slack resources.


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


Repository: mesos


Description
---

Added helper functions to filter allocation slack resources.


Diffs (updated)
-

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 41672: Test case(s) for weights + allocation behavior

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597, 41672]

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

- Mesos ReviewBot


On Dec. 30, 2015, 2:33 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41672/
> ---
> 
> (Updated Dec. 30, 2015, 2:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test case(s) for weights + allocation behavior
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41672/diff/
> 
> 
> Testing
> ---
> 
> Make check done:
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=HierarchicalAllocatorTest.UpdateWeight
> Source directory: /Users/yqwyq/Desktop/mesos
> Build directory: /Users/yqwyq/Desktop/mesos/build
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HierarchicalAllocatorTest
> [ RUN  ] HierarchicalAllocatorTest.UpdateWeight
> [   OK ] HierarchicalAllocatorTest.UpdateWeight (87 ms)
> [--] 1 test from HierarchicalAllocatorTest (87 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (176 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41600]

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

- Mesos ReviewBot


On Dec. 30, 2015, 2:02 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> ---
> 
> (Updated Dec. 30, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the reason for the test case being slow is MESOS-4111, we will add 
> comment in the test case, and remove it when MESOS-4111 is fixed.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef 
> 
> Diff: https://reviews.apache.org/r/41600/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 41672: Test case(s) for weights + allocation behavior

[Yongqiao Wang]

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

(Updated Dec. 30, 2015, 2:33 a.m.)


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


Changes
---

Address Neil's comments. Thank Neil.


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


Repository: mesos


Description
---

Test case(s) for weights + allocation behavior


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing (updated)
---

Make check done:
Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
--gtest_filter=HierarchicalAllocatorTest.UpdateWeight
Source directory: /Users/yqwyq/Desktop/mesos
Build directory: /Users/yqwyq/Desktop/mesos/build
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HierarchicalAllocatorTest
[ RUN  ] HierarchicalAllocatorTest.UpdateWeight
[   OK ] HierarchicalAllocatorTest.UpdateWeight (87 ms)
[--] 1 test from HierarchicalAllocatorTest (87 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (176 ms total)
[  PASSED  ] 1 test.


Thanks,

Yongqiao Wang

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

[Yongqiao Wang]

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

(Updated Dec. 30, 2015, 2:32 a.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 f7ada68d7111486d264284990996413bb3d6 
  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  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 41600: Speed up SlaveTest.CommandExecutorWithOverride

[Jian Qiu]

> On 十二月 28, 2015, 11:15 a.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 435
> > 
> >
> > Mind explaining why do you think explicitly killing the executor is the 
> > right solution here? I would even propose to do it in MESOS-4161 directly 
> > for posterity.
> > 
> > To clarify some approaches here for posterity (I will be glad to 
> > continue the conversation in the JIRA ticket once you post your explanation 
> > there). There are two separate techniques used in this review request: 
> > explicit process kill and `Clock::advance()`. Though the latter may indeed 
> > slightly speed up the test, it's not longer necessary since 
> > `process::reap()` has been accelerated (see MESOS-1199). I would let @Jian 
> > Qiu comment on the former approach.

Let's add a comment instead of explicitly killing the executor


- Jian


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


On 十二月 30, 2015, 2:02 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> ---
> 
> (Updated 十二月 30, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the reason for the test case being slow is MESOS-4111, we will add 
> comment in the test case, and remove it when MESOS-4111 is fixed.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef 
> 
> Diff: https://reviews.apache.org/r/41600/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride

[Jian Qiu]

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

(Updated 十二月 30, 2015, 2:02 a.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

rebase


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


Repository: mesos


Description
---

Since the reason for the test case being slow is MESOS-4111, we will add 
comment in the test case, and remove it when MESOS-4111 is fixed.


Diffs (updated)
-

  src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef 

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


Testing
---


Thanks,

Jian Qiu

Re: Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride

[Jian Qiu]

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

(Updated 十二月 30, 2015, 2 a.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description (updated)
---

Since the reason for the test case being slow is MESOS-4111, we will add 
comment in the test case, and remove it when MESOS-4111 is fixed.


Diffs (updated)
-

  docs/containerizer-internals.md 4386bbda21686b616ccf85db408899fece3680df 
  docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1 
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
  docs/powered-by-mesos.md 3f9f2dc7e004a3c949e4e268d494637e3dee100a 
  src/master/allocator/mesos/hierarchical.cpp 
7f900c4e024485704d79e57ae22407557598fe6c 
  src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
  support/apply-reviews.py ea5e43a2b0d88f366efae56c6290c6929f4a4d5b 

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


Testing (updated)
---


Thanks,

Jian Qiu

Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

[Anand Mazumdar]

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

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


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Modified Environment Variable names from r41277 + Removed Periods at end of 
string


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


Repository: mesos


Description
---

This change introduces the implementation for the executor library. 

This uses the new HTTP Connection interface to ensure calls are properly 
pipelined.

`connected` -> Callback invoked when a TCP connection is established with the 
agent.
`disconnected` -> When the TCP connection is interrupted possibly due to an 
agent restart.
`received` -> When the executor receives events from the agent upon subscribing.


Diffs (updated)
-

  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/executor/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41277: Added environment variables to be used by HTTP executors for connection/retrying disconnections.

[Anand Mazumdar]

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

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


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds some environment variables that are used by the HTTP executors 
upon startup for connecting/retrying to the agent. These variables are 
discussed in more detail in the Executor HTTP API Design Doc.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/containerizer/containerizer.cpp 
dcdf98fea4ca6f96658886db5d09c99f3bff501d 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41661: Added documentation for API versioning.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41661]

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

- Mesos ReviewBot


On Dec. 30, 2015, 12:47 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 30, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

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

(Updated Dec. 30, 2015, 12:47 a.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Review comments.


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


Repository: mesos


Description
---

This change adds documentation on how Mesos does API versioning. It also has a 
section:
- On how API versioning and Release versioning are different.
- API compatibility/upgrade guarantees.
- Implementation Details.

Most of the information is taken from the design doc on Mesos HTTP API 
Versioning:
https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#


Diffs (updated)
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/versioning.md PRE-CREATION 

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


Testing
---

https://gist.github.com/hatred/1cc6db05d0ca51397886


Thanks,

Anand Mazumdar

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

[Benjamin Hindman]

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



src/slave/container_loggers/rotate.cpp (line 89)


virtual



src/slave/container_loggers/rotate.cpp (line 99)


Why abstract this?



src/slave/container_loggers/rotate.cpp (lines 114 - 116)


This needs to be commented!



src/slave/container_loggers/rotate.cpp (line 121)


Why not just:

if ((bytesWritten + readSize) > maxSize) {

}

Instead of asking the reader to understand that you're calculating the 
number of bytes that would overflow maxSize and then checking if that's greater 
than 0?



src/slave/container_loggers/rotate.cpp (lines 125 - 131)


Okay, IIUC then you'll have a moving window of log files, rather than 
rotating through log files .1, .2, .3, .4, .maxFiles, is that right?

Any reason not to do the latter?

Either way, this needs to be documented! Preferably at the begining of this 
class, with a basic comment here that reminds folks. This was not what I 
expected so I had to read the code carefuly.



src/slave/container_loggers/rotate.cpp (line 146)


What if this fails?



src/slave/container_loggers/rotate.cpp (lines 174 - 179)


Woah! Why not use stout's `Flags`!!!??? We do this with our other 
executables and it makes the code much simpler!



src/slave/container_loggers/rotating.cpp (line 162)


Why are we not using `Path`? Do we need to do more in the codebase to make 
us use `Path` everywhere?



src/slave/container_loggers/rotating.cpp (line 217)


No indent here.



src/slave/container_loggers/rotating.cpp (line 252)


= None();



src/slave/container_loggers/rotating.cpp (line 268)


Could we create a "flags" for these parameters? And parse the parameters as 
flags? That would be very clean!!! We could then retroactively clean up the 
other modules, it would set a clean precedent.

It's especially wierd in this circumstance to pass `Result` arguments into 
the logger and then later during initialize error out. It means that everywehre 
else in the logger you "know" that it's okay to just call `.get()` on the 
`Result` objects because you've already checked them, which is a nasty global 
invariant that people now have to remember! In your case you've dodged this 
bullet by having `RotatingContainerLogger::initialize` do the checks and error 
out there, but then what if you need to have `RotatingContainerLoggerProcess` 
do it's own initialization?

I'd rather see the `create` function passed to the `Module` return an 
`Error` ... which apparently is not allowed because we didn't pull in `stout` 
for modules? But we did pull in `stout` for the `ContainerLogger` module ... ???



src/slave/container_loggers/rotating.cpp (line 273)


This line (and subsequent lines below) should not be indented.



src/slave/container_loggers/rotating.cpp (line 280)


Let's just embed the lambda here please!


- Benjamin Hindman


On Dec. 29, 2015, 10:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Dec. 29, 2015, 10:18 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.cpp 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 41283: Introduced an Executor Library based on the new executor HTTP API.

[Jojy Varghese]

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



src/executor/executor.cpp (line 363)


Why not ``` backoff = ::random() % maxBackoff + 1 ```?

In the current form, wouldnt backoff could always be 0(when env variable 
for max backoff is not present)?


- Jojy Varghese


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Vinod Kone]

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

Ship it!



docs/versioning.md (line 13)


We should make it clear that this versioning scheme applies from Mesos 1.0 
onwards.



docs/versioning.md (line 29)


,i.e.,



docs/versioning.md (line 39)


This seems to be in conflict with #31 above? Can you just move #31 here and 
delete the current wording in #39?



docs/versioning.md (line 74)


Not sure this belongs in the user doc. It's only relevant for mesos devs.



docs/versioning.md (lines 104 - 110)


you can kill this.


- Vinod Kone


On Dec. 29, 2015, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 29, 2015, 9:08 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Dec. 22, 2015, 1:35 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41378/
> ---
> 
> (Updated Dec. 22, 2015, 1:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a43e355405b37443601c0993d36821c50056c24f 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
> 
> Diff: https://reviews.apache.org/r/41378/diff/
> 
> 
> Testing
> ---
> 
> make (OSX, Centos7)
> make check (OSX, Centos7)
> sudo bin/mesos-tests.sh (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41370: Logger Module: Update DockerContainerizer test helpers

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Dec. 22, 2015, 1:35 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41370/
> ---
> 
> (Updated Dec. 22, 2015, 1:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 582b018ea0e41cb2a9cdbf8cd1cf46870179757b 
>   src/tests/mesos.cpp 11ca0519ad0dad44328b6ebdf52c5356fda818b7 
> 
> Diff: https://reviews.apache.org/r/41370/diff/
> 
> 
> Testing
> ---
> 
> Tests will be modified and run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Dec. 22, 2015, 1:32 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41294/
> ---
> 
> (Updated Dec. 22, 2015, 1:32 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, haosdent huang, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `DockerContainerizer` creates and initializes its own `ContainerLogger` 
> and passes it into the `Docker` wrapper.
> 
> The `ContainerLogger` is used before launching executors, in two two call 
> sites depending on whether the agent is running in a container or not.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp c769372090513b702daa2def265aedf536565f96 
>   src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
> 
> Diff: https://reviews.apache.org/r/41294/diff/
> 
> 
> Testing
> ---
> 
> Tests will be modified and run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Mesos ReviewBot]

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


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 Dec. 29, 2015, 10:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Dec. 29, 2015, 10:18 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.cpp 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 41277: Added environment variables to be used by HTTP executors for connection/retrying disconnections.

[Vinod Kone]

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



src/slave/constants.hpp (lines 124 - 128)


can we make these names consistent with scheduler constants defined in 
sched/constants.cpp?

EXECUTOR_REGISTRATION_BACKOFF_FACTOR

EXECUTOR_REGISTRATION_RETRY_INTERVAL_MAX

alternatively, you can keep these in an executor namespace and get rid of 
the EXECUTOR_ prefix.



src/slave/constants.cpp (line 55)


this variable seems to be unused?



src/slave/containerizer/containerizer.cpp (line 316)


new line.


- Vinod Kone


On Dec. 14, 2015, 11:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41277/
> ---
> 
> (Updated Dec. 14, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds some environment variables that are used by the HTTP 
> executors upon startup for connecting/retrying to the agent. These variables 
> are discussed in more detail in the Executor HTTP API Design Doc.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/containerizer/containerizer.cpp 
> dcdf98fea4ca6f96658886db5d09c99f3bff501d 
> 
> Diff: https://reviews.apache.org/r/41277/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Benjamin Hindman]

> On Dec. 29, 2015, 7:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 807-810
> > 
> >
> > Flying by. We try to avoid huge nested lambda functions. Can you pull 
> > the continuation into `__launch`?
> 
> Joseph Wu wrote:
> I was under the impression that nested lambdas were preferable to 
> underscore-continuations whenever the continuation is only called once.
> 
> (For now, I'll defer making this fix.)

I'm okay with this lambda, it fits on a single screen and is nicely captured 
here as the continuation. I agree that really big lambdas should be pulled into 
functions, but I think this one is pretty straightforward.


- Benjamin


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


On Dec. 22, 2015, 1:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41167/
> ---
> 
> (Updated Dec. 22, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4088
> https://issues.apache.org/jira/browse/MESOS-4088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the `MesosContainerizer` to create and initialize the 
> `ContainerLogger`.  
> 
> The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
> `::_launch`) by calling the `ContainerLogger` beforehand.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ab87cbcc843b471c7931aa38a590896f97be9865 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8242190dffa4d011ee2728a9f0a04d3857767b69 
> 
> Diff: https://reviews.apache.org/r/41167/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> This is tested later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Benjamin Hindman]

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

Ship it!


I'll make the change below and commit.


src/slave/containerizer/mesos/containerizer.cpp (line 842)


We should just construct a `Subprocess::IO` rather than cast (same for 
below).


- Benjamin Hindman


On Dec. 22, 2015, 1:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41167/
> ---
> 
> (Updated Dec. 22, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4088
> https://issues.apache.org/jira/browse/MESOS-4088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the `MesosContainerizer` to create and initialize the 
> `ContainerLogger`.  
> 
> The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
> `::_launch`) by calling the `ContainerLogger` beforehand.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ab87cbcc843b471c7931aa38a590896f97be9865 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8242190dffa4d011ee2728a9f0a04d3857767b69 
> 
> Diff: https://reviews.apache.org/r/41167/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> This is tested later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Ben Mahler]

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


The tests look great! Just a minor comment about Forbidden (sorry I sent you 
down that path). Should be good to commit after another iteration.


3rdparty/libprocess/src/authenticator.cpp (lines 53 - 54)


Are these const?



3rdparty/libprocess/src/authenticator.cpp (lines 58 - 84)


These should go at the bottom, under the implementation of 
BasicAuthenticatorProcess::authenticate.



3rdparty/libprocess/src/authenticator.cpp (line 104)


Could we avoid the .get() here and just store the option?

```
Option authorization = request.headers.get("Authorization");

if (authorization.isNone()) {
  return unauthorized;
}

vector components = strings::split(authorization.get(), " ");
```



3rdparty/libprocess/src/authenticator.cpp (lines 121 - 123)


Sorry, looking at this again, looks like this should also be 401 with a 
challenge?

https://tools.ietf.org/html/rfc7235#section-3.1

```
   If the request included authentication credentials, then the 401
   response indicates that authorization has been refused for those
   credentials.
```

Looks like 403 is the real "unauthorized", in the sense that if the request 
was authenticated but the user is not authorized to access to the resource, 
then 403 should be returned: https://tools.ietf.org/html/rfc7231#section-6.5.3

So, `AuthenticationResult.forbidden` should be set only when no-one is 
allowed and therefore we'll never issue a challenge? Perhaps we should clarify 
this in the documentation of AuthenticationResult?



3rdparty/libprocess/src/tests/http_tests.cpp (line 1389)


One more newline here



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1390 - 1446)


Very nice!


- Ben Mahler


On Dec. 14, 2015, 12:53 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 14, 2015, 12:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 9fe27039416290296e43c6327c85721342d02cb9 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

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

[Joseph Wu]

> On Dec. 29, 2015, 2:34 p.m., Timothy Chen wrote:
> > src/slave/container_loggers/rotate.cpp, line 115
> > 
> >
> > Are you expecting termination sent as a empty pipe from stdin here?

Yeah, closing the input pipe is the our current solution for shutting down the 
logger process along with the task/executor.

Other solutions would be messier.  For the Mesos Containerizer, we would need 
to track all these extra processes along with the task/executor.  For the 
Docker Containerizer, we would need to `docker exec` the subprocess (or spawn 
them in separate containers and track them there).


> On Dec. 29, 2015, 2:34 p.m., Timothy Chen wrote:
> > src/slave/container_loggers/rotate.cpp, line 125
> > 
> >
> > I think maxFiles shouldn't be zero as well right? We will then not 
> > remove the oldest log file.

Right.  There are actually a few places we can check these parameters; either 
inside the logger executable, the logger module, or both.

We presumably want at least 1 log file.  And each log file should be at least 
some multiple of the page size (`sysconf(_SC_PAGE_SIZE)`).


- Joseph


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


On Dec. 29, 2015, 2:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Dec. 29, 2015, 2:18 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.cpp 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 41675: Speed up SlaveTest.HTTPSchedulerSlaveRestart

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Dec. 29, 2015, 1:48 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41675/
> ---
> 
> (Updated Dec. 29, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4163
> https://issues.apache.org/jira/browse/MESOS-4163
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should advance a reregister timeout to speed up the slave registration.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
> 
> Diff: https://reviews.apache.org/r/41675/diff/
> 
> 
> Testing
> ---
> 
> make check -j4 GTEST_FILTER=SlaveTest.HTTPSchedulerSlaveRestart
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SlaveTest
> [ RUN  ] SlaveTest.HTTPSchedulerSlaveRestart
> I1229 09:38:10.183754 2138071808 exec.cpp:134] Version: 0.27.0
> I1229 09:38:10.188813 328146944 exec.cpp:208] Executor registered on slave 
> 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> Registered executor on 9.110.49.60
> Starting task 4f4f7ebe-625c-42eb-aaa8-d569333751e8
> sh -c 'sleep 1000'
> Forked command at 31243
> I1229 09:38:10.212271 329220096 exec.cpp:254] Received reconnect request from 
> slave 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> I1229 09:38:10.212829 327610368 exec.cpp:231] Executor re-registered on slave 
> 97a7a987-6edb-4ee1-a79f-4de08bc0af10-S0
> Re-registered executor on 9.110.49.60
> [   OK ] SlaveTest.HTTPSchedulerSlaveRestart (417 ms)
> [--] 1 test from SlaveTest (417 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (426 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

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

[Timothy Chen]

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



src/slave/container_loggers/rotate.cpp (line 115)


Are you expecting termination sent as a empty pipe from stdin here?



src/slave/container_loggers/rotate.cpp (line 125)


I think maxFiles shouldn't be zero as well right? We will then not remove 
the oldest log file.


- Timothy Chen


On Dec. 29, 2015, 10:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Dec. 29, 2015, 10:18 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.cpp 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 41780: Change ContainerLogger and Fetcher to not duplicate FDs given to a subprocess.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


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


Repository: mesos


Description
---

This change shifts the burden of FD lifecycle management away from 
ContainerLogger module writers.
This change also cleans up the Fetcher code slightly.


Diffs
-

  include/mesos/slave/container_logger.hpp 
a2362070ead0afcef1e6c2ca784083ddb01ba51a 
  src/slave/containerizer/fetcher.cpp 4ac9149980cf7f013b318218a1b29369c5dcff17 

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


Testing
---

make check


Thanks,

Joseph Wu

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

[Joseph Wu]

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

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 bf71fe6ee65233bbc47a41f610d68ff9760e19f6 
  src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22 
  src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0 

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

Review Request 41779: Extend Subprocess::IO::FD with the option to not duplicate the given FD.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


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


Repository: mesos


Description
---

If a subprocess is started with a FD, and the parent process does not need the 
FD afterwards, the FD does not need to be duplicated.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
8704cd0a8af92b848eb9ff7cffe8e5251c5dcfec 
  3rdparty/libprocess/src/subprocess.cpp 
863523404b9ef1d2024c108f3e9e1c77c3916049 

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


Testing
---

Tests are run in the next review.


Thanks,

Joseph Wu

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

[Joseph Wu]

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

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

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

[Joseph Wu]

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

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.cpp 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 41661: Added documentation for API versioning.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41661]

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

- Mesos ReviewBot


On Dec. 29, 2015, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 29, 2015, 9:08 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


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

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

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

(Updated Dec. 29, 2015, 9:08 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds documentation on how Mesos does API versioning. It also has a 
section:
- On how API versioning and Release versioning are different.
- API compatibility/upgrade guarantees.
- Implementation Details.

Most of the information is taken from the design doc on Mesos HTTP API 
Versioning:
https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#


Diffs (updated)
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/versioning.md PRE-CREATION 

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


Testing
---

https://gist.github.com/hatred/1cc6db05d0ca51397886


Thanks,

Anand Mazumdar

Re: Review Request 41661: Added documentation for API versioning.

[Neil Conway]

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

Ship it!


Ship It!


docs/home.md (line 58)


Per below, I'd opt for "Mesos Versioning" or "Mesos Versioning Policy"



docs/versioning.md (line 5)


Can we just call this "Mesos Versioning"? "Mesos API and Release 
Versioning" is pretty long.


- Neil Conway


On Dec. 29, 2015, 8:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 29, 2015, 8:47 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

> On Dec. 29, 2015, 6:26 p.m., Neil Conway wrote:
> > What do you think about using "APIs" instead of "API's"? I think it is 
> > clearer, style-wise.
> > 
> > Should we use backticks for endpoint suffixes like `/api/vN` etc?
> > 
> > Content-wise: if we're writing user-facing documentation, I would spend 
> > less time motivating the introduction of the feature ("Without a clear 
> > versioning policy, the world sucked! So we had to change Mesos in the 
> > following way: ..."), and more time just talking about the status quo ("The 
> > Mesos API versioning policy gives operators and developers clear guidelines 
> > for how long a Mesos API will be supported. It does this by ...")
> > 
> > Also, it seems weird to have "Release Versioning" down at the end of a 
> > document that is labeled "API Versioning". The difference between the two 
> > isn't clear (until you read carefully). How about we call the document 
> > "Mesos Versioning Policy", then start by talking about API versus release 
> > versions, and go from there?

Thanks for the review Neil. It helped quite a bit in cleaning things up further.

- Changed all instances of API's to APIs.
- Removed the `Background` section completely as per our discussion. Instead, 
added a short paragraph about the status quo.
- Modified the document title to be "Mesos API and Release Versioning".

I moved the sections `API version vs Release verstion` and `Upgrades` up as per 
our discussion.


> On Dec. 29, 2015, 6:26 p.m., Neil Conway wrote:
> > docs/versioning.md, line 31
> > 
> >
> > Doesn't seem all that relevant in a user-facing document. It certainly 
> > doesn't belong this early.

Agreed. As per our discussion, I moved it to the end of the document.


> On Dec. 29, 2015, 6:26 p.m., Neil Conway wrote:
> > docs/versioning.md, line 13
> > 
> >
> > "the Scheduler HTTP API", I'd think?
> > 
> > "dealing with upgrades"

I removed this section completely as per our discussion.


- Anand


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


On Dec. 29, 2015, 8:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 29, 2015, 8:47 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

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

(Updated Dec. 29, 2015, 8:47 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Review Comments.


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


Repository: mesos


Description
---

This change adds documentation on how Mesos does API versioning. It also has a 
section:
- On how API versioning and Release versioning are different.
- API compatibility/upgrade guarantees.
- Implementation Details.

Most of the information is taken from the design doc on Mesos HTTP API 
Versioning:
https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#


Diffs (updated)
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/versioning.md PRE-CREATION 

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


Testing
---

https://gist.github.com/hatred/1cc6db05d0ca51397886


Thanks,

Anand Mazumdar

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Joseph Wu]

> On Dec. 29, 2015, 11:29 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 807-810
> > 
> >
> > Flying by. We try to avoid huge nested lambda functions. Can you pull 
> > the continuation into `__launch`?

I was under the impression that nested lambdas were preferable to 
underscore-continuations whenever the continuation is only called once.

(For now, I'll defer making this fix.)


- Joseph


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


On Dec. 21, 2015, 5:21 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41167/
> ---
> 
> (Updated Dec. 21, 2015, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4088
> https://issues.apache.org/jira/browse/MESOS-4088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the `MesosContainerizer` to create and initialize the 
> `ContainerLogger`.  
> 
> The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
> `::_launch`) by calling the `ContainerLogger` beforehand.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ab87cbcc843b471c7931aa38a590896f97be9865 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8242190dffa4d011ee2728a9f0a04d3857767b69 
> 
> Diff: https://reviews.apache.org/r/41167/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> This is tested later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

[Jojy Varghese]

> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 221
> > 
> >
> > "else" case?
> 
> Anand Mazumdar wrote:
> Nothing needs to be done for the `else` case. These env variables 
> **might** be set by the agent when framework checkpointing is enabled. For 
> more info see how the old executor library checks it:
> https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L690
> 
> Jojy Varghese wrote:
> hrm in the else case, what would be the value of recoveryTimeout?
> 
> Anand Mazumdar wrote:
> The value would be the default value of `Duration` i.e. `0ns` i.e. you 
> would like your executor to not retry and shut down as soon as you detect a 
> disconnection with the agent (as is the case for frameworks that do not have 
> checkpointing enabled). Makes sense ?
> 
> Jojy Varghese wrote:
> So checkpoint with no MESOS_RECOVERY_TIMEOUT / 
> MESOS_RETRY_MAX_BACKOFF_FACTOR is same as no checkpoint?

I think if framework intends to be “checkpointed”, then we shud have a warning 
when “recoverytimeout” is not present because the only thing checkpoint env 
variable provides is recoveryTimeout and maxBackOff.


- Jojy


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


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40975: Document that libprocess ignores SIGPIPE

[Ben Mahler]

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



3rdparty/libprocess/README.md (lines 536 - 544)


Hm.. since this document is about programming, do we need to say 
'programming notes'? Or is 'notes' enough?

It would be nice to also explain why we do this.


- Ben Mahler


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

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41380, 41381]

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

- Mesos ReviewBot


On Dec. 29, 2015, 6:44 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41381/
> ---
> 
> (Updated Dec. 29, 2015, 6:44 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test cases to test the new vip and instance_port fields
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
>   src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
> 
> Diff: https://reviews.apache.org/r/41381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Jie Yu]

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



src/slave/containerizer/mesos/containerizer.cpp (lines 807 - 810)


Flying by. We try to avoid huge nested lambda functions. Can you pull the 
continuation into `__launch`?


- Jie Yu


On Dec. 22, 2015, 1:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41167/
> ---
> 
> (Updated Dec. 22, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4088
> https://issues.apache.org/jira/browse/MESOS-4088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the `MesosContainerizer` to create and initialize the 
> `ContainerLogger`.  
> 
> The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
> `::_launch`) by calling the `ContainerLogger` beforehand.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ab87cbcc843b471c7931aa38a590896f97be9865 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8242190dffa4d011ee2728a9f0a04d3857767b69 
> 
> Diff: https://reviews.apache.org/r/41167/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> This is tested later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41004: Logger Module: Introduce the ContainerLogger module.

[Benjamin Hindman]

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

Ship it!


I'll fix the indent and commit, thanks!


src/examples/test_container_logger_module.cpp (line 40)


No indent here please.


- Benjamin Hindman


On Dec. 22, 2015, 12:49 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41004/
> ---
> 
> (Updated Dec. 22, 2015, 12:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4087
> https://issues.apache.org/jira/browse/MESOS-4087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modularizes the `ContainerLogger` interface and adds the 
> `SandboxContainerLogger` as the default.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/container_logger.hpp PRE-CREATION 
>   src/Makefile.am cf7f950c0c320a6c8e956409a35a7f47d905f71d 
>   src/examples/test_container_logger_module.cpp PRE-CREATION 
>   src/module/manager.cpp 1f04790510a2ab9ccd6907fd01be192f52ee90c6 
>   src/slave/container_logger.cpp PRE-CREATION 
>   src/tests/module.hpp e46ed12c80707bf44ceef3ed1a4eb2f321ce10f6 
>   src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828 
> 
> Diff: https://reviews.apache.org/r/41004/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Tests are modified and run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41003: Logger Module: Add the SandboxContainerLogger, the default ContainerLogger implementation.

[Benjamin Hindman]

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

Ship it!


I'll fix the indents and commit, thanks!


src/slave/container_loggers/sandbox.hpp (line 66)


No indentation here (and thus below).



src/slave/container_loggers/sandbox.cpp (line 114)


No indentation here.


- Benjamin Hindman


On Dec. 22, 2015, 12:49 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41003/
> ---
> 
> (Updated Dec. 22, 2015, 12:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4087
> https://issues.apache.org/jira/browse/MESOS-4087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implementation mirrors how executor/task stdout/stderr is currently 
> saved to plain files.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/sandbox.hpp PRE-CREATION 
>   src/slave/container_loggers/sandbox.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41003/diff/
> 
> 
> Testing
> ---
> 
> This is added to the Makefile later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

[Avinash sridharan]

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

(Updated Dec. 29, 2015, 6:44 p.m.)


Review request for mesos, Adam B and Anand Mazumdar.


Repository: mesos


Description
---

Added unit test cases to test the new vip and instance_port fields


Diffs (updated)
-

  src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
  src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 41657: Added user facing documentation for containerizers.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41656, 41657]

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

- Mesos ReviewBot


On Dec. 29, 2015, 4:36 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 29, 2015, 4:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Neil Conway]

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


What do you think about using "APIs" instead of "API's"? I think it is clearer, 
style-wise.

Should we use backticks for endpoint suffixes like `/api/vN` etc?

Content-wise: if we're writing user-facing documentation, I would spend less 
time motivating the introduction of the feature ("Without a clear versioning 
policy, the world sucked! So we had to change Mesos in the following way: 
..."), and more time just talking about the status quo ("The Mesos API 
versioning policy gives operators and developers clear guidelines for how long 
a Mesos API will be supported. It does this by ...")

Also, it seems weird to have "Release Versioning" down at the end of a document 
that is labeled "API Versioning". The difference between the two isn't clear 
(until you read carefully). How about we call the document "Mesos Versioning 
Policy", then start by talking about API versus release versions, and go from 
there?


docs/versioning.md (line 13)


"the Scheduler HTTP API", I'd think?

"dealing with upgrades"



docs/versioning.md (line 18)


"of the form"



docs/versioning.md (line 29)


Maybe clarify that this means a given Mesos installation might expose two 
versions of an endpoint, say at `/v1` and `/v2`.



docs/versioning.md (line 31)


Doesn't seem all that relevant in a user-facing document. It certainly 
doesn't belong this early.



docs/versioning.md (line 39)


I'd move this toward the end of the document.



docs/versioning.md (line 80)


"/foobar"



docs/versioning.md (line 89)


This is actually what users care about (as well as the "Upgrades" section); 
I would make this more prominent, e.g., move it closer to the start of the 
document.


- Neil Conway


On Dec. 27, 2015, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 27, 2015, 9:08 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41672: Test case(s) for weights + allocation behavior

[Neil Conway]

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


Overall comments: I think we can make the test a bit more concise and focused 
on testing weight-related allocation behavior. Specifically:

1. Don't bother testing that when there is a single framework, it is allocated 
all the resources (other tests cover this already).
2. Test that if we have two frameworks in two roles with 3:1 weight ratio, 
resources are allocated accordingly. I believe we don't need to check three 
allocations in a loop.
3. Test what happens when we add a new role with a weight of 2.


src/tests/hierarchical_allocator_tests.cpp (line 2083)


Whitespace.



src/tests/hierarchical_allocator_tests.cpp (line 2103)


"Framework framework1" is unnecessary stuttering. Can we say "framework1" 
registers with "role1" instead?



src/tests/hierarchical_allocator_tests.cpp (line 2104)


Whitespace.



src/tests/hierarchical_allocator_tests.cpp (line 2123)


Typo.



src/tests/hierarchical_allocator_tests.cpp (line 2136)


I think it would be clearer to move `counts` inside the outer loop body, 
and then removing the explicit `clear()` calls.



src/tests/hierarchical_allocator_tests.cpp (line 2138)


What is the value of looping three times? i.e., do we have any reason to 
believe that looping additional times will catch more bugs? Conversely, why is 
looping three times enough? I would probably opt for removing the loop.



src/tests/hierarchical_allocator_tests.cpp (line 2140)


"index" does not seem like the best variable name. How about "iteration", 
or "j"?



src/tests/hierarchical_allocator_tests.cpp (line 2166)


Typo.



src/tests/hierarchical_allocator_tests.cpp (line 2191)


We should assert that `allocation.get().frameworkId == framework2.id()`



src/tests/hierarchical_allocator_tests.cpp (line 2200)


"reses" is not a word. How about "resources"?


commens

- Neil Conway


On Dec. 29, 2015, 7:21 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41672/
> ---
> 
> (Updated Dec. 29, 2015, 7:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test case(s) for weights + allocation behavior
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41672/diff/
> 
> 
> Testing
> ---
> 
> Make check done:
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=HierarchicalAllocatorTest.UpdateWeight
> Source directory: /Users/yqwyq/Desktop/mesos
> Build directory: /Users/yqwyq/Desktop/mesos/build
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HierarchicalAllocatorTest
> [ RUN  ] HierarchicalAllocatorTest.UpdateWeight
> [   OK ] HierarchicalAllocatorTest.UpdateWeight (257 ms)
> [--] 1 test from HierarchicalAllocatorTest (257 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (340 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

[Jojy Varghese]

> On Dec. 29, 2015, 2:46 a.m., Jojy Varghese wrote:
> > src/executor/executor.cpp, line 221
> > 
> >
> > "else" case?
> 
> Anand Mazumdar wrote:
> Nothing needs to be done for the `else` case. These env variables 
> **might** be set by the agent when framework checkpointing is enabled. For 
> more info see how the old executor library checks it:
> https://github.com/apache/mesos/blob/master/src/exec/exec.cpp#L690
> 
> Jojy Varghese wrote:
> hrm in the else case, what would be the value of recoveryTimeout?
> 
> Anand Mazumdar wrote:
> The value would be the default value of `Duration` i.e. `0ns` i.e. you 
> would like your executor to not retry and shut down as soon as you detect a 
> disconnection with the agent (as is the case for frameworks that do not have 
> checkpointing enabled). Makes sense ?

So checkpoint with no MESOS_RECOVERY_TIMEOUT / MESOS_RETRY_MAX_BACKOFF_FACTOR 
is same as no checkpoint?


- Jojy


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


On Dec. 29, 2015, 3:11 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Dec. 29, 2015, 3:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

[Anand Mazumdar]

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

Ship it!


LGTM, just some minor cleanup nits.


src/tests/common/http_tests.cpp (line 103)


Nit: `const` does not look related to this change and we do not have style 
guidelines pertaining to it too. Can we remove all the `const` related changes 
in this review ?



src/tests/slave_tests.cpp (line 2247)


Kill this redundant line. You already have specified your intentions of 
checking if the Virtual IPs are set in the `DiscoveryInfo` object a few lines 
before:

```// Check the virtual IPs are set in the `DiscoveryInfo` object.```



src/tests/slave_tests.cpp (line 2258)


Kill this line. This made sense when we were having both i.e.

`EXPECT_SOME` and `EXPECT_EQ` instead of having them in one call now.

This alone should suffice:
```// Check the ports are set in the `DiscoveryInfo` object.```



src/tests/slave_tests.cpp (line 2259)


We generally try to make each review a self containing single atomic 
change. It looks like that these port related cleanup changes are not related 
to this review and should have been part of a separate cleanup patch.

This is fine for now but keep this in mind for future patches.


- Anand Mazumdar


On Dec. 23, 2015, 2:10 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41381/
> ---
> 
> (Updated Dec. 23, 2015, 2:10 a.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test cases to test the new vip and instance_port fields
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
>   src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7 
> 
> Diff: https://reviews.apache.org/r/41381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597, 41681]

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

- Mesos ReviewBot


On Dec. 29, 2015, 12:51 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Dec. 29, 2015, 12:51 p.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 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   include/mesos/authorizer/authorizer.proto 
> 7b729e19484d92be48bbde4dff2c833a4109936e 
>   src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
>   src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
>   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 de9c3b9054611bf65f51df4d499bee927cf4d01d 
>   src/tests/mesos.cpp 11ca0519ad0dad44328b6ebdf52c5356fda818b7 
> 
> 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 41657: Added user facing documentation for containerizers.

[Jojy Varghese]

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

(Updated Dec. 29, 2015, 4:36 p.m.)


Review request for mesos, Jie Yu and Joerg Schad.


Changes
---

review addressed.


Repository: mesos


Description
---

This documentation is intended for users trying to get an overview of
containerizer technology. It also provides the criteria for selecting a
particular type of containerizer.


Diffs (updated)
-

  docs/containerizer.md PRE-CREATION 
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 

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


Testing
---

mesos website docker.


Thanks,

Jojy Varghese

Re: Review Request 41657: Added user facing documentation for containerizers.

[Jojy Varghese]

> On Dec. 29, 2015, 9:46 a.m., Joerg Schad wrote:
> > docs/containerizer.md, line 74
> > 
> >
> > Do we have more documentation on this array of isolators?

Not at present. There is a ticket for that task (in the same epic) and I might 
work on that next.


- Jojy


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


On Dec. 28, 2015, 8:23 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 28, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41657: Added user facing documentation for containerizers.

[Jojy Varghese]

> On Dec. 29, 2015, 9:46 a.m., Joerg Schad wrote:
> > docs/containerizer.md, line 9
> > 
> >
> > I thought about this a little more (not this is not an issue just an 
> > idea so feel free to ignore), maybe it makes sense to focus on the 
> > Motivation (instead of defining what containerizers are)
> > 
> > How do you feel about the following.
> > 
> > # Motivation
> > 
> > As Mesos is able of running a large number of tasks it needs to ensure 
> > the following:
> > 
> > * Isolate tasks 
> > * .
> > * 
> > * ..
> > 
> > In order to achieve this Mesos run tasks in 'containers' which in turn 
> > can be controlled by different containerizers.

Not sure if the motivation of containers/containerizer is the scale of tasks.


> On Dec. 29, 2015, 9:46 a.m., Joerg Schad wrote:
> > docs/containerizer.md, line 12
> > 
> >
> > s/a task/ tasks?

"A container" isolates "a task" from the rest of tasks.


> On Dec. 29, 2015, 9:46 a.m., Joerg Schad wrote:
> > docs/containerizer.md, line 16
> > 
> >
> > with same environment you mean the tasks environment (e.g., provided by 
> > the docker container), which can then be run across different environments 
> > (e.g., dev, production, testing)?
> > 
> > In that case the term 'same environment' seems a little misleading.

This was inspired from Docker's website which says ```This guarantees that it 
will always run the same, regardless of the environment it is running in```


- Jojy


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


On Dec. 28, 2015, 8:23 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 28, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41769: Made allocator traverse all roles for quota allocation.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41769]

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

- Mesos ReviewBot


On Dec. 29, 2015, 12:30 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Dec. 29, 2015, 12:30 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made allocator traverse all roles for quota allocation.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f900c4e024485704d79e57ae22407557598fe6c 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Yongqiao Wang]

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

(Updated Dec. 29, 2015, 12:51 p.m.)


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


Changes
---

Add authenrize logic for update weights request.


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 
19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
  include/mesos/authorizer/authorizer.proto 
7b729e19484d92be48bbde4dff2c833a4109936e 
  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  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 de9c3b9054611bf65f51df4d499bee927cf4d01d 
  src/tests/mesos.cpp 11ca0519ad0dad44328b6ebdf52c5356fda818b7 

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

[Yongqiao Wang]

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

(Updated Dec. 29, 2015, 12:49 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 f7ada68d7111486d264284990996413bb3d6 
  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  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

Review Request 41769: Made allocator traverse all roles for quota allocation.

[Guangya Liu]

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

Review request for mesos and Klaus Ma.


Repository: mesos


Description
---

Made allocator traverse all roles for quota allocation.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
7f900c4e024485704d79e57ae22407557598fe6c 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597, 41681]

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

- Mesos ReviewBot


On Dec. 29, 2015, 9:54 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Dec. 29, 2015, 9:54 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
> -
> 
>   src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
>   src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
>   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 
> 
> 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 41681: Introduce HTTP endpoint /weights for updating weight.

[Yongqiao Wang]

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

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


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


Changes
---

Add authentication logic.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  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 

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

[Yongqiao Wang]

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

(Updated Dec. 29, 2015, 9:52 a.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 f7ada68d7111486d264284990996413bb3d6 
  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  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 41657: Added user facing documentation for containerizers.

[Joerg Schad]

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

Ship it!



docs/containerizer.md (line 9)


I thought about this a little more (not this is not an issue just an idea 
so feel free to ignore), maybe it makes sense to focus on the Motivation 
(instead of defining what containerizers are)

How do you feel about the following.

# Motivation

As Mesos is able of running a large number of tasks it needs to ensure the 
following:

* Isolate tasks 
* .
* 
* ..

In order to achieve this Mesos run tasks in 'containers' which in turn can 
be controlled by different containerizers.



docs/containerizer.md (line 12)


s/a task/ tasks?



docs/containerizer.md (line 16)


with same environment you mean the tasks environment (e.g., provided by the 
docker container), which can then be run across different environments (e.g., 
dev, production, testing)?

In that case the term 'same environment' seems a little misleading.



docs/containerizer.md (line 18)


Does this make more sense in the section below where you talk about 
different containerizer?



docs/containerizer.md (line 70)


For more details see ?



docs/containerizer.md (line 74)


Do we have more documentation on this array of isolators?


- Joerg Schad


On Dec. 28, 2015, 8:23 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 28, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

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

[Yongqiao Wang]

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

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


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


Changes
---

Call allocator to update weights.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  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 

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

[Yongqiao Wang]

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

(Updated Dec. 29, 2015, 9:02 a.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 f7ada68d7111486d264284990996413bb3d6 
  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  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.

[Yongqiao Wang]

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

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


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


Changes
---

Remove weightinfo from registry when its value is updated to 1.0


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  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 

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

[Yongqiao Wang]

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

(Updated Dec. 29, 2015, 8:59 a.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 f7ada68d7111486d264284990996413bb3d6 
  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 
7f900c4e024485704d79e57ae22407557598fe6c 
  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 41672: Test case(s) for weights + allocation behavior

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597, 41672]

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

- Mesos ReviewBot


On Dec. 29, 2015, 7:21 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41672/
> ---
> 
> (Updated Dec. 29, 2015, 7:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test case(s) for weights + allocation behavior
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41672/diff/
> 
> 
> Testing
> ---
> 
> Make check done:
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=HierarchicalAllocatorTest.UpdateWeight
> Source directory: /Users/yqwyq/Desktop/mesos
> Build directory: /Users/yqwyq/Desktop/mesos/build
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HierarchicalAllocatorTest
> [ RUN  ] HierarchicalAllocatorTest.UpdateWeight
> [   OK ] HierarchicalAllocatorTest.UpdateWeight (257 ms)
> [--] 1 test from HierarchicalAllocatorTest (257 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (340 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>