Re: Review Request 41705: WIP: Added support for enforcing quota on (persistent) volumes (MESOS-4198).

[Qian Zhang]

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



src/slave/containerizer/mesos/isolators/posix/disk.cpp (line 171)


Should we simply use `!Resources::isPersistentVolume()` here?


- Qian Zhang


On Dec. 26, 2015, 10:15 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41705/
> ---
> 
> (Updated Dec. 26, 2015, 10:15 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4198
> https://issues.apache.org/jira/browse/MESOS-4198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for enforcing quota on (persistent) volumes (MESOS-4198).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> d971db09083faad08f3cf18c25a79245321d1d9a 
>   src/tests/disk_quota_tests.cpp ce736c32a9e78a8a6d0793a06fe87911b9b0558d 
> 
> Diff: https://reviews.apache.org/r/41705/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER='DiskQuotaTest*' make check -j8
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597, 41681]

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

- Mesos ReviewBot


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

Re: Review Request 41715: Support parsing url in libprocess.

[Gilbert Song]

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

Ship it!


Verified that the parse method handles some conner cases well. :)


3rdparty/libprocess/src/tests/http_tests.cpp (lines 1204 - 1207)


Should we use `EXPECT_` here?

It does not impact on this test result, but we may not expect each of them 
to yield a fatal failure. (Understand that the test passed though, but it can 
become an example to someone else, and we do want tests continue to run).



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1211 - 1214)


Ditto.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1218 - 1221)


Ditto.


- Gilbert Song


On Dec. 25, 2015, 12:10 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 25, 2015, 12:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41586: Partially enforced commit message guidelines with a hook.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41584, 41586]

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

- Mesos ReviewBot


On Dec. 26, 2015, 2:05 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41586/
> ---
> 
> (Updated Dec. 26, 2015, 2:05 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Partially enforced commit message guidelines with a hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 89d986fd95dc16bbb79623ef92e3b14a2e7009f9 
>   support/hooks/commit-msg PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41586/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

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

[Yongqiao Wang]

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

(Updated Dec. 26, 2015, 2:22 a.m.)


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


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  src/CMakeLists.txt 8169fe4f1a0eda5c2e2b30e22a7346478420a9a0 
  src/Makefile.am 8af0115caa67ac8f3193d8f0d0f1a4ae739af275 
  src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110 
  src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9 
  src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
  src/master/weights_handler.cpp PRE-CREATION 

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


Testing
---

Make & Make check successfully!

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


Thanks,

Yongqiao Wang

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

[Yongqiao Wang]

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

(Updated Dec. 26, 2015, 2:20 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
  src/master/allocator/mesos/allocator.hpp 
50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
  src/master/allocator/mesos/hierarchical.hpp 
86ea5a402ed67f8f22f11d5730147cd907d66a08 
  src/master/allocator/mesos/hierarchical.cpp 
775182515dcb52bd873ecdf98c827320251a59c8 
  src/master/allocator/sorter/drf/sorter.hpp 
050896e8b12cd4097ccd137d5284d6b39b0f06ab 
  src/master/allocator/sorter/drf/sorter.cpp 
3a442f121f3a1505513877a5c78458a4b8d0a824 
  src/master/allocator/sorter/sorter.hpp 
7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
  src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 

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


Testing
---

Make & Make check successfully!

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


Thanks,

Yongqiao Wang

Re: Review Request 41705: WIP: Added support for enforcing quota on (persistent) volumes (MESOS-4198).

[Artem Harutyunyan]

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

(Updated Dec. 25, 2015, 6:15 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description (updated)
---

Added support for enforcing quota on (persistent) volumes (MESOS-4198).


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
d971db09083faad08f3cf18c25a79245321d1d9a 
  src/tests/disk_quota_tests.cpp ce736c32a9e78a8a6d0793a06fe87911b9b0558d 

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


Testing
---

GTEST_FILTER='DiskQuotaTest*' make check -j8


Thanks,

Artem Harutyunyan

Re: Review Request 41704: WIP: Added support for checking whether a given path is absolute.

[Artem Harutyunyan]

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

(Updated Dec. 25, 2015, 6:13 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description (updated)
---

Added support for checking whether a given path is absolute.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
0b986f0898da95c4cffd8bde1adfd9994d567096 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
821dbb185f09e2f279d95fd354ce2168cddf1bac 

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


Testing
---

Ran stout tests.


Thanks,

Artem Harutyunyan

Re: Review Request 41584: Added commit message guidelines to docs.

[Artem Harutyunyan]

> On Dec. 19, 2015, 6:34 a.m., Till Toenshoff wrote:
> > docs/submitting-a-patch.md, line 52
> > 
> >
> > We actually do it a bit more differentiated.
> > 
> > The `Summary` should be past tense, start with a capital letter and end 
> > in a period.
> > 
> > The `Description` should be present or continouus. It describes the 
> > changes and may also contain a short reasoning for them.
> > 
> > The `Testing done` should state all steps you did for validating your 
> > RR.
> 
> Adam B wrote:
> I would further venture to say that the first sentence should ideally be 
> 50chars or less, but we will allow up to 72 if needed to get the point across.
> The rest of the Description should be wrapped at 72chars, but I usually 
> do that manually when committing.
> I've never really payed attention to the tense in the Description, mostly 
> just making sure the summary is past tense.
> I'd also like to point out that the Testing Done section is not included 
> in the commit message, so contributors are welcome to put additional notes to 
> reviewers there (or in a self-review).

Created a JIRA (https://issues.apache.org/jira/browse/MESOS-4250) to follow up 
with enforcing the description line lengths at 72 chars.


- Artem


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


On Dec. 19, 2015, 12:18 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41584/
> ---
> 
> (Updated Dec. 19, 2015, 12:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added commit message guidelines to docs.
> 
> 
> Diffs
> -
> 
>   docs/submitting-a-patch.md f0048f51395422645a520b61815c1ae3c7004aa3 
> 
> Diff: https://reviews.apache.org/r/41584/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41586: Partially enforced commit message guidelines with a hook.

[Artem Harutyunyan]

> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > bootstrap, lines 22-23
> > 
> >
> > Our general policy in C++ variable names is to avoid abbreviations, and 
> > I imagine that could apply to shell scripts too.
> > Please s/msg/message/

That's a standard git hook file name. 
https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 3
> > 
> >
> > "A hook script to verify commit message format. Called by..." so that 
> > the first sentence explains what the script does, rather than how it's 
> > called.

Actually that comment came with the template hook file 
(.git/hooks/commit-msg.sample).


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 4
> > 
> >
> > "should"? Are you unsure if it will?

I believe that the `if it wants to stop the commit` part of the sentence makes 
it pretty explicit, no? Again, this comment is part of the template hook file. 
If you have a suggestion on how to rewrite it I'll gladly follow it :-).


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, lines 6-7
> > 
> >
> > # To enable this hook, do this from the root of the repo:
> > #
> > # $ ln -s ../../support/hooks/commit-message .git/hooks/commit-message

In our case it's done automatically by the bootstrap script. It's part of this 
same commit. Added a clarifying comment.


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 19
> > 
> >
> > None of the lines should exceed 72 chars, and the first line should 
> > ideally be even less (~50). Ideally this hook would error for a too-long 
> > summary line, and wrap the description.

Makes sense. Added a JIRA https://issues.apache.org/jira/browse/MESOS-4250 to 
follow up.


- Artem


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


On Dec. 25, 2015, 6:05 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41586/
> ---
> 
> (Updated Dec. 25, 2015, 6:05 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Partially enforced commit message guidelines with a hook.
> 
> 
> Diffs
> -
> 
>   bootstrap 89d986fd95dc16bbb79623ef92e3b14a2e7009f9 
>   support/hooks/commit-msg PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41586/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41586: Partially enforced commit message guidelines with a hook.

[Artem Harutyunyan]

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

(Updated Dec. 25, 2015, 6:05 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.


Repository: mesos


Description
---

Partially enforced commit message guidelines with a hook.


Diffs (updated)
-

  bootstrap 89d986fd95dc16bbb79623ef92e3b14a2e7009f9 
  support/hooks/commit-msg PRE-CREATION 

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


Testing
---


Thanks,

Artem Harutyunyan

Re: Review Request 41715: Support parsing url in libprocess.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41715]

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

- Mesos ReviewBot


On Dec. 25, 2015, 8:10 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41715/
> ---
> 
> (Updated Dec. 25, 2015, 8:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing url in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 19261502be220aaa40add7ce30a9b2b65d1d9fdc 
> 
> Diff: https://reviews.apache.org/r/41715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 41715: Support parsing url in libprocess.

[Timothy Chen]

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

Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


Repository: mesos


Description
---

Support parsing url in libprocess.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
f0666f0fa48c4f3a98332d12066561a02a715236 
  3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
  3rdparty/libprocess/src/tests/http_tests.cpp 
19261502be220aaa40add7ce30a9b2b65d1d9fdc 

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


Testing
---

make check


Thanks,

Timothy Chen