Re: Review Request 39636: FreeBSD: Add basic support to stout

[David Forsythe]

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

(Updated Nov. 14, 2015, 9:03 a.m.)


Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

FreeBSD: Add basic support to stout


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
6709f5e7f6233983f389203278a0e42694591230 
  3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
9428717fac4655898d7768957f02937592e1a398 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
e49783a438157706b1be9745436bf666f45cab8b 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/bootid.hpp 
3f0bad6bba7297bcfd5e0787cf8cabdbb19257fe 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
a5719bdf26b59955e7c47b6c5677bb6e847358ab 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
828c9c777b1b0e067c2551b79b9747a3cf4fb0aa 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp 
e9b05ef3b59fd068137cb12e36591de2d4a801a1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
0a98e9e310d3931c2341053595b7d62f68214783 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp 
8a8ede325cfe8f024e1be4db24b0c8118d18f359 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
  3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
4cc781bddbf7ee10cc0671f62d710fb4fa80e293 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
8791cfe628563b25ac86a8170966d458c3a9c0c8 

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


Testing
---

make check on ubuntu 14.04

gmake check on FreeBSD 10.2 (failing, log attached)


File Attachments


check.log
  
https://reviews.apache.org/media/uploaded/files/2015/10/25/e661fec8-b342-440f-9b66-85bf0dd13fee__check.log


Thanks,

David Forsythe

Re: Review Request 39634: FreeBSD: Enable mesos build and start fixing some tests

[David Forsythe]

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

(Updated Nov. 14, 2015, 9:02 a.m.)


Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

FreeBSD: Enable mesos build and start fixing some tests


Diffs (updated)
-

  configure.ac 8b28ac78eeb3e3b5905b411b4bc0db3ccf0f543f 
  src/Makefile.am 92d07c3da897c7664c63e2af91d45921d66f64aa 
  src/tests/attributes_tests.cpp 4fc0c31c3b2eb745432818c99746a097fde65df3 
  src/tests/resources_tests.cpp 0d084fd97ec108d5ec2d050eddc2e80ea81ffac0 
  src/tests/values_tests.cpp e9b1079bbadf05390b39bedd5ad5677f3d4ec0d8 

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


Testing
---

make check on ubuntu 14.04


Thanks,

David Forsythe

Re: Review Request 39635: FreeBSD: Enable libprocess build and disable failing test

[David Forsythe]

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

(Updated Nov. 14, 2015, 9:02 a.m.)


Review request for mesos and Ian Downes.


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


Repository: mesos


Description
---

FreeBSD: Enable libprocess build and disable failing test


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
0adbe539afaf683e4a85582463a2930049a63998 
  3rdparty/libprocess/configure.ac 40801653a7fb9a943dfe33913161d28ef24040c3 
  3rdparty/libprocess/src/config.hpp 721816432621c78b3ff5cc3176753821e9ef7975 
  3rdparty/libprocess/src/tests/http_tests.cpp 
5e70f1896a86104ac01dfe725eb4d7d1d25bee77 

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


Testing
---

make check on ubuntu


Thanks,

David Forsythe

Re: Review Request 39636: FreeBSD: Add basic support to stout

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39634, 39635, 39636]

All tests passed.

- Mesos ReviewBot


On Nov. 14, 2015, 9:03 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39636/
> ---
> 
> (Updated Nov. 14, 2015, 9:03 a.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: MESOS-1563
> https://issues.apache.org/jira/browse/MESOS-1563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FreeBSD: Add basic support to stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 6709f5e7f6233983f389203278a0e42694591230 
>   3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
> 9428717fac4655898d7768957f02937592e1a398 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> e49783a438157706b1be9745436bf666f45cab8b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/bootid.hpp 
> 3f0bad6bba7297bcfd5e0787cf8cabdbb19257fe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> a5719bdf26b59955e7c47b6c5677bb6e847358ab 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 828c9c777b1b0e067c2551b79b9747a3cf4fb0aa 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp 
> e9b05ef3b59fd068137cb12e36591de2d4a801a1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
> 0a98e9e310d3931c2341053595b7d62f68214783 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp 
> 8a8ede325cfe8f024e1be4db24b0c8118d18f359 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 4cc781bddbf7ee10cc0671f62d710fb4fa80e293 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 8791cfe628563b25ac86a8170966d458c3a9c0c8 
> 
> Diff: https://reviews.apache.org/r/39636/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> gmake check on FreeBSD 10.2 (failing, log attached)
> 
> 
> File Attachments
> 
> 
> check.log
>   
> https://reviews.apache.org/media/uploaded/files/2015/10/25/e661fec8-b342-440f-9b66-85bf0dd13fee__check.log
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Joerg Schad]

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



src/master/quota_handler.cpp (line 239)


Good point especially after looking at your status endpoint for maintenance 
primitives (37364). Will add that.


- Joerg Schad


On Oct. 23, 2015, 10:34 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39492/
> ---
> 
> (Updated Oct. 23, 2015, 10:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added status handling for quota master endpoint.
> Tests are in next Review.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39492/diff/
> 
> 
> Testing
> ---
> 
> Test output: 
> {"quota":[{"guarantee":{"cpus":4.0,"disk":0,"mem":5.0},"role":"test"}]}
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

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. 14, 2015, 3:44 p.m.)


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


Changes
---

Reworked the sanity check. Please review.


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 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. 14, 2015, 3:46 p.m.)


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


Changes
---

More test ideas.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 09c3d8cb3bcdaca7a061574646da8f51cc2c1698 
  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 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.
> 
> Alexander Rukletsov wrote:
> This can happen during the allocation cycle, but we do not persist laid 
> aside resources between allocation cycles. Without refactoring `allocate()` 
> we do not know whether we get a suitable agent, hence we have to lay aside. 
> But at the next allocation cycle, `r1`'s quota is satisfied and `f2` will get 
> `a1`'s 4GB, which is OK in my opinion.
> 
> Qian Zhang wrote:
> Yes, I understand ```f2``` will get 4GB at the ***next*** allocation 
> cycle. But with the proposal in my first post, in a ***single*** allocation 
> cycle, both ```f1``` and ```f2``` can get 4GB respectively because when we 
> find we can not allocate ```a1```'s 4GB to f1 due to the filter, we will NOT 
> lay aside resources at this point, instead we will try ```a2``` and allocate 
> ```a2```'s 4GB to f1, and then when we handle the fair share, we will 
> allocate ```a2```'s 4GB to ```f2```. I think this proposal is also aligned 
> with the principle mentioned in the design doc: quota first, fair share 
> second. My understanding to this design principle is, we handle all role's 
> quota for all slaves first, and then handle all role's fair share for all 
> slaves (my proposal), rather than for ***each slave*** handle all role's 
> quota and then all role's fair share (this patch).
> 
> Alexander Rukletsov 

Re: Review Request 40241: Fixed a bug with hanging editor reported by Ben Mahler.

[Artem Harutyunyan]

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

(Updated Nov. 14, 2015, 8:55 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, Joseph Wu, and 
Vinod Kone.


Changes
---

Addressed comments and fixed errors.


Repository: mesos


Description (updated)
---

`subprocess.check_output()` used pipes to capture STDOUT and STDERR. Because of 
this certain editors (such as vi) would hang when invoked during git commit. To 
mitigate that I switched to `subprocess.call()` and did some code refactoring 
to accomodate that change.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---


Thanks,

Artem Harutyunyan

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

[Artem Harutyunyan]

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

(Updated Nov. 14, 2015, 9:02 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments, fixed commit options.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  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 39410: Added support for github to apply-reviews.py.

[Artem Harutyunyan]

> On Nov. 10, 2015, 12:18 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 192
> > 
> >
> > s/-e/--amend/ ?

Turns out `--amend` is very different from `-e`. `--amend` causes all commits 
in the branch to be squashed together into one. Excerpt from the man page 
(https://git-scm.com/docs/git-commit):

```
...
It is a rough equivalent for:


$ git reset --soft HEAD^
$ ... do something else to come up with the right tree ...
$ git commit -c ORIG_HEAD
```


- Artem


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


On Nov. 12, 2015, 10:03 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 12, 2015, 10:03 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 40225: Fixed the Review Board URL in the commit message.

[Artem Harutyunyan]

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

(Updated Nov. 14, 2015, 9:08 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, Joseph Wu, and 
Vinod Kone.


Summary (updated)
-

Fixed the Review Board URL in the commit message.


Repository: mesos


Description
---

Adjusted Review Board URL in comment.


Diffs
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---


Thanks,

Artem Harutyunyan

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

[Artem Harutyunyan]

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

(Updated Nov. 14, 2015, 9:04 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 (updated)
-

  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 40241: Fixed apply-reviews.py to prevent git commit editors from hanging.

[Artem Harutyunyan]

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

(Updated Nov. 14, 2015, 9:07 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, Joseph Wu, and 
Vinod Kone.


Summary (updated)
-

Fixed apply-reviews.py to prevent git commit editors from hanging.


Repository: mesos


Description
---

`subprocess.check_output()` used pipes to capture STDOUT and STDERR. Because of 
this certain editors (such as vi) would hang when invoked during git commit. To 
mitigate that I switched to `subprocess.call()` and did some code refactoring 
to accomodate that change.


Diffs
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---


Thanks,

Artem Harutyunyan