Re: Review Request 39285: Added Quota Request Validation.

[Joris Van Remoortere]

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



src/master/master.hpp (line 876)


Why is this protected?



src/master/quota.hpp (lines 35 - 38)


let's indent the list.



src/master/quota.cpp (lines 36 - 37)


Why are these inside the namespaces as opposed to above?



src/master/quota.cpp (line 43)


shouldn't this check that there is at least 1 resource?



src/master/quota.cpp (line 49)


It's not necessarily a "request" at this point right? It's an arbitrary 
`QuotaInfo`?
Same below



src/master/quota.cpp (line 70)


s/Check all/Check that all/



src/master/quota.cpp (lines 72 - 83)


your previous error messages specified what the requirement was. These 
specify what is wrong. Please stay consistent.



src/master/quota.cpp (line 78)


new line



src/master/quota.cpp (line 81)


trailing whitespace



src/master/quota.cpp (line 84)


What do you think about a space after the comma?



src/master/quota_handler.cpp (lines 52 - 53)


Should we call this something like `createQuotaInfo` to math other similar 
factory functions?



src/master/quota_handler.cpp (line 55)


specifically: it's being constructed from json here.
Might be valuable to log the request, if we bother to log the action?



src/master/quota_handler.cpp (lines 67 - 87)


How come we don't leverage the validation routine you introduced in this 
patch here?



src/master/quota_handler.cpp (line 100)


Expected, followed by actual



src/master/quota_handler.cpp (line 106)


should we provide the query string that we couldn't decode?
Same for error messages below.



src/master/quota_handler.cpp (line 111)


Why the hashmap `Option get()` pattern versus `contains(key)`?



src/master/quota_handler.cpp (line 116)


same here regarding the get pattern question



src/master/quota_handler.cpp (lines 122 - 126)


If we call it `createQuotaInfo` then we can stay consistent in the 
terminology (for example here we're already using extract and convert to talk 
about the same action)



src/master/quota_handler.cpp (line 132)


Here you reference the kind of request (i.e. `set quota`). In the other 
messages you don't. Let's stay consistent. I actually like referencing the 
request type like you did in this error message :-)



src/master/quota_handler.cpp (line 145)


s/Check we/Check that we/


- Joris Van Remoortere


On Nov. 10, 2015, 6:20 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39285/
> ---
> 
> (Updated Nov. 10, 2015, 6:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3199
> https://issues.apache.org/jira/browse/MESOS-3199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Request Validation.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39285/diff/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40177: Re-checkpoint frameworks after agent recovery.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40177]

All tests passed.

- Mesos ReviewBot


On Nov. 11, 2015, 5:59 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40177/
> ---
> 
> (Updated Nov. 11, 2015, 5:59 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Vinod Kone.
> 
> 
> Bugs: MESOS-3834
> https://issues.apache.org/jira/browse/MESOS-3834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When performing an upgrade cycle, it is possible for a 0.24 and
> later agent to recover from a framework checkpoint written by 0.22
> or earlier. In this case, we need to compatibly accept a missing
> FrameworkID, and then rewrite the framework checkpoint so that
> subsequent upgrades don't hit the same problem.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
>   src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
> 
> Diff: https://reviews.apache.org/r/40177/diff/
> 
> 
> Testing
> ---
> 
> make check on CentOS 6.7.
> Manual testing with a rolling upgrade from 0.22
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 40177: Re-checkpoint frameworks after agent recovery.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40177]

All tests passed.

- Mesos ReviewBot


On Nov. 11, 2015, 5:59 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40177/
> ---
> 
> (Updated Nov. 11, 2015, 5:59 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Vinod Kone.
> 
> 
> Bugs: MESOS-3834
> https://issues.apache.org/jira/browse/MESOS-3834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When performing an upgrade cycle, it is possible for a 0.24 and
> later agent to recover from a framework checkpoint written by 0.22
> or earlier. In this case, we need to compatibly accept a missing
> FrameworkID, and then rewrite the framework checkpoint so that
> subsequent upgrades don't hit the same problem.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
>   src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
> 
> Diff: https://reviews.apache.org/r/40177/diff/
> 
> 
> Testing
> ---
> 
> make check on CentOS 6.7.
> Manual testing with a rolling upgrade from 0.22
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 40177: Re-checkpoint frameworks after agent recovery.

[James Peach]

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

Review request for mesos, Kapil Arya and Vinod Kone.


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


Repository: mesos


Description
---

When performing an upgrade cycle, it is possible for a 0.24 and
later agent to recover from a framework checkpoint written by 0.22
or earlier. In this case, we need to compatibly accept a missing
FrameworkID, and then rewrite the framework checkpoint so that
subsequent upgrades don't hit the same problem.


Diffs
-

  src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
  src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 

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


Testing
---

make check on CentOS 6.7.
Manual testing with a rolling upgrade from 0.22


Thanks,

James Peach

Re: Review Request 40151: Windows: Implemented pids() to get all pids.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40151]

All tests passed.

- Mesos ReviewBot


On Nov. 11, 2015, 2:10 a.m., Matt Dotson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40151/
> ---
> 
> (Updated Nov. 11, 2015, 2:10 a.m.)
> 
> 
> Review request for Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented pids() to get all pids.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40151/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matt Dotson
> 
>

Re: Review Request 38110: Quota: Checked sanity of quota set requests.

[Guangya Liu]

> On Nov. 3, 2015, 5:52 a.m., Guangya Liu wrote:
> > src/master/quota_handler.cpp, lines 159-186
> > 
> >
> > How to handle the following cases? Say two roles request quota: r1 and 
> > r2.
> > 
> > When r1 request quota as 5, the availableInCluster is 10 and r1 set 
> > quota succeed.
> > 
> > When r2 request quota as 7 again, the availableInCluster is still 10 as 
> > no one used resources in the cluster, so r2 set quota also succeed.
> > 
> > But now the quota for r1 and r2 is 12 which exceeds 10, is this a valid 
> > case? Thanks.
> 
> Alexander Rukletsov wrote:
> This is a sanity check, the precise bookkeeping and quota allocation is 
> done by an allocator module.

OK, got it. So we have two levels check, one is sanity check here and the other 
is precise bookkeeping in allocator. The first sanity check can get some early 
exception if there are not enough resources.


- Guangya


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


On Nov. 10, 2015, 7:02 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38110/
> ---
> 
> (Updated Nov. 10, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3074
> https://issues.apache.org/jira/browse/MESOS-3074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performs a check whether a quota request is reasonable and can be satisfied 
> at the moment. A precise answer is impossible here, but a sanity check is 
> still helpful, because it allows us to filter knowingly unsatisfiable 
> requests.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38110/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40151: Windows: Implemented pids() to get all pids.

[Matt Dotson]

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

(Updated Nov. 11, 2015, 2:01 a.m.)


Review request for mesos and Alex Clemmer.


Repository: mesos


Description
---

Windows: Implemented pids() to get all pids.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows.hpp PRE-CREATION 

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


Testing
---


Thanks,

Matt Dotson

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Till Toenshoff]

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

Ship it!


Given that gmock ist not (and never should be, according to its authors) 
installed on the user machine but used as bundled dependencies, this could be 
done using quotation-marks;

```
#include "gmock/gmock.h"
```

However, the differences of sharp brackets and quotation-marks appears to be 
pretty much implementation specific, when looking at the standard. 

>6.10.2 Source file inclusion
>Constraints

>1 A #include directive shall identify a header or source file that can be 
>processed by the
>implementation.
>Semantics

>2 A preprocessing directive of the form
># include  new-line
>searches a sequence of implementation-defined places for a header identified 
>uniquely by
>the specified sequence between the < and > delimiters, and causes the 
>replacement of that
>directive by the entire contents of the header. How the places are specified 
>or the header
>identified is implementation-defined.

>3 A preprocessing directive of the form
># include "q-char-sequence" new-line
>causes the replacement of that directive by the entire contents of the source 
>file identified
>by the specified sequence between the " delimiters. The named source file is 
>searched
>for in an implementation-defined manner. If this search is not supported, or 
>if the search
>fails, the directive is reprocessed as if it read
># include  new-line
>with the identical contained sequence (including > characters, if any) from 
>the original
>directive

So our Mesos practice is not defined by standard limitations as the standard 
leaves pretty much everything to the implementation. On the other hand, the 
Mesos practice is definitely not uncommon - Xcode for example recommends using 
quotes for "user headers" and brackets for anthing else. For this specific case 
of a bundled but not installable dependency, I think we should stick with sharp 
brackets as this will allow quick recognition of non Mesos headers.

Thanks for this, Joerg!

- Till Toenshoff


On Nov. 10, 2015, 6:30 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-3879
> https://issues.apache.org/jira/browse/MESOS-3879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f3

Re: Review Request 40162: Updated docker build script to install libev package.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39953, 39954, 39914, 40162]

All tests passed.

- Mesos ReviewBot


On Nov. 10, 2015, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40162/
> ---
> 
> (Updated Nov. 10, 2015, 11:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker build script to install libev package.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh cea5eaded739bc336e818a081fa308fb6b66fbef 
> 
> Diff: https://reviews.apache.org/r/40162/diff/
> 
> 
> Testing
> ---
> 
> tested locally and on jenkins.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39914: Changed verify_review to use docker_build.sh.

[Jojy Varghese]

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

(Updated Nov. 11, 2015, 12:14 a.m.)


Review request for mesos, Cody Maloney and Vinod Kone.


Changes
---

updated description.


Repository: mesos


Description (updated)
---

reviewbot builds dont use docker today. This change would make reviewbot 
environment to be same as mesos builds on jenkins which  uses docker.


Diffs
-

  support/verify_reviews.py 0d322a19af719aba8846fbc705bf8ec7647304a3 

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


Testing
---

ran script locally


Thanks,

Jojy Varghese

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

[Qian Zhang]

> On Nov. 1, 2015, 8:14 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1005
> > 
> >
> > For this TODO, what do we plan to do in future? Include the dynamic 
> > reserved resources for this role on this agent in 
> > ```roleConsumedResources```? And what about the static reserved resources?
> 
> Alexander Rukletsov wrote:
> Dynamic reservations should account towards role's quota. [Note about 
> static 
> reservations](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)
> 
> Qian Zhang wrote:
> So when we check if a role's quota is satisfied or not, we will add the 
> role's allocated resources with the role's dynamically reserved resources, 
> and check if the sum contains the role's quota. But for role's statically 
> reserved resources, we will consider they are part of quota. So in future 
> (after these TODOs are handled) when we check if a role's quota is satified 
> or not, the formula should be ```(role's allocated resource + role's 
> dynamically reserved resources) > (role's quota + role's statically reserved 
> resources)```, right?

Alex, any comments for my question above? :-)


- Qian


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


On Nov. 10, 2015, 6:01 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 10, 2015, 6:01 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.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 40162: Updated docker build script to install libev package.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39953, 39954, 39914]

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

Error:
 2015-11-10 23:48:16 URL:https://reviews.apache.org/r/39914/diff/raw/ [908/908] 
-> "39914.patch" [1]
error: patch failed: support/verify_reviews.py:115
error: support/verify_reviews.py: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40162/
> ---
> 
> (Updated Nov. 10, 2015, 11:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker build script to install libev package.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh cea5eaded739bc336e818a081fa308fb6b66fbef 
> 
> Diff: https://reviews.apache.org/r/40162/diff/
> 
> 
> Testing
> ---
> 
> tested locally and on jenkins.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 40162: Updated docker build script to install libev package.

[Jojy Varghese]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Updated docker build script to install libev package.


Diffs
-

  support/docker_build.sh cea5eaded739bc336e818a081fa308fb6b66fbef 

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


Testing
---

tested locally and on jenkins.


Thanks,

Jojy Varghese

Re: Review Request 39914: Changed verify_review to use docker_build.sh.

[Jojy Varghese]

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

(Updated Nov. 10, 2015, 11:46 p.m.)


Review request for mesos, Cody Maloney and Vinod Kone.


Changes
---

just keeping libev in default configuration.


Repository: mesos


Description
---

SSL based tests are not currently verified on reviewbot. This change would make 
reviewbot environment to be same as mesos builds on jenkins.


Diffs (updated)
-

  support/verify_reviews.py 0d322a19af719aba8846fbc705bf8ec7647304a3 

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


Testing
---

ran script locally


Thanks,

Jojy Varghese

Re: Review Request 39628: Clear the suppressed flag when deactive a framework

[Vinod Kone]

> On Oct. 25, 2015, 2:51 p.m., Ben Mahler wrote:
> > Can you please add a test that would have caught this issue?
> 
> Guangya Liu wrote:
> I think this is a bug, I tested without my code change, the test also 
> failed sometimes. Shall we file a bug for this?
> 
> Vinod Kone wrote:
> the failed test looks unrelated. please do file a bug. also, please write 
> a test for this change.
> 
> Guangya Liu wrote:
> JIRA ticket for this https://issues.apache.org/jira/browse/MESOS-3813
> 
> Guangya Liu wrote:
> Vinod, I found that the unit test can not cover this part as the test 
> framework cannot get the variable values in hierarchical.cpp. 
> 
> What I want to test is make sure the value of 
> frameworks[frameworkId].suppressed is false, but the problem is that the test 
> framework cannot get this value. Comments?
> 
> Guangya Liu wrote:
> Vinod, any comments on this? Thanks!

But you can test that a re-activated framework gets offers right? in other 
words, restart the framework after it calls suppress and make sure it gets 
offers.


- Vinod


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


On Oct. 30, 2015, 6:52 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39628/
> ---
> 
> (Updated Oct. 30, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3802
> https://issues.apache.org/jira/browse/MESOS-3802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clear the suppressed flag when deactive a framework
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39628/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make 
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 11:06 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 (updated)
-

  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 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 359
> > 
> >
> > s/the/this

Re-phrasing it a bit deeper.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 360
> > 
> >
> > Quota is satisfied before fair share, ...

Chose different phrasing.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 137
> > 
> >
> > `Resources`

This is neither a type, nor an instance, but a proper english word.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 416
> > 
> >
> > `resources`

This is neither a type, nor an instance, but a proper english word.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 582
> > 
> >
> > `resources`

This is neither a type, nor an instance, but a proper english word.


- Alexander


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


On Nov. 5, 2015, 6:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 5, 2015, 6:25 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39399: Quota: Refactored hierarchical allocator in preparation for quota.

[Alexander Rukletsov]

> On Nov. 10, 2015, 8:56 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 354
> > 
> >
> > s/tie quota to/associate quota with/

Will move the comment into the next review request, but will update it there.


- Alexander


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


On Nov. 10, 2015, 11:02 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39399/
> ---
> 
> (Updated Nov. 10, 2015, 11:02 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.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39399/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39399: Quota: Refactored hierarchical allocator in preparation for quota.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 11:02 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 (updated)
-

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

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Guangya Liu]

> On Nov. 10, 2015, 10:41 p.m., Guangya Liu wrote:
> > src/tests/containerizer.hpp, line 19
> > 
> >
> > I see that we have some discussion in mail list, can you please add 
> > more detail for what we discussed in the "Description" section so that 
> > everyone will know how we order the include files in the future.
> 
> Joerg Schad wrote:
> Hi, the discussion on the mailing list is actually more general and the 
> review in discussion contains a large example. This is not the complete fix 
> for the entire discussion on the mailing list, but more about a) consistency 
> (as currently these includes are ordered very inconsistent across files) and 
> b) the fact that  is a custom .h header (hence it is clearly 
> specified by our styleguide how they should be ordered). So in general you 
> should order your includes as specified in the Google Styleguide (and Jan's 
> review will give more examples for that).

I see, thanks Joerg!


- Guangya


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


On Nov. 10, 2015, 6:30 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-3879
> https://issues.apache.org/jira/browse/MESOS-3879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager

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

[Alexander Rukletsov]

> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 140
> > 
> >
> > Maybe s/Introduce/Consider introducing/
> > Are we sure we want to do this?

A very good point. I actually think each `TODO` which states something should 
have been converted to code. The only reason to leave a `TODO` is because there 
is a certain level of uncertainty.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 894
> > 
> >
> > let's clarify `just updates the numbers`. Also missing a period.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 906
> > 
> >
> > A comment as to why we want to `allocate()` here would be useful.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 927
> > 
> >
> > A comment here would be helpful as well.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 225
> > 
> >
> > I think we can add an equivalent comment as above this for loop (not 
> > your fault):
> > `// Update the allocation to this framework.`

Will do, please verify if this is what you would like to see here.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 330
> > 
> >
> > same as above. let's add a comment here.

Will do, please verify if this is what you would like to see here.


- Alexander


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


On Nov. 5, 2015, 6:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 5, 2015, 6:25 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Joerg Schad]

> On Nov. 10, 2015, 10:41 p.m., Guangya Liu wrote:
> > src/tests/containerizer.hpp, line 19
> > 
> >
> > I see that we have some discussion in mail list, can you please add 
> > more detail for what we discussed in the "Description" section so that 
> > everyone will know how we order the include files in the future.

Hi, the discussion on the mailing list is actually more general and the review 
in discussion contains a large example. This is not the complete fix for the 
entire discussion on the mailing list, but more about a) consistency (as 
currently these includes are ordered very inconsistent across files) and b) the 
fact that  is a custom .h header (hence it is clearly specified 
by our styleguide how they should be ordered). So in general you should order 
your includes as specified in the Google Styleguide (and Jan's review will give 
more examples for that).


- Joerg


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


On Nov. 10, 2015, 6:30 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-3879
> https://issues.apache.org/jira/browse/MESOS-3879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager_tests.cpp 
> 82a985518276ac1015056a450634429840607524 
>   src/tes

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

[Alexander Rukletsov]

> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 925
> > 
> >
> > Do you want to -symetrically to your todu when setting quota- print the 
> > actual quota removed?

I was thinking about it, do you think it adds extra value?


- Alexander


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


On Nov. 5, 2015, 6:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 5, 2015, 6:25 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Guangya Liu]

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



src/tests/containerizer.hpp (line 19)


I see that we have some discussion in mail list, can you please add more 
detail for what we discussed in the "Description" section so that everyone will 
know how we order the include files in the future.


- Guangya Liu


On Nov. 10, 2015, 6:30 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-3879
> https://issues.apache.org/jira/browse/MESOS-3879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager_tests.cpp 
> 82a985518276ac1015056a450634429840607524 
>   src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
>   src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
>   src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
>   src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 
> 
> Diff: https://reviews.apache.org/r/40142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 39223: Added Quota Request Validation Tests.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 10:27 p.m.)


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


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


Repository: mesos


Description
---

see Summary.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 39399: Quota: Refactored hierarchical allocator in preparation for quota.

[Alexander Rukletsov]

> On Nov. 10, 2015, 8:56 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 337
> > 
> >
> > let's pull fixes like this out separately. I'll gladly commit it as a 
> > distinct patch.

I would argue that this review is actually that distinct patch. It is not 
directly related to quota, shall I remove "quota" from the description and move 
that single comment to the next review for clarity?


> On Nov. 10, 2015, 8:56 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 130
> > 
> >
> > Can you split out the removal of the namespaces (e.g. `std::` and 
> > `mesos::master::`) in to a separate review so that we can keep the quota 
> > specific changes concise?

As I mentioned above, this review is a basic refactoring, which removes 
namespaces and wrap role related stuff in a dedicated type.


- Alexander


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


On Nov. 5, 2015, 6:23 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39399/
> ---
> 
> (Updated Nov. 5, 2015, 6:23 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39399/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Joseph Wu]

> On Nov. 5, 2015, 4:39 p.m., Joseph Wu wrote:
> > src/tests/master_quota_tests.cpp, lines 110-112
> > 
> >
> > The resource string you have is equivalent to the resource string from 
> > `CreateSlaveFlags`.  Is there some reason you're re-setting it?
> 
> Alexander Rukletsov wrote:
> One of the comments I got earlier from Bernd is that it is hard to track 
> what resources are available, hence I decided to re-introduce the resources 
> explicitly.

Can you add a comment about this to the test?


> On Nov. 5, 2015, 4:39 p.m., Joseph Wu wrote:
> > src/tests/master_quota_tests.cpp, lines 147-148
> > 
> >
> > Rather than re-defining this, can you populate it via the result of 
> > `MesosTest::CreateSlaveFlags().resources`?
> 
> Alexander Rukletsov wrote:
> Do you suggest to call `MesosTest::CreateSlaveFlags()` in the `SetUp()` 
> in order to use just the `.resources` artifact? Or you think I should make 
> `defaultAgentResources` local in every test?

This ties in with the above issue.  Given what Bernd/you said, what you have is 
probably ok.


- Joseph


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


On Nov. 10, 2015, 2 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38956/
> ---
> 
> (Updated Nov. 10, 2015, 2 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3720
> https://issues.apache.org/jira/browse/MESOS-3720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38956/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 10 p.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39331: Support docker local store pull image simultaneously

[Jojy Varghese]

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



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 136)


Wondering if you need to pass "recursive" (true) to mkdir. This flag 
ensures that the the whole path of dierctory is created as needed. If its 
already existing, it wont create one.


- Jojy Varghese


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> ---
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3736
> https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is 
> written into the hashmap. All the other requests will be waiting for the 
> future of the first request. But because its return type is 
> 'Future>', if its future status is 'FAILED/DISCARDED', the 
> other requests will be waiting forever. 
> Solved by logic check: if it is the first call to get() Image_A, promise 
> associate with metadateManager->get(). If not, check whether that promised 
> future failed/discarded. If yes, over write to the hashmap.
> 
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not 
> be unique because there is chance that layer_ids can be changed. 
> One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Alexander Rukletsov]

> On Nov. 6, 2015, 12:39 a.m., Joseph Wu wrote:
> > src/tests/master_quota_tests.cpp, lines 147-148
> > 
> >
> > Rather than re-defining this, can you populate it via the result of 
> > `MesosTest::CreateSlaveFlags().resources`?

Do you suggest to call `MesosTest::CreateSlaveFlags()` in the `SetUp()` in 
order to use just the `.resources` artifact? Or you think I should make 
`defaultAgentResources` local in every test?


- Alexander


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


On Nov. 10, 2015, 1:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38956/
> ---
> 
> (Updated Nov. 10, 2015, 1:22 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3720
> https://issues.apache.org/jira/browse/MESOS-3720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38956/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39331: Support docker local store pull image simultaneously

[Timothy Chen]

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



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 136)


Actually we don't need to check exists before mkdir, so you can remove this 
check.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 240)


This is not right, you need to put the removing from the pulling hashmap in 
the onAny block in line 230, because this is block is not called if no one 
requests the same image in the same time, and will leave it in the hashmap.


- Timothy Chen


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> ---
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3736
> https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is 
> written into the hashmap. All the other requests will be waiting for the 
> future of the first request. But because its return type is 
> 'Future>', if its future status is 'FAILED/DISCARDED', the 
> other requests will be waiting forever. 
> Solved by logic check: if it is the first call to get() Image_A, promise 
> associate with metadateManager->get(). If not, check whether that promised 
> future failed/discarded. If yes, over write to the hashmap.
> 
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not 
> be unique because there is chance that layer_ids can be changed. 
> One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37336]

All tests passed.

- Mesos ReviewBot


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 
> efe0018d0414c4137fd833c153eb262232e712bc 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> (also tested functionality with an anonymous module that exposes an 
> `/execute` endpoint and runs arbitrary commands, asynchronously,
> on an Agent)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 39289: Quota: Added authorization of quota requests.

[Joseph Wu]

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


Looks good overall.


docs/authorization.md (line 35)


Can you also change the "framework" (in `Resource roles that framework 
can`) to plural?



src/master/quota_handler.cpp (line 318)


Can you add a comment here (or somewhere describing this helper) about how 
`validateQuotaRequest` will always set the role?  Otherwise, it looks like 
you're checking one optional field, but not this one.


- Joseph Wu


On Nov. 10, 2015, 4:56 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39289/
> ---
> 
> (Updated Nov. 10, 2015, 4:56 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization of quota requests.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   include/mesos/quota/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
>   src/master/master.hpp 3aa7017287506938d77bbfb1e06f510101009ee3 
>   src/master/quota_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
> 
> Diff: https://reviews.apache.org/r/39289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Marco Massenzio]

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


Changes
---

Addressed mpark's comments


Summary (updated)
-

Added `execute()` method to process::Subprocess


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

  3rdparty/libprocess/include/process/subprocess.hpp 
f17816e813d5efce1d3bb1ff1e850eeda3ba 
  3rdparty/libprocess/src/subprocess.cpp 
efe0018d0414c4137fd833c153eb262232e712bc 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ac600a551fb1a7782ff33cce204b7819497ef54a 

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


Testing
---

make check

(also tested functionality with an anonymous module that exposes an `/execute` 
endpoint and runs arbitrary commands, asynchronously,
on an Agent)


Thanks,

Marco Massenzio

Re: Review Request 40129: Updated apply-review.sh to use apply-reviews.py.

[Vinod Kone]

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

Ship it!



support/apply-review.sh 


do we even need this script anymore? is this for backwards compatiblity 
with existing tooling? is that plan to kill this eventually? worth adding some 
comments/TODO/JIRA for this.


- Vinod Kone


On Nov. 10, 2015, 8:02 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40129/
> ---
> 
> (Updated Nov. 10, 2015, 8:02 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-3868
> https://issues.apache.org/jira/browse/MESOS-3868
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated apply-review.sh to use apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
> 
> Diff: https://reviews.apache.org/r/40129/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39420: Added '--chain' option to apply-reviews.py.

[Vinod Kone]

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

Ship it!



support/apply-reviews.py (line 320)


not always right? want to update this comment?



support/apply-reviews.py (line 321)


pull this comment inside?


- Vinod Kone


On Nov. 10, 2015, 8:03 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated Nov. 10, 2015, 8:03 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--chain' option to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - With and without '-c'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39410: Added support for github to apply-reviews.py.

[Vinod Kone]

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

Ship it!



support/apply-reviews.py (line 188)


s/-e/--amend/ ?


- Vinod Kone


On Nov. 10, 2015, 7:48 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 10, 2015, 7:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 40146: Windows: Unifies POSIX and Windows PSTree implementations

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39537, 39538, 39539, 39540, 39541, 39383, 39559]

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

Error:
 2015-11-10 20:16:27 URL:https://reviews.apache.org/r/39559/diff/raw/ 
[10306/10306] -> "39559.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am:65
error: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am: patch does not 
apply
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:83
error: 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp: patch does not 
apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 8:14 p.m., Steve Butler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40146/
> ---
> 
> (Updated Nov. 10, 2015, 8:14 p.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Bugs: MESOS-3881
> https://issues.apache.org/jira/browse/MESOS-3881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Unifies POSIX and Windows PSTree implementations
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 741639a942971e48e2dac42db238d423e61cac21 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pstree.hpp 
> fd0192ca021eb0211b293eb2f4e521d588aff04f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/pstree.hpp 
> 9baa2fbfafa310517b70d54f190167f1cccbad6a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pstree.hpp 
> f75a77fc58db09fadf80409f506852e48a7df7c4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/40146/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Butler
> 
>

Re: Review Request 40146: Windows: Unifies POSIX and Windows PSTree implementations

[Steve Butler]

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

(Updated Nov. 10, 2015, 8:14 p.m.)


Review request for mesos and Alex Clemmer.


Changes
---

Fixed up review comments and suggestions


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


Repository: mesos


Description
---

Windows: Unifies POSIX and Windows PSTree implementations


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
741639a942971e48e2dac42db238d423e61cac21 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pstree.hpp 
fd0192ca021eb0211b293eb2f4e521d588aff04f 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pstree.hpp 
9baa2fbfafa310517b70d54f190167f1cccbad6a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pstree.hpp 
f75a77fc58db09fadf80409f506852e48a7df7c4 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
1a7037d64afeedc340258c92067e95d1d3caa027 

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


Testing
---


Thanks,

Steve Butler

Re: Review Request 37336: Simplified the caller interface to process::Subprocess

[Marco Massenzio]

> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 91
> > 
> >
> > How come `Invocation` is within `Subprocess::Result`? Wouldn't it make 
> > more sense for it to be at the `Subprocess` level to be 
> > `Subprocess::Invocation`?

I probably misunderstood your earlier comment.
Moved one level up, into `Subprocess`.


> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 278
> > 
> >
> > We tend to not use `getX()` style naming for accessors. How about 
> > `outData()`?

great point (wasn't sure, as it's not spelled out in our style guide - and it's 
not truly a "getter" in the Google sytle sense: `foo_` -> `foo()`)
also renamed `stderr_` to `errData` and `getStderr()` to `errData()` - please 
let me know if that was "overreach"!


> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 340
> > 
> >
> > I think this violates our use of `auto` rule. Could you please spell 
> > out the type?

I would disagree here... the type is `Try>` and 
adding that, I don't think makes the code any more readable (on the contrary).
In fact, we don't really make use of any of that - all we care is that no error 
is returned: it could very well be a `Try` for all we care :)

This seems to me a poster child of the use-case for `auto`... anyways, no 
biggie, I've added the type.


> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, lines 202-203
> > 
> >
> > Why do we need the `promise` stuff here?
> > 
> > Does something like the following not work?
> > 
> > ```cpp
> > Future>, Future, Future>> 
> > result =
> >   await(status(), getStdout(), getStderr());
> > 
> > return result.then([=](...) { ... })
> >  .onFailed([=](...) { ... })
> >  .onDiscard([this]() { cleanup(); });
> > ```

I don't quite remember what it was that I came across originally when I tried 
the above (sans Promise) then Joris suggested the pattern that you see here 
now, and this works. IIRC I could never get the `onFailed` to get invoked for 
failed commands, so the pending process never got cleaned up.

If you feel strongly this should not be here (and/or are confident the pattern 
above works) I'm happy to have another go at it.


- Marco


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


On Nov. 6, 2015, 6:24 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Nov. 6, 2015, 6:24 a.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 
> efe0018d0414c4137fd833c153eb262232e712bc 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> (also tested functionality with an anonymous module that exposes an 
> `/execute` endpoint and runs arbitrary commands, asynchronously,
> on an Agent)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 39484: Add resource usage section to MesosContainerizer and DockerContainerizer documentation

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39484]

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

Error:
 2015-11-10 19:26:22 URL:https://reviews.apache.org/r/39484/diff/raw/ 
[2788/2788] -> "39484.patch" [1]
error: docs/containerizer.md: does not exist in index
error: patch failed: docs/docker-containerizer.md:77
error: docs/docker-containerizer.md: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 7:17 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39484/
> ---
> 
> (Updated Nov. 10, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Kapil Arya, Michael Park, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-3113
> https://issues.apache.org/jira/browse/MESOS-3113
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add resource usage section to MesosContainerizer and DockerContainerizer 
> documentation
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
>   docs/docker-containerizer.md 5f8a1a9242bb795b56ac8274b9cf9a7324cbddb0 
> 
> Diff: https://reviews.apache.org/r/39484/diff/
> 
> 
> Testing
> ---
> 
> Github gist
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39851: Windows: fixed ambiguousity error in `process/owned.hpp`.

[Joseph Wu]

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

Ship it!


Double-checked non-Windows build.

- Joseph Wu


On Nov. 2, 2015, 11:56 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39851/
> ---
> 
> (Updated Nov. 2, 2015, 11:56 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: fixed ambiguousity error in `process/owned.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> 1b41477d555ce567c58033f8993dc2ae0ac80a05 
> 
> Diff: https://reviews.apache.org/r/39851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39484: Add resource usage section to MesosContainerizer and DockerContainerizer documentation

[Gilbert Song]

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

(Updated Nov. 10, 2015, 11:17 a.m.)


Review request for mesos, Kapil Arya, Michael Park, Till Toenshoff, and Timothy 
Chen.


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


Repository: mesos


Description
---

Add resource usage section to MesosContainerizer and DockerContainerizer 
documentation


Diffs (updated)
-

  docs/containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
  docs/docker-containerizer.md 5f8a1a9242bb795b56ac8274b9cf9a7324cbddb0 

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


Testing
---

Github gist


Thanks,

Gilbert Song

Re: Review Request 38110: Quota: Checked sanity of quota set requests.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 7:02 p.m.)


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


Changes
---

Removed extra logging.


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


Repository: mesos


Description
---

Performs a check whether a quota request is reasonable and can be satisfied at 
the moment. A precise answer is impossible here, but a sanity check is still 
helpful, because it allows us to filter knowingly unsatisfiable requests.


Diffs (updated)
-

  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39285: Added Quota Request Validation.

[Joseph Wu]

The tree structure of the reviews is intentional (at the moment).
It's currently roughly split into components like:

  / Registrar   / POST
Protobufs - /quota endpoint - Authentication
  \ Allocator \ \ GET
   \- DELETE

~Joseph

On Tue, Nov 10, 2015 at 10:25 AM, Vinod Kone  wrote:

> joerg, can you please make sure these reviews have linear dependencies? i'm
> seeing some reviews that block multiple reviews!? are you not using the
> post-reviews script?
>
> On Tue, Nov 10, 2015 at 10:20 AM, Joerg Schad  wrote:
>
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/39285/
> > ---
> >
> > (Updated Nov. 10, 2015, 6:20 p.m.)
> >
> >
> > Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris
> > Van Remoortere.
> >
> >
> > Bugs: MESOS-3199
> > https://issues.apache.org/jira/browse/MESOS-3199
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Added Quota Request Validation.
> >
> >
> > Diffs (updated)
> > -
> >
> >   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2
> >   src/master/quota.hpp PRE-CREATION
> >   src/master/quota.cpp PRE-CREATION
> >   src/master/quota_handler.cpp PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/39285/diff/
> >
> >
> > Testing
> > ---
> >
> > make test
> >
> >
> > Thanks,
> >
> > Joerg Schad
> >
> >
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 6:30 p.m.)


Review request for mesos and Jan Schlicht.


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


Repository: mesos


Description
---

Corrected include order for gtest and gmock according to Styleguide.


Diffs
-

  src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
  src/tests/common/recordio_tests.cpp db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
  src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d66f5198114c7bd67107ee056c2062d3118d1b4c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
4bb65afd0ee61cafef68e064a697fdce65d60058 
  src/tests/containerizer/external_containerizer_test.cpp 
4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
  src/tests/containerizer/isolator_tests.cpp 
a7072124c7e5cf38ed41779c2d9e023e089bf28f 
  src/tests/containerizer/port_mapping_tests.cpp 
73634f88b3fc46fce66b17259c5f90b8d1c86f86 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9227666868801335003aeb0fa21e6b8f0e94f2cb 
  src/tests/containerizer/sched_tests.cpp 
00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
  src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
  src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
  src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
  src/tests/fault_tolerance_tests.cpp 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
  src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
  src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
  src/tests/hierarchical_allocator_tests.cpp 
d422448606403fb39b077513ff73c6b9d41d2765 
  src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
  src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
  src/tests/master_allocator_tests.cpp 52fa472cbf764e0ed1dd334336f69774eb8c702b 
  src/tests/master_authorization_tests.cpp 
0d1f2b5d9ce1a998651798624238f635aeac4995 
  src/tests/master_contender_detector_tests.cpp 
84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
  src/tests/master_slave_reconciliation_tests.cpp 
7b0b983cae090ca5166d9fb1905fff88d3f21600 
  src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
  src/tests/master_validation_tests.cpp 
1dff6a353442cabe2eed9c136f30668d01305e25 
  src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
  src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
  src/tests/registrar_zookeeper_tests.cpp 
29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
  src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
  src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
  src/tests/scheduler_event_call_tests.cpp 
898cbbb58ac5391b8bd0cb94012410014cc3a606 
  src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
  src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
  src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
  src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
  src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
  src/tests/status_update_manager_tests.cpp 
82a985518276ac1015056a450634429840607524 
  src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
  src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
  src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
  src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 39285: Added Quota Request Validation.

[Vinod Kone]

joerg, can you please make sure these reviews have linear dependencies? i'm
seeing some reviews that block multiple reviews!? are you not using the
post-reviews script?

On Tue, Nov 10, 2015 at 10:20 AM, Joerg Schad  wrote:

>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39285/
> ---
>
> (Updated Nov. 10, 2015, 6:20 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris
> Van Remoortere.
>
>
> Bugs: MESOS-3199
> https://issues.apache.org/jira/browse/MESOS-3199
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added Quota Request Validation.
>
>
> Diffs (updated)
> -
>
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2
>   src/master/quota.hpp PRE-CREATION
>   src/master/quota.cpp PRE-CREATION
>   src/master/quota_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39285/diff/
>
>
> Testing
> ---
>
> make test
>
>
> Thanks,
>
> Joerg Schad
>
>

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 6:20 p.m.)


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


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


Repository: mesos


Description
---

Added Quota Request Validation.


Diffs (updated)
-

  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota.hpp PRE-CREATION 
  src/master/quota.cpp PRE-CREATION 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make test


Thanks,

Joerg Schad

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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



src/master/quota.cpp (line 36)


Should we add a blank line here?
Code base seems inconsistent about it...


- Joerg Schad


On Nov. 10, 2015, 6:10 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39285/
> ---
> 
> (Updated Nov. 10, 2015, 6:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3199
> https://issues.apache.org/jira/browse/MESOS-3199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Quota Request Validation.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39285/diff/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Joseph Wu]

> On Nov. 5, 2015, 12:24 p.m., Joseph Wu wrote:
> > src/master/quota_handler.cpp, lines 83-88
> > 
> >
> > Why don't you just parse a QuotaInfo object instead of a 
> > form-serialized body (with JSON components)?
> > 
> > The maintenance endpoints do this for simplicity.
> 
> Alexander Rukletsov wrote:
> Because we didn't want to expose the internal structure (`QuotaInfo`) to 
> the outside world. In the future we may want to allow operators to set quotas 
> for multiple roles in one call, which we can easier do if we do not tie the 
> operator API to `QuotaInfo` protobuf. Does it make sense?
> 
> Joseph Wu wrote:
> `QuotaInfo` should already be exposed, since it's in the `include` folder.
> 
> As for setting multiple `QuotaInfo`s in a single call, wouldn't that be 
> simpler to implement via a JSON array?
> (Maybe you can parse the request body as an array, but reject calls with 
> more than one quota request in the MVP.)
> 
> Alexander Rukletsov wrote:
> I should have been more precise and should have elaborated more in my 
> first reply. 
> 
> `QuotaInfo` is "mainly" the storage protobuf. It is indeed in the public 
> includes, but the reason for this is that it's part of the allocator 
> interface. Also we may want to let frameworks know what the quota of their 
> role is.
> 
> We do not aim to document the operator API for quota by exposing 
> `QuotaInfo` to them. We also do not intend to make the operator API for quota 
> 1:1 correspondent to `QuotaInfo` message. Let's think about post MVP: quota 
> may get optional chunks, limits; it may be set for a framework. I envision 
> the `QuotaInfo` message becomes a complex protobuf (think `Resource`). The 
> API for operators should stay simple and flexible, I am reluctant to force 
> operators provide a JSON view of `QuotaInfo` any time they want to set or 
> update quota. Recalling some of our discussions, we may even have different 
> endpoints for setting quota (`/master/quota`, `/master/roles/`)!
> 
> Does it make sense?

That sounds similar to the motivation for having V1 protos (i.e. having a 
public proto that gets converted into an internal-but-publically-exposed 
object).  And even if the API expects a non-`QuotaInfo` object, it would still 
be helpful to have a schema, of some sort.  Based on what's present in the 
review, it could just be a wrapper around a `Resource` proto.

Side note: Apparently, the `Resource` object is a bad example.  MPark found out 
yesterday that you can't take a `Resource` that Mesos outputs (via `/state`) 
and pass it to an endpoint that takes `Resource` as an input.  You have to 
reformat the `Resource`, even if both input/output are JSON formatted.


- Joseph


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


On Nov. 10, 2015, 8:19 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 10, 2015, 8:19 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c235209743de6e84deb44df31c61948f4dc8b8eb 
>   src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40142]

All tests passed.

- Mesos ReviewBot


On Nov. 10, 2015, 5:22 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager_tests.cpp 
> 82a985518276ac1015056a450634429840607524 
>   src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
>   src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
>   src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
>   src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 
> 
> Diff: https://reviews.apache.org/r/40142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 6:10 p.m.)


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


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


Repository: mesos


Description
---

Added Quota Request Validation.


Diffs (updated)
-

  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota.hpp PRE-CREATION 
  src/master/quota.cpp PRE-CREATION 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make test


Thanks,

Joerg Schad

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


Bad patch!

Reviews applied: [39317, 38218, 39399]

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

Error:
 2015-11-10 17:55:44 URL:https://reviews.apache.org/r/39399/diff/raw/ 
[9852/9852] -> "39399.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:334
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply
error: src/master/allocator/mesos/hierarchical.cpp: does not exist in index
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 5:51 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Nov. 10, 2015, 5:51 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/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
> 
> Diff: https://reviews.apache.org/r/39450/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. 10, 2015, 5:51 p.m.)


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


Changes
---

Corrected include order.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
d422448606403fb39b077513ff73c6b9d41d2765 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Nov. 10, 2015, 1:25 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Nov. 10, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40142]

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

Error:
 2015-11-10 17:33:47 URL:https://reviews.apache.org/r/40142/diff/raw/ 
[20939/20939] -> "40142.patch" [1]
error: src/tests/containerizer/provisioner_docker_tests.cpp: does not exist in 
index
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 5:22 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager_tests.cpp 
> 82a985518276ac1015056a450634429840607524 
>   src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
>   src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
>   src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
>   src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 
> 
> Diff: https://reviews.apache.org/r/40142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Joerg Schad]

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

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


Review request for mesos and Jan Schlicht.


Repository: mesos


Description
---

Corrected include order for gtest and gmock according to Styleguide.


Diffs (updated)
-

  src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
  src/tests/common/recordio_tests.cpp db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
  src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d66f5198114c7bd67107ee056c2062d3118d1b4c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
4bb65afd0ee61cafef68e064a697fdce65d60058 
  src/tests/containerizer/external_containerizer_test.cpp 
4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
  src/tests/containerizer/isolator_tests.cpp 
a7072124c7e5cf38ed41779c2d9e023e089bf28f 
  src/tests/containerizer/port_mapping_tests.cpp 
73634f88b3fc46fce66b17259c5f90b8d1c86f86 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9227666868801335003aeb0fa21e6b8f0e94f2cb 
  src/tests/containerizer/sched_tests.cpp 
00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
  src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
  src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
  src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
  src/tests/fault_tolerance_tests.cpp 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
  src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
  src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
  src/tests/hierarchical_allocator_tests.cpp 
d422448606403fb39b077513ff73c6b9d41d2765 
  src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
  src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
  src/tests/master_allocator_tests.cpp 52fa472cbf764e0ed1dd334336f69774eb8c702b 
  src/tests/master_authorization_tests.cpp 
0d1f2b5d9ce1a998651798624238f635aeac4995 
  src/tests/master_contender_detector_tests.cpp 
84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
  src/tests/master_slave_reconciliation_tests.cpp 
7b0b983cae090ca5166d9fb1905fff88d3f21600 
  src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
  src/tests/master_validation_tests.cpp 
1dff6a353442cabe2eed9c136f30668d01305e25 
  src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
  src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
  src/tests/registrar_zookeeper_tests.cpp 
29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
  src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
  src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
  src/tests/scheduler_event_call_tests.cpp 
898cbbb58ac5391b8bd0cb94012410014cc3a606 
  src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
  src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
  src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
  src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
  src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
  src/tests/status_update_manager_tests.cpp 
82a985518276ac1015056a450634429840607524 
  src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
  src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
  src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
  src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 5:14 p.m.)


Review request for mesos and Jan Schlicht.


Repository: mesos


Description
---

Corrected include order for gtest and gmock according to Styleguide.


Diffs
-

  src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
  src/tests/common/recordio_tests.cpp db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
  src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d66f5198114c7bd67107ee056c2062d3118d1b4c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
4bb65afd0ee61cafef68e064a697fdce65d60058 
  src/tests/containerizer/external_containerizer_test.cpp 
4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
  src/tests/containerizer/isolator_tests.cpp 
a7072124c7e5cf38ed41779c2d9e023e089bf28f 
  src/tests/containerizer/port_mapping_tests.cpp 
73634f88b3fc46fce66b17259c5f90b8d1c86f86 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9227666868801335003aeb0fa21e6b8f0e94f2cb 
  src/tests/containerizer/sched_tests.cpp 
00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
  src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
  src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
  src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
  src/tests/fault_tolerance_tests.cpp 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
  src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
  src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
  src/tests/hierarchical_allocator_tests.cpp 
d422448606403fb39b077513ff73c6b9d41d2765 
  src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
  src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
  src/tests/master_allocator_tests.cpp 52fa472cbf764e0ed1dd334336f69774eb8c702b 
  src/tests/master_authorization_tests.cpp 
0d1f2b5d9ce1a998651798624238f635aeac4995 
  src/tests/master_contender_detector_tests.cpp 
84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
  src/tests/master_slave_reconciliation_tests.cpp 
7b0b983cae090ca5166d9fb1905fff88d3f21600 
  src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
  src/tests/master_validation_tests.cpp 
1dff6a353442cabe2eed9c136f30668d01305e25 
  src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
  src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
  src/tests/registrar_zookeeper_tests.cpp 
29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
  src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
  src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
  src/tests/scheduler_event_call_tests.cpp 
898cbbb58ac5391b8bd0cb94012410014cc3a606 
  src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
  src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
  src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
  src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
  src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
  src/tests/status_update_manager_tests.cpp 
82a985518276ac1015056a450634429840607524 
  src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
  src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
  src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
  src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 

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


Testing (updated)
---

make check


Thanks,

Joerg Schad

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Jan Schlicht]

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

Ship it!


Ship It!

- Jan Schlicht


On Nov. 10, 2015, 6:14 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40142/
> ---
> 
> (Updated Nov. 10, 2015, 6:14 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected include order for gtest and gmock according to Styleguide.
> 
> 
> Diffs
> -
> 
>   src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
>   src/tests/common/recordio_tests.cpp 
> db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
>   src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d66f5198114c7bd67107ee056c2062d3118d1b4c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
>   src/tests/containerizer/isolator_tests.cpp 
> a7072124c7e5cf38ed41779c2d9e023e089bf28f 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 73634f88b3fc46fce66b17259c5f90b8d1c86f86 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9227666868801335003aeb0fa21e6b8f0e94f2cb 
>   src/tests/containerizer/sched_tests.cpp 
> 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
>   src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
>   src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
>   src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
>   src/tests/fault_tolerance_tests.cpp 
> 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
>   src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
>   src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
>   src/tests/hierarchical_allocator_tests.cpp 
> d422448606403fb39b077513ff73c6b9d41d2765 
>   src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
>   src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
>   src/tests/master_allocator_tests.cpp 
> 52fa472cbf764e0ed1dd334336f69774eb8c702b 
>   src/tests/master_authorization_tests.cpp 
> 0d1f2b5d9ce1a998651798624238f635aeac4995 
>   src/tests/master_contender_detector_tests.cpp 
> 84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 7b0b983cae090ca5166d9fb1905fff88d3f21600 
>   src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
>   src/tests/master_validation_tests.cpp 
> 1dff6a353442cabe2eed9c136f30668d01305e25 
>   src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
>   src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
>   src/tests/registrar_zookeeper_tests.cpp 
> 29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
>   src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
>   src/tests/scheduler_driver_tests.cpp 
> 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
>   src/tests/scheduler_event_call_tests.cpp 
> 898cbbb58ac5391b8bd0cb94012410014cc3a606 
>   src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
>   src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
>   src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
>   src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
>   src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
>   src/tests/status_update_manager_tests.cpp 
> 82a985518276ac1015056a450634429840607524 
>   src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
>   src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
>   src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
>   src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 
> 
> Diff: https://reviews.apache.org/r/40142/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40142: Corrected include order for gtest and gmock according to Styleguide.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 5:13 p.m.)


Review request for mesos and Jan Schlicht.


Repository: mesos


Description
---

Corrected include order for gtest and gmock according to Styleguide.


Diffs (updated)
-

  src/tests/common/http_tests.cpp 8f63a2125b94646eac2a65d687508cdcbf239c1c 
  src/tests/common/recordio_tests.cpp db5e5c9d7241ba2d5d9486a9299177dfb0c505c7 
  src/tests/containerizer.hpp dfa2c5bf733e944f2c22a36d0c071470dc90083d 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d66f5198114c7bd67107ee056c2062d3118d1b4c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
4bb65afd0ee61cafef68e064a697fdce65d60058 
  src/tests/containerizer/external_containerizer_test.cpp 
4f152a4b01a8ea3d6c07aaef13319c67e0d6f99f 
  src/tests/containerizer/isolator_tests.cpp 
a7072124c7e5cf38ed41779c2d9e023e089bf28f 
  src/tests/containerizer/port_mapping_tests.cpp 
73634f88b3fc46fce66b17259c5f90b8d1c86f86 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9227666868801335003aeb0fa21e6b8f0e94f2cb 
  src/tests/containerizer/sched_tests.cpp 
00723d01cd2cc37410d6f9fdd2de080063b7ccd8 
  src/tests/credentials_tests.cpp 9d9de814bb1617d64f57dbf9425fe1e8135d2f46 
  src/tests/environment.hpp 9cf14bcc8f7d386f6aa26b686d3f953c969aaf63 
  src/tests/environment.cpp fd6ac2f88ef8f694c2a19018b6b4e4e34f08bd72 
  src/tests/fault_tolerance_tests.cpp 3e353e42e1051ff3d9e3be3707ff6bc7b5f348c8 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
  src/tests/gc_tests.cpp 35701706bb16ff33d38d0dc6594bde0e011590f5 
  src/tests/group_tests.cpp 13e89c33617997aac2024a3de7beedb2fb28e03c 
  src/tests/hierarchical_allocator_tests.cpp 
d422448606403fb39b077513ff73c6b9d41d2765 
  src/tests/log_tests.cpp 4ff7a6a425dfa672f680b7072cbafad05b6249bf 
  src/tests/main.cpp 970a8b06e1c04cb6545353c8d36f5707c153d49b 
  src/tests/master_allocator_tests.cpp 52fa472cbf764e0ed1dd334336f69774eb8c702b 
  src/tests/master_authorization_tests.cpp 
0d1f2b5d9ce1a998651798624238f635aeac4995 
  src/tests/master_contender_detector_tests.cpp 
84652b1fb57d36b3a7f05beda0a2af179c3ca3e8 
  src/tests/master_slave_reconciliation_tests.cpp 
7b0b983cae090ca5166d9fb1905fff88d3f21600 
  src/tests/master_tests.cpp a754e3b3e78c639a473f79c1b24c505cc47ba1a9 
  src/tests/master_validation_tests.cpp 
1dff6a353442cabe2eed9c136f30668d01305e25 
  src/tests/partition_tests.cpp 6a6524e98d552b8107a765563e1c3f4140805bc3 
  src/tests/reconciliation_tests.cpp d9dde93e14b143a01cf9af055df081f4d034e080 
  src/tests/registrar_zookeeper_tests.cpp 
29832b43cb8c6d201ebdba6f268d7dfaddd9caa2 
  src/tests/reservation_tests.cpp 4bc2b313572a8cd76fa798ac745c319a7c11df0e 
  src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
  src/tests/scheduler_event_call_tests.cpp 
898cbbb58ac5391b8bd0cb94012410014cc3a606 
  src/tests/scheduler_tests.cpp 0afeca377dd422865cc9615c85a14b65512dfa6a 
  src/tests/slave_recovery_tests.cpp 648babec6307127ff28692de7c4868af3ac123a3 
  src/tests/slave_tests.cpp ddd01104d044b54664b3b1eefdde3f73b8f6d598 
  src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 
  src/tests/state_tests.cpp 7cee17ddeeb0bf6a9f08d990031e2fbb14a10126 
  src/tests/status_update_manager_tests.cpp 
82a985518276ac1015056a450634429840607524 
  src/tests/teardown_tests.cpp 89eb3144b458c8d3fe2b9bca03f93ac352f99f4b 
  src/tests/zookeeper.hpp ed341125595f3cb766af3d2c05e57b306a9e464e 
  src/tests/zookeeper.cpp 6ce008fbd1dda1fce10cd45e2d4d6051e1fc5a41 
  src/tests/zookeeper_tests.cpp 7d1c0665f06e067637861424aa647d6e4d3f9c85 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 4:19 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

[fixup] More fixes for trivial clang-format misformats (NFC).


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


Repository: mesos


Description
---

Also added function to query mtime of HDFS resource.


Diffs (updated)
-

  src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
  src/slave/containerizer/fetcher.hpp c7518a36f6344841880dbb11bfce603fd2791fc0 
  src/slave/containerizer/fetcher.cpp 0b676f43e8529c6790824c8cf82e21a81d885ef1 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 4:19 p.m.)


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


Changes
---

Joris' comments.


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


Repository: mesos


Description
---

Processing quota request consists of several stages: request validation, sanity 
check and so on. This patch creates a basic workflow for quota requests, while 
the stages are implemented in subsequent patches.


Diffs (updated)
-

  src/CMakeLists.txt c235209743de6e84deb44df31c61948f4dc8b8eb 
  src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota.hpp PRE-CREATION 
  src/master/quota.cpp PRE-CREATION 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

> On Nov. 10, 2015, 7:26 a.m., Joris Van Remoortere wrote:
> > src/master/quota_handler.cpp, line 48
> > 
> >
> > Let's sync this to a `Try` to avoid confusion.

I think I'd better remove the whole function alltogether.


- Alexander


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 4:14 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

More fixes for trivial clang-format misformats (NFC).


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


Repository: mesos


Description
---

Also added function to query mtime of HDFS resource.


Diffs (updated)
-

  src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
  src/slave/containerizer/fetcher.hpp c7518a36f6344841880dbb11bfce603fd2791fc0 
  src/slave/containerizer/fetcher.cpp 0b676f43e8529c6790824c8cf82e21a81d885ef1 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 

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


Testing
---

make check


Thanks,

Benjamin Bannier

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

[Joerg Schad]

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



src/tests/hierarchical_allocator_tests.cpp (line 1150)


Also the simple case of correct behavior with several frameworks in one 
role would be interesting.



src/tests/hierarchical_allocator_tests.cpp (line 1173)


Does it make sense to refelect that in the role name?



src/tests/hierarchical_allocator_tests.cpp (line 1618)


This describes more the user story, should we maybe focus on what the test 
itself intends to test?


- Joerg Schad


On Nov. 5, 2015, 6:26 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Nov. 5, 2015, 6:26 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/tests/hierarchical_allocator_tests.cpp 
> 505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 
> 
> Diff: https://reviews.apache.org/r/39450/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

[Greg Mann]

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



docs/authorization.md (line 12)


s/maintenance machine(s)/maintain machine(s)/


- Greg Mann


On Sept. 23, 2015, 5:16 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38399/
> ---
> 
> (Updated Sept. 23, 2015, 5:16 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: mesos-
> https://issues.apache.org/jira/browse/mesos-
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ACLs for the maintenance HTTP endpoints
> 
> The design doc: 
> https://docs.google.com/document/d/1sG28ASH-5MDygR2igackOb5x-Lt5nXyIUMcwrrbk4cE/edit?usp=sharing
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38399/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 3:43 p.m.)


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


Changes
---

Removed logging for bad http responses (should be logged by libprocess).


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


Repository: mesos


Description
---

Added Quota Request Validation.


Diffs (updated)
-

  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota.hpp PRE-CREATION 
  src/master/quota.cpp PRE-CREATION 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make test


Thanks,

Joerg Schad

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Jan Schlicht]

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

Ship it!



src/hdfs/hdfs.hpp (lines 205 - 210)


Indent with an extra space, like
```
/**
 * Foo
 * 
 * Bar to describe foo.
 */
```



src/hdfs/hdfs.hpp (line 206)


Each line of a doxygen comment should start with '*'.



src/slave/containerizer/fetcher.cpp (line 248)


Indent with 4 spaces.



src/slave/containerizer/fetcher.cpp (line 325)


Indent with 4 spaces.



src/slave/containerizer/fetcher.cpp (line 935)


Indent with 4 spaces.


- Jan Schlicht


On Nov. 10, 2015, 2:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39595/
> ---
> 
> (Updated Nov. 10, 2015, 2:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added function to query mtime of HDFS resource.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
>   src/slave/containerizer/fetcher.hpp 
> c7518a36f6344841880dbb11bfce603fd2791fc0 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
> 
> Diff: https://reviews.apache.org/r/39595/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 40107: Removed unused checks in command executor.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Nov. 10, 2015, 5:21 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40107/
> ---
> 
> (Updated Nov. 10, 2015, 5:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused checks in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0dad75d7d730ef6cdc4b427a358625433cfee510 
> 
> Diff: https://reviews.apache.org/r/40107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 40107: Removed unused checks in command executor.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Nov. 10, 2015, 1:21 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40107/
> ---
> 
> (Updated Nov. 10, 2015, 1:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused checks in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0dad75d7d730ef6cdc4b427a358625433cfee510 
> 
> Diff: https://reviews.apache.org/r/40107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 40107: Removed unused checks in command executor.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Nov. 10, 2015, 1:21 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40107/
> ---
> 
> (Updated Nov. 10, 2015, 1:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused checks in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0dad75d7d730ef6cdc4b427a358625433cfee510 
> 
> Diff: https://reviews.apache.org/r/40107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

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

[Joerg Schad]

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



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


laidAside are rejected resources? In that case I would prefer 
rejectedResources.



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


s/comes first/is satisfied first



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


We can sum up resources as quota is only applicable to scalar resources.



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


Wouldn't it make sense to have a flag indicating when quota ia satisfied 
for the first time in this loop (i.e. foreach slave)? Otherwise we have to 
recheck this for every agent in the cluster


- Joerg Schad


On Nov. 9, 2015, 10:01 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 9, 2015, 10:01 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 39595: Took mtime into account in the fetcher cache.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39594, 39595]

All tests passed.

- Mesos ReviewBot


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39595/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added function to query mtime of HDFS resource.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
>   src/slave/containerizer/fetcher.hpp 
> c7518a36f6344841880dbb11bfce603fd2791fc0 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
> 
> Diff: https://reviews.apache.org/r/39595/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Anand Mazumdar]

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

(Updated Nov. 10, 2015, 2:27 p.m.)


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


Changes
---

Updated based on BenM's review comments on r38876


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


Repository: mesos


Description
---

This change adds the functionality for executors to `Subscribe` via the 
`api/v1/executor` endpoint. It also stores a marker file as part of the 
`Subscribe` call if framework `checkpointing` is enabled. This can then be used 
by the agent when recovering to wait for reconnecting back with the executor.

Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
`CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
the `Subscribe` call.


Diffs (updated)
-

  src/slave/http.cpp ce48a0584ab18a8d95dd02619f62df18b2276639 
  src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
  src/slave/slave.cpp 535adc3b17d5af3fe811a8e2505f126a28212dbf 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 39297: Added support for recovering RunState for HTTP based executors

[Anand Mazumdar]

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

(Updated Nov. 10, 2015, 2:24 p.m.)


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


Changes
---

Updated based on BenM's review comments on r38876


Repository: mesos


Description
---

This change adds support for recovering the `RunState` for `HTTP` based 
executors. Upon agent recovery, it checks if the marker file for HTTP exists 
and populates `RunState.http` based on that. This is later used by the Agent 
for recovering `HTTP` based executors.


Diffs (updated)
-

  src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
  src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38876: Added functionality to store a marker file to denote HTTP based executors

[Anand Mazumdar]

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

(Updated Nov. 10, 2015, 2:22 p.m.)


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


Changes
---

Address comments from BenM + Updated Description


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


Repository: mesos


Description (updated)
---

This change adds the relevant functionality to `src/slave/paths.cpp/hpp` to 
store a marker file to denote HTTP based executors. We create the file when 
`checkpointing` is enabled as part of handling the `Subscribe` request. This is 
then used by the agent when recovering to ascertain if the executor was 
connected via `HTTP` before the agent restart.

-- Detailed Explanation of Changes ( not to be included in the commit message )
This marker file is used when recovering HTTP based executors (assuming 
framework checkpointing is enabled). Currently we support the following 
recovery options on the agent.

1. `--cleanup` : If `PID` marker file is not found, the current behavior is to 
directly destroy the container the executor was running in. With the help of 
this `HTTP` marker file, we can now check if the executor was connected via 
HTTP previously and if so, send it a `Event::SHUTDOWN` when it retries the 
`Subscribe` call.
2. `--reconnect` : If `PID` marker file is not found, the current behavior is 
to just `LOG` that we were not able to reconnect back to the executor. With the 
help of the `HTTP` marker file, we are able to correctly distinguish between 
the cases when a `PID` based executor failed to checkpoint its PID and it being 
an `HTTP` based executor. An example: 
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L4177


Diffs (updated)
-

  src/slave/paths.hpp f743fb4b1ca278fade9134e0ae8f6a6450d4a977 
  src/slave/paths.cpp aab7a4b63f0e7c2104097077369bb10bcd28c6a1 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

> On Nov. 5, 2015, 8:24 p.m., Joseph Wu wrote:
> > src/master/quota_handler.cpp, lines 83-88
> > 
> >
> > Why don't you just parse a QuotaInfo object instead of a 
> > form-serialized body (with JSON components)?
> > 
> > The maintenance endpoints do this for simplicity.
> 
> Alexander Rukletsov wrote:
> Because we didn't want to expose the internal structure (`QuotaInfo`) to 
> the outside world. In the future we may want to allow operators to set quotas 
> for multiple roles in one call, which we can easier do if we do not tie the 
> operator API to `QuotaInfo` protobuf. Does it make sense?
> 
> Joseph Wu wrote:
> `QuotaInfo` should already be exposed, since it's in the `include` folder.
> 
> As for setting multiple `QuotaInfo`s in a single call, wouldn't that be 
> simpler to implement via a JSON array?
> (Maybe you can parse the request body as an array, but reject calls with 
> more than one quota request in the MVP.)

I should have been more precise and should have elaborated more in my first 
reply. 

`QuotaInfo` is "mainly" the storage protobuf. It is indeed in the public 
includes, but the reason for this is that it's part of the allocator interface. 
Also we may want to let frameworks know what the quota of their role is.

We do not aim to document the operator API for quota by exposing `QuotaInfo` to 
them. We also do not intend to make the operator API for quota 1:1 
correspondent to `QuotaInfo` message. Let's think about post MVP: quota may get 
optional chunks, limits; it may be set for a framework. I envision the 
`QuotaInfo` message becomes a complex protobuf (think `Resource`). The API for 
operators should stay simple and flexible, I am reluctant to force operators 
provide a JSON view of `QuotaInfo` any time they want to set or update quota. 
Recalling some of our discussions, we may even have different endpoints for 
setting quota (`/master/quota`, `/master/roles/`)!

Does it make sense?


- Alexander


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

> On Oct. 6, 2015, 12:40 p.m., Guangya Liu wrote:
> > src/master/quota_handler.cpp, line 60
> > 
> >
> > I think that this function is mainly doing getQuoteRequest()?
> 
> Alexander Rukletsov wrote:
> Converting JSON to protobuf is not a big deal, I wanted to put the accent 
> on the validation checks.
> 
> Guangya Liu wrote:
> Can we split this to two functions, one is validataQuotaRequest and the 
> other is getQuotaRequest?
> 
> Joerg Schad wrote:
> Constructing the protobuf is part of the basic checks and from my point 
> it feels ok that validates return the protobuf.

We revisited the design and I think we addressed this issue in the next review. 
I'll drop it here, please check the next patch in the chain and comment there 
in case you feel like.


- Alexander


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 2:07 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Improved "jaggedness".


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


Repository: mesos


Description
---

[stout]: Added function to simultaneously query size and mtime of URI.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp 
311b00b41398a9fd7374f3847190468ba59c1dc9 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
e49783a438157706b1be9745436bf666f45cab8b 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

> On Oct. 13, 2015, 3:35 p.m., Joris Van Remoortere wrote:
> > src/master/quota_handler.cpp, line 60
> > 
> >
> > Should we follow the pattern of other validation routines where we use 
> > the validation namespace and pull these utility routines into a separate 
> > file?
> > 
> > I think we can separate the validation of the request from the content 
> > of the protobuf. Syntax vs. semantic.
> > 
> > See maintenance as an example.
> 
> Alexander Rukletsov wrote:
> The reason it's conflated into one function is because in order to do 
> proper validation we should convert JSON to protobuf. Some checks are 
> performed with the JSON object and some with the converted protobuf. I though 
> that having three functions `validateJSON`, `convertToQuotaInfo`, and 
> `validateQuotaInfo` is too much.
> 
> Joseph Wu wrote:
> What specific validation are you doing to the JSON objects (other than 
> converting to Protobuf)?  And why can't the validation wait until after the 
> JSON->Protobuf conversion sufficient?
> 
> Alexander Rukletsov wrote:
> Let's discuss this in the next review 
> https://reviews.apache.org/r/39285/, where validation is actually implemented.

We will address it in the next review.


- Alexander


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39223: Added Quota Request Validation Tests.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 2:05 p.m.)


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


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


Repository: mesos


Description
---

see Summary.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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

(Updated Nov. 10, 2015, 2:03 p.m.)


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


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


Repository: mesos


Description
---

Added Quota Request Validation.


Diffs (updated)
-

  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/master/quota.hpp PRE-CREATION 
  src/master/quota.cpp PRE-CREATION 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make test


Thanks,

Joerg Schad

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Alexander Rukletsov]

> On Oct. 7, 2015, 2:41 p.m., Bernd Mathiske wrote:
> > src/master/quota_handler.cpp, line 66
> > 
> >
> > Given that we have the request object, we can output more diagnostic 
> > info here.
> 
> Guangya Liu wrote:
> I think that we can use BadRequest("Expecting POST"); instead, please 
> refer to https://github.com/apache/mesos/blob/master/src/master/http.cpp#L686
> 
> Alexander Rukletsov wrote:
> We are using `BadRequest` in the caller. The intention is to send HTTP 
> responses only from the "entry" function, which processes the request 
> (`Master::QuotaHandler::request()`, will be renamed to 
> `Master::QuotaHandler::set()`). All callees return errors. Does it make sense?
> 
> Bernd Mathiske wrote:
> Yes, but the argument of Error() here could include what the request was, 
> so a human can inspect it in the log. Unless it is clear from the context 
> that the request has already been logged. But I prefer context-free extra 
> diagnostics :-)

We do provide more info, please look in https://reviews.apache.org/r/36913/. 
Also, this line is changed to a CHECK in the next review in the chain.


- Alexander


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Jan Schlicht]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 132)


Please add a newline after `=` as you did in l125.


- Jan Schlicht


On Nov. 10, 2015, 2:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 10, 2015, 2:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [stout]: Added function to simultaneously query size and mtime of URI.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp 
> 311b00b41398a9fd7374f3847190468ba59c1dc9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> e49783a438157706b1be9745436bf666f45cab8b 
> 
> Diff: https://reviews.apache.org/r/39594/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.

[Joris Van Remoortere]

> On Oct. 6, 2015, 12:40 p.m., Guangya Liu wrote:
> > src/master/quota_handler.cpp, line 65
> > 
> >
> > Just as early comments: What about PUT and DELETE?
> 
> Alexander Rukletsov wrote:
> See my comment in the subsequent review: 
> https://reviews.apache.org/r/39285/diff/4?file=1103067#file1103067line60
> I think we should move this line out of this function.
> 
> Guangya Liu wrote:
> PUT is not MVP, DELETE should be still in MVP, right?
> 
> Alexander Rukletsov wrote:
> Yes.

We'll address Delete once we implement remove quota.


- Joris


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


On Nov. 9, 2015, 11:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> ---
> 
> (Updated Nov. 9, 2015, 11:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Processing quota request consists of several stages: request validation, 
> sanity check and so on. This patch creates a basic workflow for quota 
> requests, while the stages are implemented in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f6ae05d4652df6de98a9e110efed87f7fcbd29f9 
>   src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39594]

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

Error:
 2015-11-10 13:53:38 URL:https://reviews.apache.org/r/39594/diff/raw/ 
[3630/3630] -> "39594.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp:85
error: 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp: patch does not 
apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39595/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added function to query mtime of HDFS resource.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
>   src/slave/containerizer/fetcher.hpp 
> c7518a36f6344841880dbb11bfce603fd2791fc0 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
> 
> Diff: https://reviews.apache.org/r/39595/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Benjamin Bannier]

> On Nov. 10, 2015, 12:52 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/fetcher.cpp, line 940
> > 
> >
> > 4 spaces instead of 2 spaces.

It appears that continuations for assignments are indented by 2 spaces, not by 
4. Please re-check your version of the C++ style guide.


> On Nov. 10, 2015, 12:52 p.m., Jan Schlicht wrote:
> > src/slave/containerizer/fetcher.cpp, line 262
> > 
> >
> > Newline after this line please.

Fair enough, added an extra newline.


- Benjamin


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


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39595/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added function to query mtime of HDFS resource.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
>   src/slave/containerizer/fetcher.hpp 
> c7518a36f6344841880dbb11bfce603fd2791fc0 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
> 
> Diff: https://reviews.apache.org/r/39595/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 130
> > 
> >
> > Add blank line here please.

I would be happy to oblige, but that wouldn't commit due cpplint,

Redundant blank line at the start of a code block should be deleted.  
[whitespace/blank_line] [2]


> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 141
> > 
> >
> > Why not use `nullptr` instead?
> 
> Jan Schlicht wrote:
> Is `nullptr` okay to use? Judging from MESOS-3243 I'd say that it's not 
> there yet. Also there's no mention of it in the C++ style guide.
> 
> Benjamin Bannier wrote:
> I would have commented that since this file completely relies on `NULL` 
> instead of `nullptr` already consistence would suggest to stick to `NULL`.
> 
> Till Toenshoff wrote:
> I would interpret MESOS-3243 as a transition ticket for old, existing 
> code and not for new code.

No problem, I `nullptr`ized that instance, but left the others for the 
automatic cleanup.


- Benjamin


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


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [stout]: Added function to simultaneously query size and mtime of URI.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp 
> 311b00b41398a9fd7374f3847190468ba59c1dc9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> e49783a438157706b1be9745436bf666f45cab8b 
> 
> Diff: https://reviews.apache.org/r/39594/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Benjamin Bannier]

> On Nov. 9, 2015, 1:01 p.m., Till Toenshoff wrote:
> > src/hdfs/hdfs.hpp, lines 204-208
> > 
> >
> > The examples of our doxygen-styleguide suggest that we should be using 
> > C style comments. Same goes with the parameter description which should be 
> > starting with a capital letter.
> > 
> > ```
> >   /**
> >* Obtain path modification time.
> >*
> >* @param path Path to the resource to query.
> >* @return Returns the last modification time of the resource in 
> > seconds
> >*   since the POSIX epoch, or an Error if it cannot be retrieved.
> >*/
> > ```

Of course!


> On Nov. 9, 2015, 1:01 p.m., Till Toenshoff wrote:
> > src/slave/containerizer/fetcher.hpp, line 256
> > 
> >
> > Let's only use the key here...
> > 
> > ```
> > bool hasKey(const std::string& key);
> > ```

Done, also a `const` member fct now.


- Benjamin


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


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39595/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
> https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added function to query mtime of HDFS resource.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
>   src/slave/containerizer/fetcher.hpp 
> c7518a36f6344841880dbb11bfce603fd2791fc0 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
>   src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 
> 
> Diff: https://reviews.apache.org/r/39595/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39595: Took mtime into account in the fetcher cache.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 1:45 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Incorporated review comments.

* fixed doxygen comment style,
* made 2 member functions `const`, and
* explicitly lookup key instead of wrapping this too much, and
* style fixes.


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


Repository: mesos


Description
---

Also added function to query mtime of HDFS resource.


Diffs (updated)
-

  src/hdfs/hdfs.hpp 42c150186f2ce676407e4e00e84bd7e38063d9ba 
  src/slave/containerizer/fetcher.hpp c7518a36f6344841880dbb11bfce603fd2791fc0 
  src/slave/containerizer/fetcher.cpp 0b676f43e8529c6790824c8cf82e21a81d885ef1 
  src/tests/fetcher_cache_tests.cpp 0b3245105b4c1efae54f0bc34f672290819a6f0b 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 1:45 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Incorporated review comments.

* remove unused function `contentLength`,
* fix doxygen comment style,
* use `nullptr` instead of `NULL` in new code, and
* use symbol `CURLE_OK` instead of literal `0`.


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


Repository: mesos


Description
---

[stout]: Added function to simultaneously query size and mtime of URI.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp 
311b00b41398a9fd7374f3847190468ba59c1dc9 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
e49783a438157706b1be9745436bf666f45cab8b 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Alexander Rukletsov]

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

Ship it!



src/master/http.cpp (line 910)


s/Update/update



src/master/quota_handler.cpp (line 20)


Remove extra newline.


- Alexander Rukletsov


On Nov. 10, 2015, 8:49 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated Nov. 10, 2015, 8:49 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
> and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c235209743de6e84deb44df31c61948f4dc8b8eb 
>   src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
>   src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/master.cpp 7bac0fea4bcd040307fdfdcd002387d5baee46d1 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Alexander Rukletsov]

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

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


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


Changes
---

Formatting.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40107: Removed unused checks in command executor.

[Timothy Chen]

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

(Updated Nov. 10, 2015, 5:21 a.m.)


Review request for mesos, Anand Mazumdar and Till Toenshoff.


Changes
---

Added bug number.


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


Repository: mesos


Description
---

Removed unused checks in command executor.


Diffs
-

  src/launcher/executor.cpp 0dad75d7d730ef6cdc4b427a358625433cfee510 

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


Testing
---

make check


Thanks,

Timothy Chen

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

[Joris Van Remoortere]

> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1055-1057
> > 
> >
> > Since we know the guarentee of each quota'ed role and the gap between 
> > it and role's current allocation, I think it might be better to do the 
> > "find-grained" allocation, i.e., only allocate resources to fill the gap. 
> > Otherwise, we may run into the situation that we allocate too much resource 
> > to satisfy a role's guarentee but another role's guarentee can not be 
> > satisfied.
> 
> Alexander Rukletsov wrote:
> Yep, this can be the case and it was a hot debate during the design 
> phase. I have decided to choose this approach because it's follows the least 
> surprise principle for people who already familiar with DRF. I think we'll 
> have to revisit the strategy anyway and (most probably) introduce 
> `Quota.limit`. I think this is fine for MVP.

We also discussed this during the Community Sync on Nov 5th. Coarse-grained is 
sufficient for the MVP, and we can look at the fine-grained implications as 
parth of the next design doc.


- Joris


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


On Nov. 9, 2015, 10:01 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 9, 2015, 10:01 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 39289: Quota: Added authorization of quota requests.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39211, 39018, 39102, 36913, 38059]

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

Error:
 2015-11-10 13:11:05 URL:https://reviews.apache.org/r/38059/diff/raw/ 
[7991/7991] -> "38059.patch" [1]
error: patch failed: src/master/master.hpp:860
error: src/master/master.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 10, 2015, 12:56 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39289/
> ---
> 
> (Updated Nov. 10, 2015, 12:56 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization of quota requests.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
>   include/mesos/authorizer/authorizer.hpp 
> d667a52f90f970a313580446a5a006cec4b5e25b 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
>   include/mesos/quota/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 
>   src/authorizer/local/authorizer.hpp 
> 32de102fd588f029882ef121ca83a7410c65 
>   src/authorizer/local/authorizer.cpp 
> 6d7da87731a438c2180cf91003e09d4aa5a1c773 
>   src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
>   src/master/master.hpp 3aa7017287506938d77bbfb1e06f510101009ee3 
>   src/master/quota_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
> 
> Diff: https://reviews.apache.org/r/39289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39223: Added Quota Request Validation Tests.

[Alexander Rukletsov]

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



src/tests/master_quota_tests.cpp (line 449)


Do you think it's safer to use "cpus" as revocable resoource?


- Alexander Rukletsov


On Nov. 9, 2015, 10:45 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39223/
> ---
> 
> (Updated Nov. 9, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3199
> https://issues.apache.org/jira/browse/MESOS-3199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see Summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39223/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40107: Removed unused checks in command executor.

[Till Toenshoff]

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


This is supposed to be the fix (or workaround) for MESOS-3851, right? Let's 
please tag it in the "bugs" section. However, just as Vinod commented in that 
ticket, it seems the implications of the problem might really be bigger and not 
only localized to tbe command executor.

- Till Toenshoff


On Nov. 9, 2015, 9:37 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40107/
> ---
> 
> (Updated Nov. 9, 2015, 9:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused checks in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0dad75d7d730ef6cdc4b427a358625433cfee510 
> 
> Diff: https://reviews.apache.org/r/40107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 12:52 p.m.)


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


Changes
---

Updated comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 6ec0488027d6cfccc63ac3a6a8b0c3d8eb6c3330 
  src/tests/master_quota_tests.cpp PRE-CREATION 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39289: Quota: Added authorization of quota requests.

[Jan Schlicht]

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

(Updated Nov. 10, 2015, 1:56 p.m.)


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


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


Repository: mesos


Description
---

Added authorization of quota requests.


Diffs
-

  docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  include/mesos/quota/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 
  src/authorizer/local/authorizer.hpp 32de102fd588f029882ef121ca83a7410c65 
  src/authorizer/local/authorizer.cpp 6d7da87731a438c2180cf91003e09d4aa5a1c773 
  src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
  src/master/master.hpp 3aa7017287506938d77bbfb1e06f510101009ee3 
  src/master/quota_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 

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


Testing
---

make check


Thanks,

Jan Schlicht