Re: Review Request 42194: Handle unreserve logic for dynamic reservation (3/3).

[Guangya Liu]

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

(Updated 一月 19, 2016, 8:22 a.m.)


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


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


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAvailable to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 42476: Introduced protobuf for set quota requests.

[Joerg Schad]

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

Ship it!



include/mesos/quota/quota.proto (line 53)


Does it make sense to use this protobuf as well in 
master_quota_tests.cpp::createRequestBody?



include/mesos/quota/quota.proto (line 54)


Given the naming scheme QuotaStatus wouldn't a more consistent name be 
QuotaSet?



include/mesos/quota/quota.proto (line 56)


I feel that optimizing here (with = 16) is more confusing than helpful, 
especially as this protobuf is mostly used as the schema for the json request.



src/master/quota_handler.cpp (line 246)


Not an issue, but question/remark: the protoRequest.error() is most likely 
to be less concise compared to the previous error messages?


- Joerg Schad


On Jan. 18, 2016, 11:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42476/
> ---
> 
> (Updated Jan. 18, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joerg Schad, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4410
> https://issues.apache.org/jira/browse/MESOS-4410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 1a6d2f07fb74d168a7eb30764ab9ff80cea5e3b6 
>   include/mesos/quota/quota.proto 338412ee967e14aa1957a47f4a50f2e19e4eca79 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/role_tests.cpp 979391306e2427aaa63a5df32704913f79e20e36 
> 
> Diff: https://reviews.apache.org/r/42476/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 4:40 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Also handle slave in whitelist.


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


Repository: mesos


Description
---

Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 42490: Updated Presentaions.md and fixed two typos in support/hooks.

[Disha Singh]

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

(Updated Jan. 19, 2016, 8:51 a.m.)


Review request for mesos, Alexander Rukletsov, Guangya Liu, and Vinod Kone.


Changes
---

This is the originalpatch file .The previous one had an error.


Repository: mesos


Description
---

Updated Presentaions.md and fixed two typos in support/hooks.


Diffs (updated)
-

  docs/presentations.md 2ce4b12 
  support/hooks/post-rewrite af907de 
  support/hooks/pre-commit bdc12af 

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


Testing
---

No testing required. Not a functional change.


Thanks,

Disha  Singh

Re: Review Request 42458: Made links to .md files consistent across documentation.

[Till Toenshoff]

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

Ship it!


Thanks Joerg!


docs/markdown-style-guide.md (line 120)


I would like to make it a bit more deterministic...

How about something like:

```
We use the following format for links pointing to  pages within the 
documentation folder:
```


- Till Toenshoff


On Jan. 19, 2016, 7:52 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42458/
> ---
> 
> (Updated Jan. 19, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-4295
> https://issues.apache.org/jira/browse/MESOS-4295
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made links to .md files consistent across documentation.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 1dc1abaf9e28abc01de728634593b42c7cfc8af5 
>   docs/app-framework-development-guide.md 
> eedf936457ac2a4b0f0a4f1c3982930beaddca6f 
>   docs/authentication.md 5f13181e93df4dc15a62d5540eeb43c40f91344d 
>   docs/c++-style-guide.md c045a8d5bef171fb475a7d6bc3e78ffb494a6d2e 
>   docs/clang-format.md 206bc2ccf42e5593f715c6ccf0cc42e5cbc62469 
>   docs/committers.md c1ae2574cef66c53886cb36e8ec38d474e3b19f4 
>   docs/containerizer-internals.md 14c4c6ce5ed2a28defb7d6623c45ee337f1e6c3c 
>   docs/containerizer.md a694de988f3326ac62ff52deba82b659a7080d8d 
>   docs/deploy-scripts.md 0dce7d88cdcdeaf18b585b9c000faf25a1bd5dfa 
>   docs/documentation-guide.md 9548bcabe95a2a26f222022530e6b66d3537c07b 
>   docs/effective-code-reviewing.md fdfafc7d11f8455b68395cd2a4df6dab75f07b22 
>   docs/fetcher-cache-internals.md b71d22591b8213be2e2a37555895176b8a6a4e19 
>   docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 
>   docs/high-availability.md 31aa66220617a3f8606b185ef247c11f00735227 
>   docs/home.md ae3ca71d8ecce63a95697a4d2a657e354d1dfc0e 
>   docs/markdown-style-guide.md 2f400d8ed61d26d5978907baec7761a328f770ad 
>   docs/operational-guide.md 811cfc4a634e44a1d2e8db40b44f6b744a1643dd 
>   docs/sandbox.md a2ad226b0ee7969622495f090579f15029a2a083 
>   docs/tools.md 3e5f4d786ccfcef85b341cdb698c4d0579019980 
>   docs/versioning.md 7af6581f2eef10c78c76c07c44e88c6841465398 
> 
> Diff: https://reviews.apache.org/r/42458/diff/
> 
> 
> Testing
> ---
> 
> Viewed via Docker Website Container (including changes to rakefile provided 
> by previous review).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 42458: Made links to .md files consistent across documentation.

[Joerg Schad]

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

(Updated Jan. 19, 2016, 9:09 a.m.)


Review request for mesos, Neil Conway and Till Toenshoff.


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


Repository: mesos


Description
---

Made links to .md files consistent across documentation.


Diffs (updated)
-

  docs/allocation-module.md 1dc1abaf9e28abc01de728634593b42c7cfc8af5 
  docs/app-framework-development-guide.md 
eedf936457ac2a4b0f0a4f1c3982930beaddca6f 
  docs/authentication.md 5f13181e93df4dc15a62d5540eeb43c40f91344d 
  docs/c++-style-guide.md c045a8d5bef171fb475a7d6bc3e78ffb494a6d2e 
  docs/clang-format.md 206bc2ccf42e5593f715c6ccf0cc42e5cbc62469 
  docs/committers.md c1ae2574cef66c53886cb36e8ec38d474e3b19f4 
  docs/containerizer-internals.md 14c4c6ce5ed2a28defb7d6623c45ee337f1e6c3c 
  docs/containerizer.md a694de988f3326ac62ff52deba82b659a7080d8d 
  docs/deploy-scripts.md 0dce7d88cdcdeaf18b585b9c000faf25a1bd5dfa 
  docs/documentation-guide.md 9548bcabe95a2a26f222022530e6b66d3537c07b 
  docs/effective-code-reviewing.md fdfafc7d11f8455b68395cd2a4df6dab75f07b22 
  docs/fetcher-cache-internals.md b71d22591b8213be2e2a37555895176b8a6a4e19 
  docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 
  docs/high-availability.md 31aa66220617a3f8606b185ef247c11f00735227 
  docs/home.md ae3ca71d8ecce63a95697a4d2a657e354d1dfc0e 
  docs/markdown-style-guide.md 2f400d8ed61d26d5978907baec7761a328f770ad 
  docs/operational-guide.md 811cfc4a634e44a1d2e8db40b44f6b744a1643dd 
  docs/sandbox.md a2ad226b0ee7969622495f090579f15029a2a083 
  docs/tools.md 3e5f4d786ccfcef85b341cdb698c4d0579019980 
  docs/versioning.md 7af6581f2eef10c78c76c07c44e88c6841465398 

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


Testing
---

Viewed via Docker Website Container (including changes to rakefile provided by 
previous review).


Thanks,

Joerg Schad

Review Request 42490: Updated Presentaions.md and fixed two typos in support/hooks.

[Disha Singh]

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

Review request for mesos, Alexander Rukletsov, Guangya Liu, and Vinod Kone.


Repository: mesos


Description
---

Updated Presentaions.md and fixed two typos in support/hooks.


Diffs
-

  support/hooks/post-rewrite 2ce4b12 
  support/hooks/pre-commit bdc12af 

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


Testing
---

No testing required. Not a functional change.


Thanks,

Disha  Singh

Re: Review Request 42490: Updated Presentaions.md and fixed two typos in support/hooks.

[Guangya Liu]

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


support/hooks/post-rewrite cannot be accessed.

- Guangya Liu


On 一月 19, 2016, 8:34 a.m., Disha  Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42490/
> ---
> 
> (Updated 一月 19, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Presentaions.md and fixed two typos in support/hooks.
> 
> 
> Diffs
> -
> 
>   support/hooks/post-rewrite 2ce4b12 
>   support/hooks/pre-commit bdc12af 
> 
> Diff: https://reviews.apache.org/r/42490/diff/
> 
> 
> Testing
> ---
> 
> No testing required. Not a functional change.
> 
> 
> Thanks,
> 
> Disha  Singh
> 
>

Re: Review Request 42415: Added a roles field to FrameworkInfo.

[Benjamin Bannier]

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



include/mesos/mesos.proto (lines 238 - 240)


Let's break this down into multiple pieces. How about

// A framework can have multiple roles. Either one of `role` or `roles`
// can be set, but never both. The `role` field will be removed once its
// deprecation cycle is over.



include/mesos/v1/mesos.proto (lines 238 - 240)


Same here.


- Benjamin Bannier


On Jan. 19, 2016, 1:25 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42415/
> ---
> 
> (Updated Jan. 19, 2016, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-4363
> https://issues.apache.org/jira/browse/MESOS-4363
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a roles field to FrameworkInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42415/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.5)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 42289: Quota doesn't allocate resources on slave joining.

[Alexander Rukletsov]

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


Let's tweak some wording and testing and we are good to go!

I liked the initial summary more. IMO a patch should describe the solution, and 
not the problem. It's quite opposite for JIRA tickets, hence I'm convinced 
patches and tickets should not share the same title. How about "Calcuated 
'remainingClusterResources' using all available agents."

I also think it won't hurt to extend the description and explain why we need 
this change. How about
"Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources."

Changes touching allocator are vulnerable to races, especially in tests. Please 
extend the testing (and mention this in the "Testing Done" section) with 
goodies like `GTEST_FILTER="*HierarchicalAllocatorTest*" ./bin/mesos-tests.sh 
--gtest_shuffle --gtest-repeat`.

Let's add a test for this change. Ideally the test should fail without the 
change and pass it with the change. I think Neil has already provided the test 
in the ticket, could you please include it?


src/master/allocator/mesos/hierarchical.cpp (lines 1227 - 1228)


Let's add a note here that we iterate over all slaves and why. How about

// NOTE: We use all active agents and not just those visible in the current 
`allocate()` call so that frameworks in roles without quota are not 
unnecessarily deprived of resources.


- Alexander Rukletsov


On Jan. 19, 2016, 8:50 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 8:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42458: Made links to .md files consistent across documentation.

[Joerg Schad]

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

(Updated Jan. 19, 2016, 10:50 a.m.)


Review request for mesos, Neil Conway and Till Toenshoff.


Changes
---

Removed addition to markdown-styleguide.


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


Repository: mesos


Description
---

Made links to .md files consistent across documentation.


Diffs (updated)
-

  docs/allocation-module.md 1dc1abaf9e28abc01de728634593b42c7cfc8af5 
  docs/app-framework-development-guide.md 
eedf936457ac2a4b0f0a4f1c3982930beaddca6f 
  docs/authentication.md 5f13181e93df4dc15a62d5540eeb43c40f91344d 
  docs/c++-style-guide.md c045a8d5bef171fb475a7d6bc3e78ffb494a6d2e 
  docs/clang-format.md 206bc2ccf42e5593f715c6ccf0cc42e5cbc62469 
  docs/committers.md c1ae2574cef66c53886cb36e8ec38d474e3b19f4 
  docs/containerizer-internals.md 14c4c6ce5ed2a28defb7d6623c45ee337f1e6c3c 
  docs/containerizer.md a694de988f3326ac62ff52deba82b659a7080d8d 
  docs/deploy-scripts.md 0dce7d88cdcdeaf18b585b9c000faf25a1bd5dfa 
  docs/documentation-guide.md 9548bcabe95a2a26f222022530e6b66d3537c07b 
  docs/effective-code-reviewing.md fdfafc7d11f8455b68395cd2a4df6dab75f07b22 
  docs/fetcher-cache-internals.md b71d22591b8213be2e2a37555895176b8a6a4e19 
  docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 
  docs/high-availability.md 31aa66220617a3f8606b185ef247c11f00735227 
  docs/home.md ae3ca71d8ecce63a95697a4d2a657e354d1dfc0e 
  docs/operational-guide.md 811cfc4a634e44a1d2e8db40b44f6b744a1643dd 
  docs/sandbox.md a2ad226b0ee7969622495f090579f15029a2a083 
  docs/tools.md 3e5f4d786ccfcef85b341cdb698c4d0579019980 
  docs/versioning.md 7af6581f2eef10c78c76c07c44e88c6841465398 

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


Testing
---

Viewed via Docker Website Container (including changes to rakefile provided by 
previous review).


Thanks,

Joerg Schad

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 6:55 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Address comments.


Summary (updated)
-

Calcuated 'remainingClusterResources' by all activated slaves.


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


Repository: mesos


Description (updated)
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

> On Jan. 19, 2016, 5:23 p.m., Alexander Rukletsov wrote:
> > Let's tweak some wording and testing and we are good to go!
> > 
> > I liked the initial summary more. IMO a patch should describe the solution, 
> > and not the problem. It's quite opposite for JIRA tickets, hence I'm 
> > convinced patches and tickets should not share the same title. How about 
> > "Calcuated 'remainingClusterResources' using all available agents."
> > 
> > I also think it won't hurt to extend the description and explain why we 
> > need this change. How about
> > "Event-triggered allocations do not include all available agents. If we
> > calculate remaining resources in the cluster using the partial view,
> > we may overlook already laid away resources for quota'ed roles and lay
> > away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> > of resources."
> > 
> > Changes touching allocator are vulnerable to races, especially in tests. 
> > Please extend the testing (and mention this in the "Testing Done" section) 
> > with goodies like `GTEST_FILTER="*HierarchicalAllocatorTest*" 
> > ./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`.
> > 
> > Let's add a test for this change. Ideally the test should fail without the 
> > change and pass it with the change. I think Neil has already provided the 
> > test in the ticket, could you please include it?
> 
> Alexander Rukletsov wrote:
> Maybe even better, you can modify existing `QuotaAbsentFramework` test.

Yes, that's what I'm thinking :). Current patch passed Neil's test in Mac OS, 
I'll re-check it in Ubuntu for `./bin/mesos-tests.sh`; it seems perf did not 
support no-Linux system.


- Klaus


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


On Jan. 19, 2016, 6:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 6:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40632: 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 一月 19, 2016, 8:03 a.m.)


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


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


Repository: mesos


Description
---

Enabled oversubscribed resources for reservations in allocator.
The allocator part including 5 patches:
1) https://reviews.apache.org/r/40632 Enabled oversubscribed resources for 
reservations in allocator
2) https://reviews.apache.org/r/41847 Updated allocation slack when slave was 
updated.
3) https://reviews.apache.org/r/41791 Updated allocation slack for dynamic 
reserve (1/3).
4) https://reviews.apache.org/r/42113 Handle unreserve logic for dynamic 
reservation (2/3).
5) https://reviews.apache.org/r/42194 Handle unreserve logic for dynamic 
reservation (3/3).


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 42477: Corrected example in quota user doc.

[Joerg Schad]

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

Ship it!


Ship It!

- Joerg Schad


On Jan. 18, 2016, 11:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42477/
> ---
> 
> (Updated Jan. 18, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 1a6d2f07fb74d168a7eb30764ab9ff80cea5e3b6 
> 
> Diff: https://reviews.apache.org/r/42477/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 42491: WIP: Updated allocatable() to distinguish different resources.

[Guangya Liu]

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

Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Updated allocatable() to distinguish different resources.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 42289: Quota doesn't allocate resources on slave joining.

[Alexander Rukletsov]

> On Jan. 19, 2016, 9:23 a.m., Alexander Rukletsov wrote:
> > Let's tweak some wording and testing and we are good to go!
> > 
> > I liked the initial summary more. IMO a patch should describe the solution, 
> > and not the problem. It's quite opposite for JIRA tickets, hence I'm 
> > convinced patches and tickets should not share the same title. How about 
> > "Calcuated 'remainingClusterResources' using all available agents."
> > 
> > I also think it won't hurt to extend the description and explain why we 
> > need this change. How about
> > "Event-triggered allocations do not include all available agents. If we
> > calculate remaining resources in the cluster using the partial view,
> > we may overlook already laid away resources for quota'ed roles and lay
> > away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> > of resources."
> > 
> > Changes touching allocator are vulnerable to races, especially in tests. 
> > Please extend the testing (and mention this in the "Testing Done" section) 
> > with goodies like `GTEST_FILTER="*HierarchicalAllocatorTest*" 
> > ./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`.
> > 
> > Let's add a test for this change. Ideally the test should fail without the 
> > change and pass it with the change. I think Neil has already provided the 
> > test in the ticket, could you please include it?

Maybe even better, you can modify existing `QuotaAbsentFramework` test.


- Alexander


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


On Jan. 19, 2016, 8:50 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 8:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

(Updated 一月 19, 2016, 10 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus Ma, 
and Neil Conway.


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


Repository: mesos


Description (updated)
---

This patch include two parts:
1) If there are some `non-active roles` in front of active roles after 
`quotaRoleSorter`, when the allocator encounter a `non-active role`, the 
allocator should not `break` but `continue` to allocate Quota for other active 
roles to make sure other roles can get its quotaed resources.
2) If some role's quota reach its guaranteed value, the allocator should handle 
another role but not break. Take the following case: role1 has quota 5 and got 
5, role2 has quota 100 and got 50, the role1 will be put in front of role2 by 
the `quotaRoleSorter`, if allocator `break` when found role1 is satisfied, then 
role2 will never get its quotaed resources.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
72e69a0f42dd724713f2a7a75f1b92ef16eb5569 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing (updated)
---

make
make check
GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose --gtest_repeat=100 --gtest_shuffle


Thanks,

Guangya Liu

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 7:10 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Repeated test done for allocator.


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


Repository: mesos


Description
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing (updated)
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=100 --gtest_shuffle


Thanks,

Klaus Ma

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

[Alexander Rukletsov]

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


How about a more succinct summary: "Traversed all roles for quota allocation"?

Changes touching allocator are vulnerable to races, especially in tests. Please 
extend the testing (and mention this in the "Testing Done" section) with 
something like like `GTEST_FILTER="*HierarchicalAllocatorTest*" 
./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`.

I think we can simplify the second test. Your intention is to test the 
scenario, where one role with quota and lower share is saturated while the 
other is not, and check whether the other will get offers, correct? (Btw, 
scenario description is a great comment for a test!). If so, I'd suggest to 
start with two agents, say "1resource", "2resources" and quotas "1resource", 
"4resources". This guarantees that the first role is 1)satisfied and 2)has a 
lower share. Then you add a third agent with, say, "2resources" and check that 
the second role gets resources offered. Do you think this is cleaner and more 
intuitive?


src/master/allocator/mesos/hierarchical.cpp (lines 1142 - 1144)


... to do any allocations for this role.



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


s/allocations,/allocations for this role,



src/tests/hierarchical_allocator_tests.cpp (lines 1842 - 1844)


How about this:
This test checks that if one role with quota has no frameworks in it, other 
roles with quota are still offered resources.



src/tests/hierarchical_allocator_tests.cpp (line 1845)


Do you think this one is better?
s/MultiQuotaAbsentFramework/MultiQuotaAbsentFrameworks



src/tests/hierarchical_allocator_tests.cpp (line 1849)


It's minor, but I think we tend to put numerals at the back of the string, 
something like "quota-role-1".



src/tests/hierarchical_allocator_tests.cpp (line 1856)


Any reason why you don't use the defacto-standard "cpus:2;mem:1024;disk:0"? 
I think some people may wonder why this test differs from the other.

Same for the next test.



src/tests/hierarchical_allocator_tests.cpp (line 1860)


How about: "Set quota for both roles".



src/tests/hierarchical_allocator_tests.cpp (lines 1862 - 1863)


blank line, please.



src/tests/hierarchical_allocator_tests.cpp (line 1866)


s/`framework`/a framework (no need for backticks)
s/QUOTA_ROLE2/`QUOTA_ROLE2` (mind backticks).

Same for the next test.



src/tests/hierarchical_allocator_tests.cpp (line 1871)


How about "Due to the coarse-grained nature of the allocations, `framework` 
will get all `agent`'s resources."



src/tests/hierarchical_allocator_tests.cpp (lines 1879 - 1880)


How about "This test checks that if there are multiple roles with quota all 
of them get enough offers given there are enough resources."



src/tests/hierarchical_allocator_tests.cpp (lines 1892 - 1893)


Same question as above.



src/tests/hierarchical_allocator_tests.cpp (line 1894)


Let's add a comment here about what is the initial state. For example, take 
a look at `DRFWithQuota` or `QuotaAgainstStarvation` tests.



src/tests/hierarchical_allocator_tests.cpp (line 1895)


We should definitely drag reader's attention to the fact that quota is 
pretty different. How about: "Quota for `QUOTA_ROLE1` is 10 times smaller than 
for `QUOTA_ROLE2`."



src/tests/hierarchical_allocator_tests.cpp (lines 1897 - 1898)


Blank line, please.



src/tests/hierarchical_allocator_tests.cpp (lines 1925 - 1937)


Let's prepend this section with a comment. What is your intention here?



src/tests/hierarchical_allocator_tests.cpp (lines 1939 - 1940)


If you want to wait for `addSlave()` to complete, please move it before 
recover section. Also, if you do not expect any allocations for addSalve, 
please explain why.



src/tests/hierarchical_allocator_tests.cpp (line 1941)


Let's 

Re: Review Request 42476: Introduced protobuf for set quota requests.

[Alexander Rukletsov]

> On Jan. 19, 2016, 8:37 a.m., Joerg Schad wrote:
> > src/master/quota_handler.cpp, line 280
> > 
> >
> > Not an issue, but question/remark: the protoRequest.error() is most 
> > likely to be less concise compared to the previous error messages?

Do you think we should hide the real error from an operator? I'd rather have 
the precise error which an operator can show me on irc or user list : ).


> On Jan. 19, 2016, 8:37 a.m., Joerg Schad wrote:
> > include/mesos/quota/quota.proto, line 54
> > 
> >
> > Given the naming scheme QuotaStatus wouldn't a more consistent name be 
> > QuotaSet?

The question here is whether we will be reusing the same protobuf for, say, 
update requests? My feeling is that this should be possible, hence a more 
general naming.


- Alexander


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


On Jan. 18, 2016, 11:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42476/
> ---
> 
> (Updated Jan. 18, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joerg Schad, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4410
> https://issues.apache.org/jira/browse/MESOS-4410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 1a6d2f07fb74d168a7eb30764ab9ff80cea5e3b6 
>   include/mesos/quota/quota.proto 338412ee967e14aa1957a47f4a50f2e19e4eca79 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/role_tests.cpp 979391306e2427aaa63a5df32704913f79e20e36 
> 
> Diff: https://reviews.apache.org/r/42476/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Alexander Rukletsov]

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


It would be great if you add a comment describing the cluster state, have a 
look at other tests for an example!


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


Backtick allocate() please.



src/tests/hierarchical_allocator_tests.cpp (lines 1823 - 1824)


Let's add a `NOTE:` here that each `addSlave()` triggers an event-based 
allocation.



src/tests/hierarchical_allocator_tests.cpp (lines 1826 - 1828)


Any reason why you change the original comment?


- Alexander Rukletsov


On Jan. 19, 2016, 11:10 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 11:10 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
> --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Guangya Liu]

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



src/tests/hierarchical_allocator_tests.cpp (lines 1812 - 1824)


What about make the test cases cover both `addSlave()` before and after 
`createFrameworkInfo`?



src/tests/hierarchical_allocator_tests.cpp (line 1818)


4 spaces


- Guangya Liu


On 一月 19, 2016, 11:10 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated 一月 19, 2016, 11:10 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
> --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

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


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Address comments


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


Repository: mesos


Description
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=100 --gtest_shuffle


Thanks,

Klaus Ma

Re: Review Request 42476: Introduced protobuf for set quota requests.

[Joerg Schad]

> On Jan. 19, 2016, 8:37 a.m., Joerg Schad wrote:
> > include/mesos/quota/quota.proto, line 54
> > 
> >
> > Given the naming scheme QuotaStatus wouldn't a more consistent name be 
> > QuotaSet?
> 
> Alexander Rukletsov wrote:
> The question here is whether we will be reusing the same protobuf for, 
> say, update requests? My feeling is that this should be possible, hence a 
> more general naming.

It is ok with me, but it would be different for the Status and Remove Request 
(which are also subtypes of Requests).


> On Jan. 19, 2016, 8:37 a.m., Joerg Schad wrote:
> > src/master/quota_handler.cpp, line 280
> > 
> >
> > Not an issue, but question/remark: the protoRequest.error() is most 
> > likely to be less concise compared to the previous error messages?
> 
> Alexander Rukletsov wrote:
> Do you think we should hide the real error from an operator? I'd rather 
> have the precise error which an operator can show me on irc or user list : ).

I said not an issue :-), I just wanted to note that before we added more 
semantic meaning to the error string (as we checked for the error ourself) and 
the protobuf error will be most likely less precise.


- Joerg


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


On Jan. 18, 2016, 11:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42476/
> ---
> 
> (Updated Jan. 18, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joerg Schad, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4410
> https://issues.apache.org/jira/browse/MESOS-4410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 1a6d2f07fb74d168a7eb30764ab9ff80cea5e3b6 
>   include/mesos/quota/quota.proto 338412ee967e14aa1957a47f4a50f2e19e4eca79 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/role_tests.cpp 979391306e2427aaa63a5df32704913f79e20e36 
> 
> Diff: https://reviews.apache.org/r/42476/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41459: Invoked `_Deferred`'s `operator F()` explicitly.

[Michael Park]

> On Dec. 28, 2015, 11:31 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 347
> > 
> >
> > Other similar constructs will be converted when the code using them 
> > will compile on Windows.
> 
> Michael Park wrote:
> Sorry, I don't think I understand what this means. Could you 
> elaborate/clarify?

Synced with Daniel offline. This was meant to be a note that this 
transformation will need to be applied in other places in the codebase such as 
`slave.cpp`.


- Michael


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


On Jan. 5, 2016, 11:58 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41459/
> ---
> 
> (Updated Jan. 5, 2016, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4221
> https://issues.apache.org/jira/browse/MESOS-4221
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> VS 2015 won't support C++14 `std::function` SFINAE until Update 2, so 
> converting `_Deferred` to `std::function` must be done by explicitly calling 
> `_Deferred`'s conversion function.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> bcb5668565298825056f1b48d48efe12d2e56e7c 
> 
> Diff: https://reviews.apache.org/r/41459/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41462: Used SFINAE-friendly `result_of` in libprocess.

[Michael Park]

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

(Updated Jan. 19, 2016, 10:43 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Used the SFINAE `result_of` implemented in 
[r41461](https://reviews.apache.org/r/41461/).

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


Diffs (updated)
-

  3rdparty/libprocess/include/process/async.hpp 
b80b60a7ceb5e87eed8cdab17144ae9617485a04 
  3rdparty/libprocess/include/process/future.hpp 
bcb5668565298825056f1b48d48efe12d2e56e7c 

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


Testing
---

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


Thanks,

Michael Park

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

[Joerg Schad]

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


The central part lgtm, will check the tests tomorrow morning in more detail.


src/tests/hierarchical_allocator_tests.cpp (line 1855)


s/0;/0



src/tests/hierarchical_allocator_tests.cpp (line 1900)


s/0;/0



src/tests/hierarchical_allocator_tests.cpp (line 1901)


s/0;/0



src/tests/hierarchical_allocator_tests.cpp (line 1903)


it 10 times smaller only for mem



src/tests/hierarchical_allocator_tests.cpp (line 1934)


s/2048;/2048


he

- Joerg Schad


On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Jan. 19, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus 
> Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-4411
> https://issues.apache.org/jira/browse/MESOS-4411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch include two parts:
> 1) If there are some non-active roles in front of active roles after 
> quotaRoleSorter, when the allocator encounter a non-active role, the 
> allocator should not break but continue to allocate Quota for other active 
> roles to make sure other roles can get its quotaed resources.
> 2) If some role's quota reach its guaranteed value, the allocator should 
> handle another role but not break. Take the following case: role1 has quota 5 
> and got 5, role2 has quota 100 and got 50, the role1 will be put in front of 
> role2 by the quotaRoleSorter, if allocator break when found role1 is 
> satisfied, then role2 will never get its quotaed resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Jan. 19, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus 
> Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-4411
> https://issues.apache.org/jira/browse/MESOS-4411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch include two parts:
> 1) If there are some non-active roles in front of active roles after 
> quotaRoleSorter, when the allocator encounter a non-active role, the 
> allocator should not break but continue to allocate Quota for other active 
> roles to make sure other roles can get its quotaed resources.
> 2) If some role's quota reach its guaranteed value, the allocator should 
> handle another role but not break. Take the following case: role1 has quota 5 
> and got 5, role2 has quota 100 and got 50, the role1 will be put in front of 
> role2 by the quotaRoleSorter, if allocator break when found role1 is 
> satisfied, then role2 will never get its quotaed resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42182: Removed recovery checks in slave/http.cpp.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:49 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated summary.


Summary (updated)
-

Removed recovery checks in slave/http.cpp.


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


Repository: mesos


Description
---

Removed recovery checks in slave/http.cpp


Diffs
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 42519: Fixed race between SchedDriver.{stop(), abort()} and SchedDriver.join().

[Kevin Klues]

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

Review request for mesos, Ben Mahler and Greg Mann.


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


Repository: mesos


Description
---

Previously, it was possible for join() to return before a schedDriver
was actually fully stopped or aborted (breaking the semantics of the
join() call). The race came from a short circuit in join(), which
simply checked for status != DRIVER_RUNNING before returning. It appears
this short circuit was introduced to handle cases where initialize() or
start() ended up aborting before ever starting the driver to begin with.
However, it unintentionally covers cases where stop() or abort() were
called *after* the driver started running as well.

The problem is that stop() and abort() will change the status
to DRIVER_STOPPED or DRIVER_ABORTED before actually processing
dispatched stop or abort events (which happen asynchronously in a
libprocess thread). Under normal operation, join() would wait for these
events to trigger a latch that allowed the join() call to return.
However, with the short circuit, join() exits immediately even if the
libprocess thread hasn't yet processed the stop() or abort() events.

This commit fixes the semantics of the join() call to avoid this race.
We considered removing the latch completely and replacing it with
process.wait(), but, unlike the latch, this wouldn't ensure that stop()
or abort() was ever called in the first place.


Diffs
-

  src/sched/sched.cpp 38940b7e2563a2156be2f8c228afdc27c45b6e83 

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


Testing
---

Ran the entire 'make check' suite with no failures on both Mac OS X and ubuntu 
14.04.


Thanks,

Kevin Klues

Review Request 42524: Simplified SchedulerDriver.run().

[Kevin Klues]

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

Review request for mesos, Ben Mahler and Greg Mann.


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


Repository: mesos


Description
---

Previously, the status check in run() was unnecessary due to the short
circuit path in join(). This commit simplifies run() by removing the
check completely.


Diffs
-

  src/sched/sched.cpp 38940b7e2563a2156be2f8c228afdc27c45b6e83 

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


Testing
---

Ran the entire 'make check' suite with no failures on both Mac OS X and ubuntu 
14.04.


Thanks,

Kevin Klues

Re: Review Request 42182: Allowed `Subscribe` Calls to pass through when agent is recovering.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:51 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated summary.


Summary (updated)
-

Allowed `Subscribe` Calls to pass through when agent is recovering.


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


Repository: mesos


Description (updated)
---

See Summary.


Diffs
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42182: Allowed `Subscribe` Calls to pass through when agent is recovering.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:56 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

removed attached bug.


Repository: mesos


Description
---

See Summary.


Diffs
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41460: Used `std::is_bind_expression` to SFINAE correctly.

[Michael Park]

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

(Updated Jan. 19, 2016, 10:42 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

The Standard (C++11 through 17) does not require `std::bind`'s function call 
operator to SFINAE, and VS 2015's doesn't. `std::is_bind_expression` can be 
used to manually reroute bind expressions to the 1-arg overload, where 
(conveniently) the argument will be ignored if necessary.

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


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
bcb5668565298825056f1b48d48efe12d2e56e7c 

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


Testing
---

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


Thanks,

Michael Park

Re: Review Request 41459: Invoked `_Deferred`'s `operator F()` explicitly.

[Michael Park]

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

(Updated Jan. 19, 2016, 10:42 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

VS 2015 won't support C++14 `std::function` SFINAE until Update 2, so 
converting `_Deferred` to `std::function` must be done by explicitly calling 
`_Deferred`'s conversion function.

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


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
bcb5668565298825056f1b48d48efe12d2e56e7c 

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


Testing
---

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


Thanks,

Michael Park

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

[Michael Park]

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

(Updated Jan. 19, 2016, 10:42 p.m.)


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


Changes
---

Addressed BenH's comment.


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


Repository: mesos


Description
---

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

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

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


Diffs (updated)
-

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

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


Testing
---

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


Thanks,

Michael Park

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Jie Yu]

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


Do we want to expose == and != for DiskInfo::Source yet? We don't even have == 
and != exposed for DiskInfo yet (same for ReservationInfo, etc.). Maybe make 
them file local helpers in src/common/resources.cpp for now.


include/mesos/type_utils.hpp (line 29)


Do you need this?



include/mesos/type_utils.hpp (lines 64 - 74)


Not yours. I wish we could sort them according to lexical order as we did 
before.



include/mesos/type_utils.hpp (lines 167 - 175)


Can you move this above `operator==(const SlaveID&`



include/mesos/type_utils.hpp (line 171)


Can you move non trivial impl. to common/type_utils.cpp?



include/mesos/type_utils.hpp (lines 179 - 187)


Ditto. Move non trivial impl. to cpp file.



include/mesos/type_utils.hpp (lines 242 - 255)


Not yours, can you put these above SlaveID.



include/mesos/v1/mesos.hpp (line 30)


No you need this?



include/mesos/v1/values.hpp (line 22)


Why this change?



src/common/resources.cpp (lines 93 - 99)


I prefer to check 'source' first. Can we move this up above the persistence 
checks.



src/v1/resources.cpp (lines 93 - 99)


Ditto.


- Jie Yu


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   include/mesos/v1/values.hpp 03a19ee44b3d7f801f7ca2a61f1d42d51e0c144b 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42472: Multiple Disk: Checkpoint persistent volume based on 'DiskInfo.Source'.

[Jie Yu]

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



src/slave/paths.hpp (lines 269 - 273)


I would prefer keep paths functions as simple as possible and rely on the 
caller to properly pass in the correct 'rootDir'.


- Jie Yu


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42472/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4400
> https://issues.apache.org/jira/browse/MESOS-4400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now create persistent volume directories based on 'DiskInfo.Source'
> if it is present.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 6b9a65d2e8ed771041adce6f8c6fb0601422c6e8 
>   src/slave/paths.cpp 9315b4cf0664a381d02f5cc72083eb22055daf4c 
>   src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 
>   src/tests/paths_tests.cpp 5e8495f2a19a4641e258f78911772b991d34d85a 
> 
> Diff: https://reviews.apache.org/r/42472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42470: Multiple Disk: Added 'Source' to 'Resource.DiskInfo'.

[Jie Yu]

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



include/mesos/mesos.proto (line 646)


Can we punt on that field for now for MVP?

I guess the goal of this field is to let the framework tell if it has 
exclusive access to the disk or not. I am wondering if we should use other 
primitives for this (e.g, ExclusiveInfo at the top level of Resource), instead 
of this heuristic.

My concern is that operator can potentially change the size of a directory 
(e.g., using lvm with some filesystem), which will make this field not so 
reliable.



include/mesos/v1/mesos.proto (line 645)


Ditto.


- Jie Yu


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42470/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4378
> https://issues.apache.org/jira/browse/MESOS-4378
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
> 
> Diff: https://reviews.apache.org/r/42470/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42182: Fixed check thereby allowing HTTP executors to reconnect.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:29 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Update Depends on. This should go in 0.27.


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


Repository: mesos


Description
---

Previously, we used to return a `ServiceUnavailable` when slave was recovering. 
However, we only need to do so, for all other calls and let `Subscribe` pass 
through.


Diffs
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42183: Introduced HttpEvent based filters in libprocess.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:29 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated Deps


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


Repository: mesos


Description
---

This change introduces the ability to filter `HTTPEvent` in libprocess similar 
to the already existing `MessageEvent` and `DispatchEvent`.


Diffs
-

  3rdparty/libprocess/include/process/gmock.hpp 
53fdb15ef8d0c6bb95a974be416ce1922a211064 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42374: Logger Module: Add test filter for tests requiring `logrotate`.

[Joseph Wu]

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

(Updated Jan. 19, 2016, 12:22 p.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem 
Harutyunyan.


Changes
---

The test filters were alphabetized (yay!).  Rebased.


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


Repository: mesos


Description
---

The `logrotate` binary is used by the non-default `RotatingContainerLogger` 
module.  On some systems, `logrotate` is not provided by default.


Diffs (updated)
-

  src/tests/environment.cpp a6322f260c23796ceaa5d2080126ea9fef0b5ac6 

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


Testing
---

See later reviews.


Thanks,

Joseph Wu

Re: Review Request 41285: Initialized `pid` to None() instead of `UPID()` in Slave.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:28 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Updated Depends on and moved this out of the existing review chain. This should 
go in 0.27.


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


Repository: mesos


Description
---

This was earlier pointed out by Jie on https://reviews.apache.org/r/38874.

Currently, most of our logic for finding the executor type is based on the 
fields `pid`/`http` in the `Executor` struct. We were erroneously setting the 
initial `pid` value to be `UPID()` instead of being `None()`.

The value of `pid` being `UPID()` is only possible in this scenario:

- We were unable to find the type of executor upon agent recovery i.e. no 
pid/http marker file was found. We then mark this special case as `pid=UPID()`.

This special case helps us while recovery in the following ways:

- If the agent crashed before checkpointing the pid file, both `executor->pid` 
and `executor->http` would be `None()`. This is similar to the case for HTTP 
based executors (pid/http being `None`). In order, to distinguish between these 
two cases, we set the `pid=UPID()`. This helps us in not shutting the HTTP 
executor accidently by destroying the container when the agent is recovered 
using `recovery=cleanup`
- This special value also helps us to do better logging by correctly 
distinguishing between HTTP based executors and agent dying before 
checkpointing the pid/http marker file. ( Both have `pid/http` as `None`)


Diffs
-

  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:28 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Update deps


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


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42386: Updated `createFrameworkInfo` for hierarchical_allocator_tests.cpp.

[Joseph Wu]

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

Ship it!


Test cleanup, LGTM!

- Joseph Wu


On Jan. 16, 2016, 6:24 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42386/
> ---
> 
> (Updated Jan. 16, 2016, 6:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The function of `createFrameworkInfo` was updated by enabling
> caller can set a bool parameter to create a framework which can
> use revocable resources.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42386/diff/
> 
> 
> Testing
> ---
> 
> make
> make tests
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42182: Removed recovery checks in slave/http.cpp

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:48 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Offline review comment from Vinod.


Summary (updated)
-

Removed recovery checks in slave/http.cpp


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


Repository: mesos


Description (updated)
---

Removed recovery checks in slave/http.cpp


Diffs (updated)
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On Jan. 19, 2016, 2:25 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 2:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> Mac & Ubuntu: ./bin/mesos-tests.sh 
> --gtest_filter="HierarchicalAllocatorTest.*" --verbose --gtest_repeat=100 
> --gtest_shuffle
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

[Joseph Wu]

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

Ship it!


Much easier to read now :)


include/mesos/resources.hpp (lines 166 - 167)


s/slack resources/slack, resources/
s/which is/that are/

Ditto for `allocationSlack()` and for v1.


- Joseph Wu


On Jan. 15, 2016, 7:57 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> ---
> 
> (Updated Jan. 15, 2016, 7:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41460: Used `std::is_bind_expression` to SFINAE correctly.

[Michael Park]

> On Jan. 3, 2016, 11:55 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/include/process/future.hpp, lines 228-230
> > 
> >
> > Why do we need to capture/alias the type `F` as `G` again and then use 
> > it in `std::result_of::type`? Why can't we just use `F` there again?
> 
> Michael Park wrote:
> We could if there was a guarantee that the default template arguments are 
> instantiated left-to-right. I'm not sure whether such guarantee exists and 
> would prefer not rely on it even if it did.
> 
> This way, it's required that the default argument to `G` be instantiated 
> first, and there's no chance of `std::result_of` being instantiated with `F` 
> if `F` is the result of a `std::bind`.

Synced with BenH offline about this. The SFINAE evaluation is not guaranteed to 
be performed in lexical order until C++14. Once we get to C++14 we can write 
something like:

```cpp
template <
typename F,
typename = typename std::enable_if::type>::value>::type,
typename = typename std::result_of::type>
...
```

Meanwhile, we simply inline the `G`.


- Michael


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


On Jan. 6, 2016, 2:27 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41460/
> ---
> 
> (Updated Jan. 6, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4228
> https://issues.apache.org/jira/browse/MESOS-4228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Standard (C++11 through 17) does not require `std::bind`'s function call 
> operator to SFINAE, and VS 2015's doesn't. `std::is_bind_expression` can be 
> used to manually reroute bind expressions to the 1-arg overload, where 
> (conveniently) the argument will be ignored if necessary.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> bcb5668565298825056f1b48d48efe12d2e56e7c 
> 
> Diff: https://reviews.apache.org/r/41460/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41460: Used `std::is_bind_expression` to SFINAE correctly.

[Michael Park]

> On Jan. 14, 2016, 3:58 a.m., Benjamin Hindman wrote:
> > This LGTM; IIUC we're going to rely on the lexical substitution of the 
> > template parameters and that we're going to just do `typename =` instead of 
> > `typename G =`?

Refer to https://reviews.apache.org/r/41460/#comment172930


- Michael


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


On Jan. 6, 2016, 2:27 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41460/
> ---
> 
> (Updated Jan. 6, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4228
> https://issues.apache.org/jira/browse/MESOS-4228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Standard (C++11 through 17) does not require `std::bind`'s function call 
> operator to SFINAE, and VS 2015's doesn't. `std::is_bind_expression` can be 
> used to manually reroute bind expressions to the 1-arg overload, where 
> (conveniently) the argument will be ignored if necessary.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> bcb5668565298825056f1b48d48efe12d2e56e7c 
> 
> Diff: https://reviews.apache.org/r/41460/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42491: WIP: Updated allocatable() to distinguish different resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 41334, 41333, 40529, 41772, 40339, 42386, 40632, 
41847, 41791, 42113, 42194, 41848, 42491]

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

- Mesos ReviewBot


On Jan. 19, 2016, 9:43 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42491/
> ---
> 
> (Updated Jan. 19, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4426
> https://issues.apache.org/jira/browse/MESOS-4426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated allocatable() to distinguish different resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42491/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42384: Correctted a typo in test case DeactivateAndReactivateFramework.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Jan. 16, 2016, 2:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42384/
> ---
> 
> (Updated Jan. 16, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correctted a typo in test case DeactivateAndReactivateFramework.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42384/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42172: Add documentation for the ContainerLogger.

[Joseph Wu]

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

(Updated Jan. 19, 2016, 10:31 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil Conway.


Changes
---

Removed Rakefile change in lieu of [42457](/r/42457).  Note that some links in 
this doc will appear broken until 42457 is committed.


Summary (updated)
-

Add documentation for the ContainerLogger.


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


Repository: mesos


Description (updated)
---

Add documentation for the ContainerLogger.


Diffs (updated)
-

  docs/configuration.md 1834992907a29f571200fe8422f7c26eaac7d5a0 
  docs/home.md ff797fb050d710283fa7e648515ec75779e58f65 
  docs/logging.md PRE-CREATION 
  docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 

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


Testing
---

Previewed on GitHub and via:
```
docker build -t mesos/website support/site-docker
docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
```


Thanks,

Joseph Wu

Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

Review request for mesos and TimothyIL TimothyIL.


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


Repository: mesos


Description
---

Since docker 1.9, Docker now supports user-defined networks (both local and 
overlays). The user can then create containers that need to be attached to 
these networks (e.g., docker run --net=my-network
In order to support user-defined network we need to make 2 changes:
1a) Add a new network type in mesos.proto so that frameworks will be able to 
pass this network type
1b) Add a field to add the actual network name. Till now this was not needed as 
users could not define their i own names.
2) Change the executer so it supports processing port mapping also for 
user-define network (instead of just for legacy bridge)

Note that using the new user-defined network (both local and overlay) is 
considered the prefered Docker networking option.

Signed-off-by: Ezra Silvera 


Diffs
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

[Joseph Wu]

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

(Updated Jan. 19, 2016, 10:35 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil Conway.


Changes
---

Reverted change in summary/description due to change in apply-reviews.py


Summary (updated)
-

Add documentation for logging and ContainerLogger.


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


Repository: mesos


Description (updated)
---

Moves (and updates) logging flags from configuration.md into the new logging.md.

Add documentation for the ContainerLogger.
Adds documentation about logging in Master/Agent/Framework and the 
ContainerLogger.


Diffs
-

  docs/configuration.md 1834992907a29f571200fe8422f7c26eaac7d5a0 
  docs/home.md ff797fb050d710283fa7e648515ec75779e58f65 
  docs/logging.md PRE-CREATION 
  docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 

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


Testing
---

Previewed on GitHub and via:
```
docker build -t mesos/website support/site-docker
docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
```


Thanks,

Joseph Wu

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

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


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Updated Bugs link


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


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 42517: Added discussion about allowing multiple frameworks in a role.

[Neil Conway]

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

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


Repository: mesos


Description
---

Added discussion about allowing multiple frameworks in a role.


Diffs
-

  docs/persistent-volume.md 4af7d6e8dc648fb630f56db5fbad1b7b438ebcfb 

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


Testing
---

Previewed on github.


Thanks,

Neil Conway

Re: Review Request 42068: Porting Mesos on ppc64le.

[Joseph Wu]

> On Jan. 17, 2016, 10:37 a.m., Artem Harutyunyan wrote:
> > We avoid making changes in 3rdparty and the Mesos code within the same 
> > patch. Could you please break this patch into 2?
> 
> Qian Zhang wrote:
> Did you mean splitting the commit between mesos and libprocess? I saw we 
> have a check in the pre-commit (see the link below) which ensures we should 
> not have the code changes for Mesos and libprocess in the same patch, that's 
> why I have two commit: this one and https://reviews.apache.org/r/42069/, 
> please let me know if you have further comments.
> https://github.com/apache/mesos/blob/0.26.0/support/hooks/pre-commit#L29

There's isn't a pre-commit hook for everything in 3rdparty, but it would be 
good to separate:
3rdparty/leveldb.patch
3rdparty/zookeeper-3.4.5.patch

---

src/linux/fs.cpp


- Joseph


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


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

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

[Joseph Wu]

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

(Updated Jan. 19, 2016, 11:58 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil Conway.


Changes
---

Mirror updates to logging flag docs to the actual flag's help.
Add blurb in new doc about /logging/toggle.


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


Repository: mesos


Description
---

Moves (and updates) logging flags from configuration.md into the new logging.md.

Add documentation for the ContainerLogger.
Adds documentation about logging in Master/Agent/Framework and the 
ContainerLogger.


Diffs (updated)
-

  docs/configuration.md 1834992907a29f571200fe8422f7c26eaac7d5a0 
  docs/home.md ff797fb050d710283fa7e648515ec75779e58f65 
  docs/logging.md PRE-CREATION 
  docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 
  src/logging/flags.cpp b321c28492dbc9711b937f6cd9a8423ce557957f 

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


Testing
---

Previewed on GitHub and via:
```
docker build -t mesos/website support/site-docker
docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
```


Thanks,

Joseph Wu

Re: Review Request 42124: Updated /state to show usage slack and allocation slack resources.

[Joseph Wu]

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


You may want to revisit this patch in light of the recent /state jsonify 
changes.


src/common/http.cpp (lines 126 - 143)


Note: This only affects the agent now.  You'll need to modify the jsonify 
version so that it shows up in the master.

You should ask [~mcypark] if there are plans to fully replace the various 
`model` functions.


- Joseph Wu


On Jan. 16, 2016, 12:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42124/
> ---
> 
> (Updated Jan. 16, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4327
> https://issues.apache.org/jira/browse/MESOS-4327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated /state to show usage slack and allocation slack resources.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 6d8809137602089d77ba7353d285af8de070c752 
>   src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
> 
> Diff: https://reviews.apache.org/r/42124/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.ModelResources
> I0111 11:06:26.616374 1928241920 resources.cpp:513] Parsing resources as JSON 
> failed: cpus:1;cpus(foo):1;mem:512;disk:1024;ports(foo):[1-10];bar:1
> Trying semicolon-delimited string format instead
> [   OK ] HTTPTest.ModelResources (2 ms)
> [--] 1 test from HTTPTest (2 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (12 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41285: PID none bug in slave

[Anand Mazumdar]

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

(Updated Jan. 20, 2016, 1:28 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Explicit assign `pid` to `None()`


Summary (updated)
-

PID none bug in slave


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


Repository: mesos


Description (updated)
---

PID none bug in slave


Diffs (updated)
-

  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Jan. 20, 2016, 2:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 20, 2016, 2:29 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 41491: Exposed docker/appc image manifest to mesos containerizer.

[Gilbert Song]

> On Jan. 17, 2016, 10:57 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.hpp, line 60
> > 
> >
> > We only expose Docker image manifest? But the summary of this RR says 
> > we will expose both docker and appc image manifest.

Thanks. Msg fixed.


- Gilbert


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


On Jan. 15, 2016, 11:04 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41491/
> ---
> 
> (Updated Jan. 15, 2016, 11:04 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4225
> https://issues.apache.org/jira/browse/MESOS-4225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed docker/appc image manifest to mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 03425daaf015cf231920b7eda1d5424895a41286 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 4202f5552174ad3ab595ab3f16ab91d0854951f0 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 023f7363f33c4a927fae11037a408f6747fc3c39 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> f81f0039dc12d09d85fda0be345d1509b4c6a664 
> 
> Diff: https://reviews.apache.org/r/41491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41491: Exposed docker image manifest to mesos containerizer.

[Gilbert Song]

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

(Updated Jan. 19, 2016, 6:56 p.m.)


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


Changes
---

Updated summary msg.


Summary (updated)
-

Exposed docker image manifest to mesos containerizer.


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


Repository: mesos


Description (updated)
---

Exposed docker image manifest to mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
4202f5552174ad3ab595ab3f16ab91d0854951f0 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
023f7363f33c4a927fae11037a408f6747fc3c39 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
f81f0039dc12d09d85fda0be345d1509b4c6a664 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 42355: Removed the timeout from the filter.

[Qian Zhang]

> On Jan. 20, 2016, 9:49 a.m., Qian Zhang wrote:
> > One question: Say allocation interval is 10s, at the time 5s, framework 
> > sets a filter with 3s, so with this patch, we will expire the filter 10s 
> > (max(10, 3)) later, i.e., at the time 15s. Then at the time of 10s (the 
> > next allocation cycle), allocator will not allocate any resources to the 
> > framework due to the 10s filter which is good and is the issue that we 
> > intend to fix. And then in the time 12, a new slave joins, at this moment, 
> > allocator will not allocate any resources to the framework too due to the 
> > 10s filter, but maybe the new slave has the resources needed by the 
> > framework. So my question is whether this is a reasonable behavior, do we 
> > filter too much for the framework in this case?

In this case, do we need to cancel the filter once it has taken effect for one 
time and last for long enough time?


- Qian


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


On Jan. 20, 2016, 7:32 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42355/
> ---
> 
> (Updated Jan. 20, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Without the timeout, we rely on filter expiration only. This guarantees
> that filter removal is scheduled after `allocate()` if the allocator is
> backlogged given default parameters are used. Additionally we ensure the
> filter timeout is at least as big as the allocation interval.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42355/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
> without.
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Joseph Wu]

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



src/tests/hierarchical_allocator_tests.cpp (lines 1918 - 1919)


Note: This separation comment is probably unnecessary.  As long as the new 
tests are named properly (i.e. AllocationSlack...) we can tell them apart quite 
easily.



src/tests/hierarchical_allocator_tests.cpp (line 1922)


s/resources on a slave that are statically reserved/statically reserved 
resources/



src/tests/hierarchical_allocator_tests.cpp (lines 1924 - 1925)


s/to any role and/to any role, and/

s/for a role are only offered to frameworks in that role/are still only 
allocated to the reserved role/



src/tests/hierarchical_allocator_tests.cpp (lines 1952 - 1960)


It would be great to have a comment above this :)



src/tests/hierarchical_allocator_tests.cpp (lines 1995 - 1997)


Consider moving this test to after you've implemented optimistic offers for 
`Allocator::recoverResources`.



src/tests/hierarchical_allocator_tests.cpp (lines 2045 - 2047)


Consider moving this test to after you've implemented optimistic offers for 
`Allocator::recoverResources`.



src/tests/hierarchical_allocator_tests.cpp (line 2089)


s/was not count in Quota/can be allocated regardless of Quota/



src/tests/hierarchical_allocator_tests.cpp (line 2107)


It may improve readability to split up that big comment.  Suggestion:

Set quota for the quota'd role.  In the first allocation cycle, this 
effectively prevents `agent1` from being offered to any role except the quota'd 
role (as doing so would put the quota'd role under quota).



src/tests/hierarchical_allocator_tests.cpp (line 2111)


Continued suggestion:

Add a framework that can only be offered non-revocable resources from 
`agent2`.



src/tests/hierarchical_allocator_tests.cpp (lines 2116 - 2123)


Continued suggestion:

Check that `framework1` gets offered resources from `agent2` as well as 
allocation slack from `agent1`.


- Joseph Wu


On Jan. 19, 2016, 12:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Jan. 19, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled oversubscribed resources for reservations in allocator.
> The allocator part including 5 patches:
> 1) https://reviews.apache.org/r/40632 Enabled oversubscribed resources for 
> reservations in allocator
> 2) https://reviews.apache.org/r/41847 Updated allocation slack when slave was 
> updated.
> 3) https://reviews.apache.org/r/41791 Updated allocation slack for dynamic 
> reserve (1/3).
> 4) https://reviews.apache.org/r/42113 Handle unreserve logic for dynamic 
> reservation (2/3).
> 5) https://reviews.apache.org/r/42194 Handle unreserve logic for dynamic 
> reservation (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42361: Added dynamic reservation test with no principal.

[Michael Park]

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



src/tests/reservation_tests.cpp (lines 1762 - 1765)


Can we just initialize it to `DEFAULT_FRAMEWORK_INFO` and __unset__ the 
principal instead?



src/tests/reservation_tests.cpp (line 1831)


Can we check that we receive `unreserved` instead to make sure that nothing 
changed? Rather than "it didn't change to `dynamicallyReserved`"?


- Michael Park


On Jan. 16, 2016, 4:31 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42361/
> ---
> 
> (Updated Jan. 16, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dynamic reservation test with no principal.
> 
> Currently, we do not allow dynamic reservations without a principal. This 
> test was added to verify that framework reserve operations without a 
> principal will fail as expected.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 2ea3b6e48e75f438b325211fb562db19bd3a82e0 
> 
> Diff: https://reviews.apache.org/r/42361/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationTest.ReservationInfoNoPrincipal`, was added, and 
> `make check` was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42265: Fixed more tests that didn't set a shutdown expect for MockExecutor.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 18, 2016, 9:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42265/
> ---
> 
> (Updated Jan. 18, 2016, 9:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Timothy Chen.
> 
> 
> Bugs: MESOS-4349
> https://issues.apache.org/jira/browse/MESOS-4349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specifically, the following tests:
> 
> MasterTest.OfferNotRescindedOnceUsed
> OversubscriptionTest.FetchResourceUsageFromMonitor
> OversubscriptionTest.QoSFetchResourceUsageFromMonitor
> SlaveTest.ContainerUpdatedBeforeTaskReachesExecutor
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp aa0e0d8087175e3a3ed5b63a23d31aa6fe00d1b3 
>   src/tests/oversubscription_tests.cpp 
> 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
>   src/tests/slave_tests.cpp 7fe566770bbd802111885de061a53a3edf914840 
> 
> Diff: https://reviews.apache.org/r/42265/diff/
> 
> 
> Testing
> ---
> 
> Ran each test a few hundred times with and without the changes; without the 
> changes, a warning is observed. With the changes, no warning is observed.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42368: Added reservation endpoint test without auth and principal.

[Greg Mann]

> On Jan. 20, 2016, 12:44 a.m., Michael Park wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 1173-1184
> > 
> >
> > Why are we testing for this?

When we met previously, Vinod mentioned that we sometimes equate 
default-initialized values with null values in the codebase, so I though it 
worth testing that case here.


> On Jan. 20, 2016, 12:44 a.m., Michael Park wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 1200-1201
> > 
> >
> > It seems like this framework is being spun up just to reserve resources 
> > on this agent. Do we not have a way to reserve the resources via the HTTP 
> > endpoint?

Unfortunately not; currently, the /reserve and /unreserve endpoints do not work 
when HTTP authentication is disabled, as is the case in this test.


- Greg


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


On Jan. 20, 2016, 12:11 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> ---
> 
> (Updated Jan. 20, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added reservation endpoint test without auth and principal.
> 
> Currently, dynamic reservation endpoints will not work when HTTP 
> authentication is not set. This test checks that these endpoints will fail as 
> expected in this case.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42368/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, 
> was added, and make check was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41858: Added helper functions to kill executor.

[Joseph Wu]

> On Jan. 7, 2016, 11:49 p.m., Jian Qiu wrote:
> > src/slave/slave.cpp, line 4886
> > 
> >
> > Should this be used also by qos_correction?
> 
> Guangya Liu wrote:
> Want to leverage but because of `log` issues, the `qosCorrections` has 
> many special logs related with QoS controller, it is difficult to make 
> `qosCorrections` leverage this. Do you have any idea/comments for how to 
> improve?

I would recommend refactoring out the body of `if (correction.type() == 
QoSCorrection::KILL) {`.

Instead of logging directly, return an `Option`.  
The QoS controller can do `LOG(WARNING) << "Ignoring QoS correction KILL: " << 
error.get();`.


- Joseph


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


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

Re: Review Request 42517: Added discussion about allowing multiple frameworks in a role.

[Guangya Liu]

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



docs/persistent-volume.md (line 404)


s/As discussed above//

s/with a role. This/with a role, this/


- Guangya Liu


On 一月 19, 2016, 7:34 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42517/
> ---
> 
> (Updated 一月 19, 2016, 7:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added discussion about allowing multiple frameworks in a role.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 4af7d6e8dc648fb630f56db5fbad1b7b438ebcfb 
> 
> Diff: https://reviews.apache.org/r/42517/diff/
> 
> 
> Testing
> ---
> 
> Previewed on github.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42361: Added dynamic reservation test with no principal.

[Michael Park]

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



src/tests/reservation_tests.cpp (line 1823)


Why do we use `DEFAULT_ALLOCATION_INTERVAL` rather than 
`masterFlags.allocation_interval`? If we can just use the one from 
`masterFlags`, then we should be able to get rid of the `using` declaration at 
the top as well.


- Michael Park


On Jan. 16, 2016, 4:31 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42361/
> ---
> 
> (Updated Jan. 16, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dynamic reservation test with no principal.
> 
> Currently, we do not allow dynamic reservations without a principal. This 
> test was added to verify that framework reserve operations without a 
> principal will fail as expected.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 2ea3b6e48e75f438b325211fb562db19bd3a82e0 
> 
> Diff: https://reviews.apache.org/r/42361/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationTest.ReservationInfoNoPrincipal`, was added, and 
> `make check` was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42511: Updated comments around hierarchical tests.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 19, 2016, 5:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42511/
> ---
> 
> (Updated Jan. 19, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4411
> https://issues.apache.org/jira/browse/MESOS-4411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42511/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Jan. 19, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus 
> Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-4411
> https://issues.apache.org/jira/browse/MESOS-4411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch include two parts:
> 1) If there are some non-active roles in front of active roles after 
> quotaRoleSorter, when the allocator encounter a non-active role, the 
> allocator should not break but continue to allocate Quota for other active 
> roles to make sure other roles can get its quotaed resources.
> 2) If some role's quota reach its guaranteed value, the allocator should 
> handle another role but not break. Take the following case: role1 has quota 5 
> and got 5, role2 has quota 100 and got 50, the role1 will be put in front of 
> role2 by the quotaRoleSorter, if allocator break when found role1 is 
> satisfied, then role2 will never get its quotaed resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42252: Enabled master totalResources include allocation slack.

[Joseph Wu]

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


Considering that https://reviews.apache.org/r/41847/ has the same change, it 
may be appropriate to add a `Resources::nonUsageSlack()` helper.

- Joseph Wu


On Jan. 13, 2016, 6:37 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42252/
> ---
> 
> (Updated Jan. 13, 2016, 6:37 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4321
> https://issues.apache.org/jira/browse/MESOS-4321
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After master call updateSlave, the totalResources do not include
> allocation resources, this will cause some problem when end user
> using /state endpoint to check resources status.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
> 
> Diff: https://reviews.apache.org/r/42252/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42355: Removed the timeout from the filter.

[Guangya Liu]

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


What about inverseOfferFilter?

- Guangya Liu


On 一月 19, 2016, 11:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42355/
> ---
> 
> (Updated 一月 19, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Without the timeout, we rely on filter expiration only. This guarantees
> that filter removal is scheduled after `allocate()` if the allocator is
> backlogged given default parameters are used. Additionally we ensure the
> filter timeout is at least as big as the allocation interval.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42355/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
> without.
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41334: 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 一月 20, 2016, 2:51 a.m.)


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


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


Repository: mesos


Description
---

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 42361: Added dynamic reservation test with no principal.

[Greg Mann]

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

(Updated Jan. 19, 2016, 11:59 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added dynamic reservation test with no principal.

Currently, we do not allow dynamic reservations without a principal. This test 
was added to verify that framework reserve operations without a principal will 
fail as expected.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 2ea3b6e48e75f438b325211fb562db19bd3a82e0 

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


Testing
---

A new test, `ReservationTest.ReservationInfoNoPrincipal`, was added, and `make 
check` was used to test. The new test was also run with `--gtest_repeat=1000 
--gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42505: Multiple Disk: Adjusted DiskInfo validation.

[Jie Yu]

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



src/master/validation.cpp (lines 206 - 210)


You still need to do this check for disk resource that's persistent volume?


- Jie Yu


On Jan. 19, 2016, 3:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42505/
> ---
> 
> (Updated Jan. 19, 2016, 3:53 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 98360b690382ed1912a868ac93b058cb28003a12 
> 
> Diff: https://reviews.apache.org/r/42505/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Joris Van Remoortere]

> On Jan. 19, 2016, 10:50 p.m., Jie Yu wrote:
> > include/mesos/type_utils.hpp, line 29
> > 
> >
> > Do you need this?

Not after we get rid of `total_size`. I needed it for the scalar comparison.


- Joris


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


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   include/mesos/v1/values.hpp 03a19ee44b3d7f801f7ca2a61f1d42d51e0c144b 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42368: Added reservation endpoint test without auth and principal.

[Greg Mann]

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

(Updated Jan. 20, 2016, 1:30 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added reservation endpoint test without auth and principal.

Currently, dynamic reservation endpoints will not work when HTTP authentication 
is not set. This test checks that these endpoints will fail as expected in this 
case.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 

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


Testing
---

A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was 
added, and make check was used to test. The new test was also run with 
`--gtest_repeat=1000 --gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42530: Changed `createReservationInfo` to take `Option`.

[Greg Mann]

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

(Updated Jan. 20, 2016, 1:30 a.m.)


Review request for mesos and Michael Park.


Summary (updated)
-

Changed `createReservationInfo` to take `Option`.


Repository: mesos


Description (updated)
---

Changed `createReservationInfo` to take `Option`.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 42355: Removed the timeout from the filter.

[Qian Zhang]

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


One question: Say allocation interval is 10s, at the time 5s, framework sets a 
filter with 3s, so with this patch, we will expire the filter 10s (max(10, 3)) 
later, i.e., at the time 15s. Then at the time of 10s (the next allocation 
cycle), allocator will not allocate any resources to the framework due to the 
10s filter which is good and is the issue that we intend to fix. And then in 
the time 12, a new slave joins, at this moment, allocator will not allocate any 
resources to the framework too due to the 10s filter, but maybe the new slave 
has the resources needed by the framework. So my question is whether this is a 
reasonable behavior, do we filter too much for the framework in this case?

- Qian Zhang


On Jan. 20, 2016, 7:32 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42355/
> ---
> 
> (Updated Jan. 20, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Without the timeout, we rely on filter expiration only. This guarantees
> that filter removal is scheduled after `allocate()` if the allocator is
> backlogged given default parameters are used. Additionally we ensure the
> filter timeout is at least as big as the allocation interval.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42355/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
> without.
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Joseph Wu]

> On Jan. 15, 2016, 6:11 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1291-1319
> > 
> >
> > None of this should be necessary:
> > 
> > 1) You should have all the allocation slack inside `available`.
> > 2) We don't have allocation slack ACLs in the MVP, so all allocation 
> > slack should fall under `available.unreserved`.  If we do have ACLs, some 
> > of the allocation slack will be accounted for in `available.reserved(role)`.
> > 3) You shouldn't need to recalculate this during `::allocate`.
> > 
> > If any of the above are not true, you probably have a problem in the 
> > `Resource` helpers or in `::addSlave`.
> 
> Guangya Liu wrote:
> We can take a look at the following cases:
> 1) total resources cpus(r1):100
> 2) register a framework f1, cycle 1: allocated 
> cpus(r1):100;cpu(*){ALLOCATION_SLACK}:100 to f1
> 3) f1 recovered cpu(*){ALLOCATION_SLACK}:100 back
> 4) register another framwork f2, f2 get nothing here as f1 used up all 
> reserved resoures and there is no allocation slack now.
> 
> The above logic make sure that step 4) can always get the latest 
> remaining allocation slack.

The step 3) in your case is presumably because you haven't implemented all the 
allocator methods yet (as of this review in the chain), like 
`recoverResources`, `updateAllocation`, and `updateAvailable`.

If so, you should consider moving the test additions/changes before/after (this 
lets us know what your intended behavior is) the allocator changes.


- Joseph


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


On Jan. 19, 2016, 12:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Jan. 19, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled oversubscribed resources for reservations in allocator.
> The allocator part including 5 patches:
> 1) https://reviews.apache.org/r/40632 Enabled oversubscribed resources for 
> reservations in allocator
> 2) https://reviews.apache.org/r/41847 Updated allocation slack when slave was 
> updated.
> 3) https://reviews.apache.org/r/41791 Updated allocation slack for dynamic 
> reserve (1/3).
> 4) https://reviews.apache.org/r/42113 Handle unreserve logic for dynamic 
> reservation (2/3).
> 5) https://reviews.apache.org/r/42194 Handle unreserve logic for dynamic 
> reservation (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42510: Updated comments around hierarchical tests.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 19, 2016, 5:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42510/
> ---
> 
> (Updated Jan. 19, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Klaus Ma.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42510/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41847: Updated allocation slack when slave was updated.

[Joseph Wu]

> On Jan. 15, 2016, 6:21 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > 
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + 
> > oversubscribed;
> > ```
> > 
> > Where `nonUsageSlack` is a helper that filters out usage slack.  (You 
> > could just use a Resource filter directly here.)
> > 
> > ---
> > 
> > This would have fewer operations too (1 filter operation instead of 2).
> 
> Guangya Liu wrote:
> I think that the current logic is that when 
> enableReservationOversubscription is true, we only need to add the allocation 
> slack to the total resources, there is no other change except this.
> 
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , 
> comments?

I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.

The current patch is a pretty roundabout way of writing this:
```
slaves[slaveId].total = oversubscribed + 
  slaves[slaveId].total.filter([](const Resource& resource) { return 
!isUsageSlack(resource); });
```

^ This makes it explicit that we're just resetting the `USAGE_SLACK` component 
and leaving everything else untouched.


- Joseph


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


On Jan. 13, 2016, 4:55 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> ---
> 
> (Updated Jan. 13, 2016, 4:55 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated allocation slack when slave was updated.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41847/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42355: Removed the timeout from the filter.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2016, 11:32 p.m.)


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


Summary (updated)
-

Removed the timeout from the filter.


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


Repository: mesos


Description (updated)
---

Without the timeout, we rely on filter expiration only. This guarantees
that filter removal is scheduled after `allocate()` if the allocator is
backlogged given default parameters are used. Additionally we ensure the
filter timeout is at least as big as the allocation interval.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing (updated)
---

On Mac OS 10.10.4:

`make check`

`GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
without.

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 42361: Added dynamic reservation test with no principal.

[Michael Park]

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

Ship it!



src/tests/reservation_tests.cpp (lines 1761 - 1762)


We still should set the `role` explicitly right?

`DEFAULT_FRAMEWORK_INFO` doesn't include a `role`.



src/tests/reservation_tests.cpp (line 1786)


Can we be more specific in that this is a `ReservationInfo` with a missing 
principal, rather than just an empty `ReservationInfo`?


- Michael Park


On Jan. 19, 2016, 11:40 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42361/
> ---
> 
> (Updated Jan. 19, 2016, 11:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dynamic reservation test with no principal.
> 
> Currently, we do not allow dynamic reservations without a principal. This 
> test was added to verify that framework reserve operations without a 
> principal will fail as expected.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 2ea3b6e48e75f438b325211fb562db19bd3a82e0 
> 
> Diff: https://reviews.apache.org/r/42361/diff/
> 
> 
> Testing
> ---
> 
> A new test, `ReservationTest.ReservationInfoNoPrincipal`, was added, and 
> `make check` was used to test. The new test was also run with 
> `--gtest_repeat=1000 --gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Joris Van Remoortere]

> On Jan. 19, 2016, 10:50 p.m., Jie Yu wrote:
> > include/mesos/v1/values.hpp, line 22
> > 
> >
> > Why this change?

It was due to a cyclical include between `values.hpp` and `mesos.hpp`; however, 
since I moved the operators it's not necessary anymore.


- Joris


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


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   include/mesos/v1/values.hpp 03a19ee44b3d7f801f7ca2a61f1d42d51e0c144b 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42361: Added dynamic reservation test with no principal.

[Greg Mann]

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

(Updated Jan. 20, 2016, 1:27 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.


Changes
---

Introduced `createReservationInfo(None())`.


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


Repository: mesos


Description
---

Added dynamic reservation test with no principal.

Currently, we do not allow dynamic reservations without a principal. This test 
was added to verify that framework reserve operations without a principal will 
fail as expected.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 2ea3b6e48e75f438b325211fb562db19bd3a82e0 

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


Testing
---

A new test, `ReservationTest.ReservationInfoNoPrincipal`, was added, and `make 
check` was used to test. The new test was also run with `--gtest_repeat=1000 
--gtest_break_on_failure=1`.


Thanks,

Greg Mann

Review Request 42530: Changed 'createReservationInfo' to take 'Option'.

[Greg Mann]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Changed 'createReservationInfo' to take 'Option'.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 41285: Made Executor struct assign `pid/http` to be None() explicitly.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41285]

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

- Mesos ReviewBot


On Jan. 20, 2016, 1:41 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41285/
> ---
> 
> (Updated Jan. 20, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, most of our logic for finding the executor type is based on the 
> fields pid/http in the Executor struct. We were erroneously setting the 
> initial pid value to be UPID() instead of being None().
> 
> The value of pid being UPID() is only possible in this scenario:
>   
> We were unable to find the type of executor upon agent recovery i.e. no 
> pid/http marker file was found. We then mark this special case as pid=UPID().
>   
> This special case helps us while recovery in the following ways:
>   
> - If the agent crashed before checkpointing the pid file, both executor->pid 
> and executor->http would be None(). This is similar to the case for HTTP 
> based executors (pid/http being None). In order, to distinguish between these 
> two cases, we set the pid=UPID(). This helps us in not shutting the HTTP 
> executor accidently by destroying the container when the agent is recovered 
> using recovery=cleanup
> - This special value also helps us to do better logging by correctly 
> distinguishing between HTTP based executors and agent dying before 
> checkpointing the pid/http marker file. ( Both have pid/http as None)
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/41285/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42491: Updated allocatable() to distinguish different resources.

[Guangya Liu]

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

(Updated 一月 20, 2016, 1:41 a.m.)


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


Summary (updated)
-

Updated allocatable() to distinguish different resources.


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


Repository: mesos


Description
---

Updated allocatable() to distinguish different resources.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
e32ee4aa3ed9793bb5a99233e699e5cc2bdd796b 
  src/tests/hierarchical_allocator_tests.cpp 
953712149bd951789beb29c72779c4ac65aa48dc 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose --gtest_repeat=100 --gtest_shuffle


Thanks,

Guangya Liu

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

[Benjamin Hindman]

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



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


Future loop()
{
  return io::read(STDIN_FILENO, buffer, length)
.then([&](size_t readSize) -> Future {
  // ... comment here ...
  if (readSize <= 0) {
  EXIT(EXIT_SUCCESS);
  }
  Try result = write(buffer, size);
  if (result.isError()) {
return Error();
  }
  return dispatch(self(), ::loop);
}

Try write(buffer, size)
{
  if file is closed: if (leading.isNone()) {
// open the leading file
// deal with errors
  }
  
  if (need_to_rotate) {
rotate();
return write(buffer, size);
  }
  
  ... do the actual write ...
  return Nothing();
}


- Benjamin Hindman


On Jan. 16, 2016, 12:54 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Jan. 16, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the rotating container logger module.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/rotate.hpp PRE-CREATION 
>   src/slave/container_loggers/rotate.cpp PRE-CREATION 
>   src/slave/container_loggers/rotating.hpp PRE-CREATION 
>   src/slave/container_loggers/rotating.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41789: Expose the http::internal::request function.

[Adam B]

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


Looks good. Besides the previously mentioned `http::request()` name change and 
the redundant code in `createRequest()`s, this looks shippable.


3rdparty/libprocess/include/process/http.hpp (lines 812 - 814)


Seems like a good candidate for a doxygen-style @param



3rdparty/libprocess/src/http.cpp (lines 1240 - 1268)


Looks like a lot of redundant content between these two `createRequest()` 
calls. Can one of them call the other, or both of them call a common helper? 
Let's try to remove code duplication whenever possible.



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


s/enableSSL?/enableSSL ?/ since we like whitespace around our operators.



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


Rename to `validateDeleteHttpRequest` since that's what it's doing, and it 
clearly won't validate any other `http::Request`s



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


"Newline when calling or defining a function: indent with 4 spaces."
Although you were correct for the previous line:
"Newline for an assignment statement: indent with 2 spaces."
See http://mesos.apache.org/documentation/latest/c++-style-guide/


- Adam B


On Jan. 17, 2016, 10:48 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> ---
> 
> (Updated Jan. 17, 2016, 10:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil 
> Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose the internal::http::request function in the header and not add an 
> additional method/overload for put function.
> 
> (TODO): Clean the other instances of post/get to use the http::request method.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 1fe549e9c3c64b310048388d90ab04e5641e08a1 
>   3rdparty/libprocess/src/http.cpp 40fd87c4aa1417d4746a5e4268c30c0e55d0ec0e 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c23d0bf1929686cfc42969f39ce046f4794539d4 
> 
> Diff: https://reviews.apache.org/r/41789/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 19, 2016, 2:25 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 2:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> Mac & Ubuntu: ./bin/mesos-tests.sh 
> --gtest_filter="HierarchicalAllocatorTest.*" --verbose --gtest_repeat=100 
> --gtest_shuffle
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42474: Multiple Disk: Updated Slave initialize to create DiskInfo paths.

[Jie Yu]

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



src/slave/slave.cpp (line 360)


you can do `resources->filter(...)`



src/slave/slave.cpp (lines 364 - 365)


Can we use EXIT(1) instead of CHECK here so that it's less confusing to 
operators.


- Jie Yu


On Jan. 18, 2016, 9:23 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42474/
> ---
> 
> (Updated Jan. 18, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4403
> https://issues.apache.org/jira/browse/MESOS-4403
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create disk paths based on 'DiskInfo.Source' if the type is 'PATH'
> and the directory does not yet exist.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 
> 
> Diff: https://reviews.apache.org/r/42474/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 41791: Updated allocation slack for dynamic reserve (1/3).

[Joseph Wu]

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


Is there some reason why you've split up these reviews into three pieces?  
(42113 and 42194)

See below for my confusion:


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


I see several places where you overwrite changes from this review in the 
subsequent two.  Did you mean to have three reviews for the changes to 
`::updateAllocation`, `::updateAvailable`, and `::recoverResources`?


- Joseph Wu


On Jan. 14, 2016, 4:44 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 14, 2016, 4:44 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update allocation slack resources if reserve some
> new stateless reserved resources. For such case,
> the allocation slack resources will be increased.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42504: Multiple Disk: Modified 'DiskInfo' stripping logic.

[Jie Yu]

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



src/common/resources.cpp (lines 918 - 919)


We should update the comments here.



src/common/resources.cpp (line 926)


No need for the 'has_disk()' check before it's already checked above. THe 
following looks more clean to me.

```
if (stripped.disk().has_source()) {
  ...
} else {
  ...
}
```



src/common/resources.cpp (lines 961 - 966)


Do you need to update this as well?



src/tests/persistent_volume_tests.cpp (lines 91 - 106)


Instead of overwriting CreateSlaveFlags() which is a little implicit, can 
we add a SlaveResources() method to return the slave's resources to be more 
explicit.

```
Resources getDiskResources(const Megabytes& mb)
{
  ...
}

slaveFlags.resources = getDiskResources(...);
```



src/tests/persistent_volume_tests.cpp (lines 576 - 580)


Wondering if we should introduce an overload of 'createPersistentVolume' 
that takes resource, id and containerPath. The might be able to simply your 
subsequent patch on tests.


- Jie Yu


On Jan. 19, 2016, 3:51 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42504/
> ---
> 
> (Updated Jan. 19, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored persistent volume tests to use common slave resources.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/common/resources_utils.cpp d64db54a175299bf7c32ecbeeb1ce9a7afb1c88a 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/persistent_volume_tests.cpp 
> 7acf7ab29d64d891f3288f8042d267dcc82a32e9 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42504/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>