Review Request 40586: Corrected a typo and a formatting issue.

[Alexander Rukletsov]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/quota/quota.hpp 8276749efbca167c56e337aa9155fb7db739c1c9 
  src/master/allocator/mesos/hierarchical.hpp 
c65fe35198b846da2dc959dd467a21ff6edd30a9 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

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

[Alexander Rukletsov]

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

(Updated Nov. 23, 2015, 12:12 a.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
740cfa801ee90417c038308192d1f4f2416f8315 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Review Request 40224: Fix wrong flags infos in /state and /flags

[Jian Qiu]

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

Review request for mesos, Ben Mahler and haosdent huang.


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


Repository: mesos


Description
---

Fix wrong flags infos in /state and /flags. We should convert protobuf type 
flag to JSON object explicitly instead of relying on stringify


Diffs
-

  src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 

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


Testing
---

make & make check

./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
[{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
[{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
}}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
},"framework_principals": { "values": ["c"] }}]}'

curl http://master:5050/state
curl http://master:5050/flag


Thanks,

Jian Qiu

Re: Review Request 40531: Added the public Mesos events calendar to the Community page.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40531]

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

- Mesos ReviewBot


On Nov. 23, 2015, 5:48 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40531/
> ---
> 
> (Updated Nov. 23, 2015, 5:48 a.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   site/source/community.html.md 81b8d3d02739fb624b599e002df6bc5274381453 
> 
> Diff: https://reviews.apache.org/r/40531/diff/
> 
> 
> Testing
> ---
> 
> Used `support/site-docker` to render the calendar locally.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2015-11-19 at 11.34.17 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/11/20/b39ee774-96f7-4afb-b6db-adcfa8bab9aa__Screen_Shot_2015-11-19_at_11.34.17_PM.png
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38287: Check if the futrue is failed before dispatch in freeze()

[Jian Qiu]

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

(Updated Nov. 23, 2015, 5:37 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.


Changes
---

rebase


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


Repository: mesos


Description
---

In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the 
freezer process is terminated in it initilize method, which causes the core 
dump when cgroups::freezer::freeze() calls dispatch(freezer, 
::Freezer::freeze). 

This check is added in cgroups::freezer::freeze() to avoid calling dispatch if 
the freezer process is terminated.


Diffs (updated)
-

  src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a 

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


Testing
---

./mesos-tests.sh 
--gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer"
 --gtest_repeat=1000 --gtest_break_on_failure


Thanks,

Jian Qiu

Re: Review Request 40531: Added the public Mesos events calendar to the Community page.

[Michael Park]

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

(Updated Nov. 23, 2015, 5:48 a.m.)


Review request for mesos and Dave Lester.


Changes
---

Addressed Adam's comment.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  site/source/community.html.md 81b8d3d02739fb624b599e002df6bc5274381453 

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


Testing
---

Used `support/site-docker` to render the calendar locally.


File Attachments


Screen Shot 2015-11-19 at 11.34.17 PM.png
  
https://reviews.apache.org/media/uploaded/files/2015/11/20/b39ee774-96f7-4afb-b6db-adcfa8bab9aa__Screen_Shot_2015-11-19_at_11.34.17_PM.png


Thanks,

Michael Park

Re: Review Request 38287: Check if the futrue is failed before dispatch in freeze()

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38287]

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

- Mesos ReviewBot


On Nov. 23, 2015, 5:37 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38287/
> ---
> 
> (Updated Nov. 23, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.
> 
> 
> Bugs: MESOS-3272
> https://issues.apache.org/jira/browse/MESOS-3272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the 
> freezer process is terminated in it initilize method, which causes the core 
> dump when cgroups::freezer::freeze() calls dispatch(freezer, 
> ::Freezer::freeze). 
> 
> This check is added in cgroups::freezer::freeze() to avoid calling dispatch 
> if the freezer process is terminated.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a 
> 
> Diff: https://reviews.apache.org/r/38287/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer"
>  --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40570: Modified stateUpdate(...) function to get the TaskStatus object by constant reference instead of copy

[Qian Zhang]

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

Ship it!


Ship It!

- Qian Zhang


On Nov. 21, 2015, 8:21 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40570/
> ---
> 
> (Updated Nov. 21, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3476
> https://issues.apache.org/jira/browse/MESOS-3476
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d1126f00d947fdb4823b0c495335b743254ac7ee 
> 
> Diff: https://reviews.apache.org/r/40570/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40375: [WIP] Support distinguishing revocable resources in the Resource protobuf.

[Guangya Liu]

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



include/mesos/mesos.proto (line 643)


s/oversubscription/oversubscription for allocations



include/mesos/mesos.proto (line 647)


The project name may need update to ""oversubscription for reservations"



src/tests/resources_tests.cpp (line 802)


s/Standard/USAGE_SLACK



src/tests/resources_tests.cpp (lines 804 - 805)


one line



src/tests/resources_tests.cpp (lines 812 - 813)


one line



src/tests/resources_tests.cpp (line 1899)


new line



src/tests/resources_tests.cpp (line 1929)


new line



src/tests/resources_tests.cpp (line 1931)


new line



src/tests/resources_tests.cpp (line 1933)


new line



src/tests/resources_tests.cpp (line 1937)


I think we can still use "sum" here



src/tests/resources_tests.cpp (line 1961)


s/revocable/USAGE_SLACK revocable


- Guangya Liu


On 十一月 21, 2015, 3:47 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated 十一月 21, 2015, 3:47 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5ad48bd 
>   include/mesos/v1/mesos.proto e71ddda 
>   src/common/resources.cpp f1da323 
>   src/tests/resources_tests.cpp 0d084fd 
>   src/v1/resources.cpp 6f50101 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Michael Park]

> On Nov. 19, 2015, 7:09 p.m., Michael Park wrote:
> > docs/persistent-volume.md, lines 255-280
> > 
> >
> > This looks to be formatted weird, could you double check? Here and below
> 
> Neil Conway wrote:
> I previewed with mesos-website-container and it seems fine to me -- what 
> weirdness did you notice?

>From the diff here, it looks like the `-u` and below are indented 1 space more 
>than `-i`.

I just noticed that there is a tab character on line 277 (279 in the latest 
diff).

```
274   "container_path": 
275 }
276   }
277 }  // tab character here
278   ]' \
279   -X POST http://:/master/create-volume
```


- Michael


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


On Nov. 20, 2015, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> ---
> 
> (Updated Nov. 20, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> ---
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Alexander Rukletsov]

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

(Updated Nov. 23, 2015, 12:31 a.m.)


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


Summary (updated)
-

Quota: Implemented recovery in hierarchical allocator.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
c65fe35198b846da2dc959dd467a21ff6edd30a9 
  src/master/allocator/mesos/hierarchical.cpp 
2765526047767cbd19d13c11ecfa6e90c505b3a7 

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


Testing (updated)
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40351: Quota: Added rescinding offers for set quota requests.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2015, 12:57 a.m.)


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


Changes
---

Comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp d4b1edde98925fd51e056f253758afea779be9ed 
  src/master/quota_handler.cpp 86d7331aa79adb1d9a3009552fc4c2aed0229804 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

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

[Guangya Liu]

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



src/slave/slave.cpp (line 4417)


s/oversubscribable/USAGE_SLACK oversubscribable



src/slave/slave.cpp (line 4419)


I think that the oversubscribable resources should always has_revocable()? 
For which case does the oversubscribable resource do not has_revocable()? 
Caused by a bad resource estimitator? For such case, can we let slave throw 
exception?


- Guangya Liu


On 十一月 21, 2015, 11:26 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated 十一月 21, 2015, 11:26 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/resource_estimators/fixed.cpp 305664c 
>   src/slave/slave.cpp d1126f0 
>   src/tests/oversubscription_tests.cpp 0d0bf7e 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40375: [WIP] Support distinguishing revocable resources in the Resource protobuf.

[Guangya Liu]

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



src/common/resources.cpp (lines 94 - 106)


For compatibility issue, shall we take no_type same as USAGE_SLACK?


- Guangya Liu


On 十一月 21, 2015, 3:47 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated 十一月 21, 2015, 3:47 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5ad48bd 
>   include/mesos/v1/mesos.proto e71ddda 
>   src/common/resources.cpp f1da323 
>   src/tests/resources_tests.cpp 0d084fd 
>   src/v1/resources.cpp 6f50101 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

[Guangya Liu]

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



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


Can we add some comments for why using 0.8 as default?



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


The log message should also be updated.


- Guangya Liu


On 十一月 23, 2015, 12:31 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40332/
> ---
> 
> (Updated 十一月 23, 2015, 12:31 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3874
> https://issues.apache.org/jira/browse/MESOS-3874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c65fe35198b846da2dc959dd467a21ff6edd30a9 
>   src/master/allocator/mesos/hierarchical.cpp 
> 2765526047767cbd19d13c11ecfa6e90c505b3a7 
> 
> Diff: https://reviews.apache.org/r/40332/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Klaus Ma]

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



src/common/resources.cpp (line 654)


After confirmed in design dock, `stateless` is `!persistentVolume`.


- Klaus Ma


On Nov. 20, 2015, 8:59 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40529/
> ---
> 
> (Updated Nov. 20, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-3955
> https://issues.apache.org/jira/browse/MESOS-3955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get stateless resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp ce12a26e9cc9057d6612cf2380c07be959e2990b 
>   include/mesos/v1/resources.hpp 92db123507ba3442d475c5d3bb7e1a375cf5c1b2 
>   src/common/resources.cpp f1da3237724b3766a5df1cb0a4c0159fb9098e01 
>   src/tests/resources_tests.cpp 0d084fd97ec108d5ec2d050eddc2e80ea81ffac0 
>   src/v1/resources.cpp 6f50101538cf30ebb5a8022558108f103d62a44c 
> 
> Diff: https://reviews.apache.org/r/40529/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Guangya Liu]

> On 十一月 23, 2015, 1:35 a.m., Guangya Liu wrote:
> > src/master/http.cpp, line 541
> > 
> >
> > Can you please add some comments here to clarify why need this?

Another point is that can we set this as a helper function in resources.cpp?


- Guangya


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


On 十一月 20, 2015, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> ---
> 
> (Updated 十一月 20, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> ---
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40586, 40551, 39450]

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

- Mesos ReviewBot


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

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

[Guangya Liu]

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



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


What about rename it to recoverQuota()?


- Guangya Liu


On 十一月 23, 2015, 12:31 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40332/
> ---
> 
> (Updated 十一月 23, 2015, 12:31 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3874
> https://issues.apache.org/jira/browse/MESOS-3874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c65fe35198b846da2dc959dd467a21ff6edd30a9 
>   src/master/allocator/mesos/hierarchical.cpp 
> 2765526047767cbd19d13c11ecfa6e90c505b3a7 
> 
> Diff: https://reviews.apache.org/r/40332/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37336: Added `execute()` method to process::Subprocess

[Michael Park]

> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 328
> > 
> >
> > I don't like the name 'execute'. When you create the Subprocess 
> > instance, the subprocss is already launched and exec'ed. This is rather 
> > waiting for the subprocess to terminate.
> 
> Marco Massenzio wrote:
> This method most definitely does **not** wait.
> 
> This is how one can use it as a caller (code simplified):
> ```
>   auto s = process::subprocess(commandInfo.command(), args);
> 
>   if (s.isError()) {
> LOG(ERROR) << "Could not spawn subprocess: " << s.error();
> return http::ServiceUnavailable(s.error());
>   }
> 
>   store(s.get().pid());  // <-- needed to reconcile with GETs
> 
>   Future result_ = s->execute();
>   result_.then([](const Future ) {
> if (future.isFailed()) {
>   // mark the command as failed
>   return Nothing();
> }
> auto result = future.get();
> // update status of job - use pid(); something equivalent to:
> LOG(INFO) << "Result of '" << result.invocation.command << "'was: 
> "
>   << result.stdout();
> return Nothing();
>   }).after(Seconds(30), [s](const Future ) {
> // update status of job to timed out; use `invocation` and 
> `stdout`.
> s.get().cleanup();
> return Nothing();
>   });
> 
>   http::Response response = http::OK("{\"result\": \"OK\", \"pid\": \"" +
>  stringify(s.get().pid()) + "\"}");
>   response.headers["Content-Type"] = "application/json";
>   return response;
> ```
> 
> Jie Yu wrote:
> From the code above, can you just caputure commandInfo.command() in the 
> lambda and print it?
> 
> ```
> string command = commandInfo.command();
> 
> result_.then([command](...) {
>   ...
>   LOG(INFO) << command << "...";
> });
> ```
> 
> Jie Yu wrote:
> ALso, `auto s = process::subprocess(commandInfo.command(), args);` this 
> line fork and exec the subprocess. So having another `s->execute()` sounds 
> very confusing to me.
> 
> Marco Massenzio wrote:
> I don't disagree - what would you suggest instead?
> 
> (note about the example above: it's contrived - one can also imagine 
> storing the Future in a map keyed by the id, and retrieve the outcome upon 
> receiving a GET requests on the pid status; there may be other scenarios 
> where just passing the commandInfo and/or the args or whatever in the lambda 
> may be less desirable  -- but, again, this is *one* way of doing things, not 
> necessarily unique, and admittedly maybe not even *the* best).

In terms of the name, why not `communicate` given that the behavior is similar 
to Python's `communicate`?

In terms of whether to store the `Invocation`, I can see how storing it would 
simplify the caller's code in cases where there are multiple async commands.
The caller would otherwise need to manually pair-up the commands to the 
corresponding results, by storing them in different data structures in the same 
order, pairing them up explicitly via `std::pair`, or whatever else.

Having said that, I think it would be great to have an example from our 
codebase to make a stronger argument for this.


- Michael


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


On Nov. 10, 2015, 8:51 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Nov. 10, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added an `execute()` method
> that returns a `Future`.
>  
> `Subprocess::Result`, also introduced with this patch, contains useful 
> information
> about the command invocation (an `Invocation` struct); the exit code; 
> `stdout`;
> and, optionally, `stderr` too.
>  
> Once the Future completes, if successful, the caller will be able to retrieve
> stdout/stderr; whether the command was successful; and whether it received a 
> signal
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> f17816e813d5efce1d3bb1ff1e850eeda3ba 
>   3rdparty/libprocess/src/subprocess.cpp 
> 

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40332]

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

- Mesos ReviewBot


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

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

[Alexander Rukletsov]

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

(Updated Nov. 22, 2015, 11:55 p.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
14fef63714721fcda7cea3c28704766efda6d007 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39401]

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

Error:
 2015-11-23 00:20:07 URL:https://reviews.apache.org/r/39401/diff/raw/ 
[8039/8039] -> "39401.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.cpp:988
error: src/master/allocator/mesos/hierarchical.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 22, 2015, 11:55 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 22, 2015, 11:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Guangya Liu]

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



docs/persistent-volume.md (line 236)


not yours, but here should be MESOS-2408



src/master/http.cpp (line 541)


Can you please add some comments here to clarify why need this?


- Guangya Liu


On 十一月 20, 2015, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> ---
> 
> (Updated 十一月 20, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> ---
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40351: Quota: Added rescinding offers for set quota requests.

[Guangya Liu]

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



src/master/quota_handler.cpp (line 332)


s/enough/as many as?

I'm proposing this because we cannot guarantee there are enough outstanding 
offers to satisify the quota request, comments?


- Guangya Liu


On 十一月 23, 2015, 12:57 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40351/
> ---
> 
> (Updated 十一月 23, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
> https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp d4b1edde98925fd51e056f253758afea779be9ed 
>   src/master/quota_handler.cpp 86d7331aa79adb1d9a3009552fc4c2aed0229804 
> 
> Diff: https://reviews.apache.org/r/40351/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40396: Quota: Added a test for offer rescinding.

[Guangya Liu]

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



src/tests/master_quota_tests.cpp (line 539)


Add ASSERT here to make sure no offers?


- Guangya Liu


On 十一月 20, 2015, 8:21 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40396/
> ---
> 
> (Updated 十一月 20, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
> https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40396/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40497: Add hex number support to numify()

[Cong Wang]

> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 39
> > 
> >
> > Where you planning to output `c` here? Otherwise I suggest replacing 
> > this at least `ss.get()` (and drop the decl above).
> > 
> > I like H.Sutter's synopsis of `boost::lexical_cast` even better since 
> > it shows clearly the failure sites (modulo the throw for here), see 
> > http://www.gotw.ca/publications/mill19.htm:
> > 
> > template
> > Target lexical_cast(Source arg)
> > {
> >   std::stringstream interpreter;
> >   Target result;
> > 
> >   if(!(interpreter << arg) ||
> >  !(interpreter >> result) ||
> >  !(interpreter >> std::ws).eof())
> > throw bad_lexical_cast();
> > 
> >   return result;
> > }
> > 
> > You could do a similar impl just by injection a `<< std::hex`.
> 
> Cong Wang wrote:
> I replace that ss.get() with ss.eof(). And also I don't understand why 
> checking the return value of operator<< could work, it always return the 
> stream right? The document says we should check ss.fail().
> 
> Benjamin Bannier wrote:
> http://en.cppreference.com/w/cpp/io/basic_ios/operator_bool

Ah, I didn't know that operator before. So, what is the advantage of using 
operator bool to ss.fail()? At least ss.fail() is more readable for me.


> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp, line 26
> > 
> >
> > Would be nice to add a couple more positives here, e.g., to catch 
> > `hex_cast` impls like `boost::lexical_cast(s.substr(2)` ;)
> 
> Cong Wang wrote:
> I added some more positives, but not sure if they are 
> boost::lexical_cast(s.substr(2))...
> 
> Benjamin Bannier wrote:
> Hmm, for `0x00` the hex prefix doesn't matter, and just extracting 
> without it would give the same answer `0`. How about testing e.g., `0x10` 
> which really translates to `16`, but to `10` without the prefix?

Sure, I am adding them now.


- Cong


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


On Nov. 20, 2015, 10:26 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 20, 2015, 10:26 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
> not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40586: Corrected a typo and a formatting issue.

[Guangya Liu]

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



src/master/allocator/mesos/hierarchical.hpp (lines 367 - 376)


Sorry, what is the update for this file?


- Guangya Liu


On 十一月 22, 2015, 11:59 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40586/
> ---
> 
> (Updated 十一月 22, 2015, 11:59 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.hpp 8276749efbca167c56e337aa9155fb7db739c1c9 
>   src/master/allocator/mesos/hierarchical.hpp 
> c65fe35198b846da2dc959dd467a21ff6edd30a9 
> 
> Diff: https://reviews.apache.org/r/40586/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40544: Added quota remove handling.

[Joerg Schad]

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

(Updated Nov. 22, 2015, 1:35 p.m.)


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


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


Repository: mesos


Description
---

Added quota remove handling.


Diffs (updated)
-

  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 

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


Testing
---

Test are in the next review.


Thanks,

Joerg Schad

Re: Review Request 40580: Added remove quota test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018, 39102, 36913, 38059, 39285, 38110, 40342, 
40351, 38956, 40396, 39223, 39492, 39614, 40544, 40580]

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

- Mesos ReviewBot


On Nov. 22, 2015, 1:36 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40580/
> ---
> 
> (Updated Nov. 22, 2015, 1:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added remove quota test.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40580/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 40580: Added remove quota test.

[Joerg Schad]

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

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


Repository: mesos


Description
---

Added remove quota test.


Diffs
-

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 40291: Added the stye guideline for blank line after line-wrapping.

[Joerg Schad]

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

(Updated Nov. 22, 2015, 11:52 a.m.)


Review request for mesos, Bernd Mathiske and Neil Conway.


Changes
---

Updated example


Repository: mesos


Description
---

Added the stye guideline for blank line after line-wrapping.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

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


Testing
---

viewed rendered doc.


Thanks,

Joerg Schad

Re: Review Request 40400: Quota: Added registry tests.

[Alexander Rukletsov]

> On Nov. 22, 2015, 6:07 p.m., Joris Van Remoortere wrote:
> > src/tests/registrar_tests.cpp, line 687
> > 
> >
> > Let's use full words. `Alternate`, or we could enumerate them.

The reason why I didin't want to enumerate is to avoid potential confusion, 
because there are `quota1` and `quota2`, which are not directly tighed to 
`quotaResources1` and `quotaResources2`.


- Alexander


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


On Nov. 22, 2015, 5:02 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40400/
> ---
> 
> (Updated Nov. 22, 2015, 5:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3165
> https://issues.apache.org/jira/browse/MESOS-3165
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of https://reviews.apache.org/r/39983
> 
> 
> Diffs
> -
> 
>   src/tests/registrar_tests.cpp f4ed01e9f37f8291921b67f7fb2f54839812c8b1 
> 
> Diff: https://reviews.apache.org/r/40400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joris Van Remoortere]

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

Ship it!



src/master/allocator/mesos/hierarchical.cpp (lines 152 - 153)


Let's add a comment as to why we don't update the quotaRoleSorter here yet 
(i.e. we do it during recovery).



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


Let's account for this in the quota role sorter as well (moving it from the 
next review into this one):
```
if (roles[role].quota.isSome()) {
  quotaRoleSorter->allocated(role, slaveId, resources.unreserved());
}
```


- Joris Van Remoortere


On Nov. 11, 2015, 10:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 11, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40401: Quota: Persisted quota to registry for set request.

[Joris Van Remoortere]

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

Ship it!



src/master/quota_handler.cpp (lines 240 - 241)


Why are we leaving this here?



src/master/quota_handler.cpp (line 243)


s/grant/acknowledge/


- Joris Van Remoortere


On Nov. 22, 2015, 5:02 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40401/
> ---
> 
> (Updated Nov. 22, 2015, 5:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3165
> https://issues.apache.org/jira/browse/MESOS-3165
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of https://reviews.apache.org/r/39974
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
> 
> Diff: https://reviews.apache.org/r/40401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40398: Quota: Extended allocator interface with recovery method.

[Joris Van Remoortere]

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

Ship it!


If we move `struct Quota` from "master.hpp" to "quota.hpp", then we can use the 
wrapped C++ type in the interface.
This makes the interface more maintainable.

- Joris Van Remoortere


On Nov. 22, 2015, 5 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40398/
> ---
> 
> (Updated Nov. 22, 2015, 5 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3873
> https://issues.apache.org/jira/browse/MESOS-3873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of https://reviews.apache.org/r/40330/
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp f76118bbf028610c330cebe937d81457fc67a6f3 
>   src/master/allocator/mesos/allocator.hpp 
> d2d32af227d66c4030becd4cd64b907a70d25f49 
>   src/master/allocator/mesos/hierarchical.hpp 
> 64ccf4164197e59d93d739fa2afbdee2cc2a1d23 
>   src/master/allocator/mesos/hierarchical.cpp 
> f2e3b639f210eb06c70584ee7294609d9fd978ad 
>   src/tests/allocator.hpp e0cb2e75e6cac41ae8d8ed1608f1d64e7c533e34 
> 
> Diff: https://reviews.apache.org/r/40398/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40402: Quota: Loaded quotas upon master recovery.

[Joris Van Remoortere]

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

Ship it!



src/master/master.cpp (lines 1421 - 1423)


With the modified allocator interface that also uses the C++ wrapper, we 
don't need to make an "allocator-friendly" map anymore.



src/master/master.cpp (lines 1429 - 1441)


Let's recover every time. This will be less confusing to the allocator 
writer, especially since the interface comment suggests that the quota map can 
be empty.


- Joris Van Remoortere


On Nov. 17, 2015, 8:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40402/
> ---
> 
> (Updated Nov. 17, 2015, 8:54 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3717
> https://issues.apache.org/jira/browse/MESOS-3717
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populate master bookkeeping structures and notify allocator about quota upon 
> recovery.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
> 
> Diff: https://reviews.apache.org/r/40402/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40329: Removed stale comment.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Nov. 16, 2015, 3:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40329/
> ---
> 
> (Updated Nov. 16, 2015, 3:27 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 7bac0fea4bcd040307fdfdcd002387d5baee46d1 
> 
> Diff: https://reviews.apache.org/r/40329/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40401: Quota: Persisted quota to registry for set request.

[Alexander Rukletsov]

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

(Updated Nov. 22, 2015, 5:02 p.m.)


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


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


Repository: mesos


Description (updated)
---

Follow up of https://reviews.apache.org/r/39974


Diffs
-

  src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40400: Quota: Added registry tests.

[Alexander Rukletsov]

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

(Updated Nov. 22, 2015, 5:02 p.m.)


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


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


Repository: mesos


Description (updated)
---

Follow up of https://reviews.apache.org/r/39983


Diffs
-

  src/tests/registrar_tests.cpp f4ed01e9f37f8291921b67f7fb2f54839812c8b1 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40399: Quota: Introduced quota registry operations.

[Alexander Rukletsov]

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

(Updated Nov. 22, 2015, 5:03 p.m.)


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


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


Repository: mesos


Description (updated)
---

Follow up of https://reviews.apache.org/r/38958


Diffs
-

  src/master/quota.hpp 8fc984003762a339a796b7a516362e21dab0a6e5 
  src/master/quota.cpp 835b191460d86e912ffdac8160459b4a0643bd88 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40399: Quota: Introduced quota registry operations.

[Joris Van Remoortere]

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

Ship it!



src/master/quota.cpp (lines 70 - 85)


We don't need to keep track of changed here right?



src/master/quota.cpp (line 78)


Missing period.


- Joris Van Remoortere


On Nov. 22, 2015, 5:03 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40399/
> ---
> 
> (Updated Nov. 22, 2015, 5:03 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3165
> https://issues.apache.org/jira/browse/MESOS-3165
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of https://reviews.apache.org/r/38958
> 
> 
> Diffs
> -
> 
>   src/master/quota.hpp 8fc984003762a339a796b7a516362e21dab0a6e5 
>   src/master/quota.cpp 835b191460d86e912ffdac8160459b4a0643bd88 
> 
> Diff: https://reviews.apache.org/r/40399/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40400: Quota: Added registry tests.

[Joris Van Remoortere]

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

Ship it!



src/tests/registrar_tests.cpp (line 685)


I don't think the `once` is necessary.



src/tests/registrar_tests.cpp (line 687)


Let's use full words. `Alternate`, or we could enumerate them.



src/tests/registrar_tests.cpp (lines 691 - 693)


Since `Resources` has this as an implicit conversion, why do you need the 
explicit static cast here? (and below)



src/tests/registrar_tests.cpp (lines 722 - 723)


I think there is an implicit constructor that takes a repeated resource 
proto.



src/tests/registrar_tests.cpp (line 745)


We can just reference the `guarantee` here for the implicit constructor.


- Joris Van Remoortere


On Nov. 22, 2015, 5:02 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40400/
> ---
> 
> (Updated Nov. 22, 2015, 5:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3165
> https://issues.apache.org/jira/browse/MESOS-3165
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of https://reviews.apache.org/r/39983
> 
> 
> Diffs
> -
> 
>   src/tests/registrar_tests.cpp f4ed01e9f37f8291921b67f7fb2f54839812c8b1 
> 
> Diff: https://reviews.apache.org/r/40400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40398: Quota: Extended allocator interface with recovery method.

[Alexander Rukletsov]

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

(Updated Nov. 22, 2015, 5 p.m.)


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


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


Repository: mesos


Description (updated)
---

Follow up of https://reviews.apache.org/r/40330/


Diffs
-

  include/mesos/master/allocator.hpp f76118bbf028610c330cebe937d81457fc67a6f3 
  src/master/allocator/mesos/allocator.hpp 
d2d32af227d66c4030becd4cd64b907a70d25f49 
  src/master/allocator/mesos/hierarchical.hpp 
64ccf4164197e59d93d739fa2afbdee2cc2a1d23 
  src/master/allocator/mesos/hierarchical.cpp 
f2e3b639f210eb06c70584ee7294609d9fd978ad 
  src/tests/allocator.hpp e0cb2e75e6cac41ae8d8ed1608f1d64e7c533e34 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40568: Windows: 2/3 Added zlib compilation steps for Windows.

[Dario Bazan]

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

(Updated Nov. 22, 2015, 5:54 p.m.)


Review request for mesos and Alex Clemmer.


Repository: mesos


Description
---

Windows: 2/3 Added zlib compilation steps for Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
9b61376ea6aad304607c20c9823d9ef19013eca0 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
a9cd9902567cef4c7ab4125463a68e19907db0b4 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
2a37fdb6501aaf7baac2ada0a714bbe67e7c5aca 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
6eda46b0e08829b269b13a78f22510e6d03940d9 

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


Testing
---


Thanks,

Dario Bazan

Re: Review Request 40351: Quota: Added rescinding offers for set quota requests.

[Joris Van Remoortere]

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



src/master/master.hpp (line 898)


sat/at the/



src/master/master.hpp (line 903)


s/as much/as many/
s/are in/are in the/



src/master/master.hpp (line 905)


s/increases chances a/increases the chances that a/



src/master/master.hpp (line 908)


s/but not necessarily happening, remember fair shares/but not guaranteed, 
due to fair sharing)



src/master/quota_handler.cpp (lines 162 - 164)


Let's drop the `num`. If you really want to be explicit you can postfix 
with `Count`.



src/master/quota_handler.cpp (line 164)


This suggests we're counting the total number of frameworks with a quota; 
which we are not doing.
Maybe it's better to call this something like `frameworksInRole`



src/master/quota_handler.cpp (line 180)


s/at time/at once/



src/master/quota_handler.cpp (lines 180 - 181)


"Offers that do
  // not contribute to satisfying quota request will be rescinded 
regardless."
  Not sure what you mean here.



src/master/quota_handler.cpp (lines 184 - 186)


I think `numVA`, and `numVisitedAgents` below should only be incremented 
when we actually have an offer to rescind. Currently we increment it even if 
the agent is packed.



src/master/quota_handler.cpp (lines 211 - 220)


I don't understand your argument here.
As long as we set quota before we rescind, the next `allocate` call 
triggered by any rescinded offer will go through the `quota` stage first.



src/master/quota_handler.cpp (lines 324 - 328)


We should set the quota before we rescind so that we avoid the race above 
;-)
Please add a comment as to why we want to do them in the new order.


- Joris Van Remoortere


On Nov. 19, 2015, 5:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40351/
> ---
> 
> (Updated Nov. 19, 2015, 5:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
> https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
> 
> Diff: https://reviews.apache.org/r/40351/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>