Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

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

(Updated May 29, 2015, 12:56 p.m.)


Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone.


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


Repository: mesos


Description
---

Support manipulating scheduler policy on Linux.


Diffs (updated)
-

  src/Makefile.am e7281ac6667562a27b0427b0ba7f41de98702928 
  src/linux/sched.hpp PRE-CREATION 
  src/tests/sched_tests.cpp PRE-CREATION 

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


Testing
---

Added test.


Thanks,

Ian Downes

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Vinod Kone]

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

Ship it!



src/tests/sched_tests.cpp


new line.


- Vinod Kone


On May 19, 2015, 7:58 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 19, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Niklas Nielsen]

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

Ship it!



src/tests/sched_tests.cpp


Thanks for splitting up! I would still like a short snippit of:
1) Make sure child inherits parent scheduling policy
2) Make sure parent can change child scheduling policy

Or something along those lines.


- Niklas Nielsen


On May 19, 2015, 12:58 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 19, 2015, 12:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Joris Van Remoortere]

> On May 19, 2015, 6:56 p.m., Joris Van Remoortere wrote:
> > Hey Ian, looks good.
> > 
> > I have a prototype for testing the pre-emption. I will add some reviews 
> > that depend on this patch set, as I will need to introduce sched_affinity 
> > first.

I've added the test in subsequent reviews:
https://reviews.apache.org/r/34442
https://reviews.apache.org/r/34456


- Joris


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


On May 19, 2015, 7:58 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 19, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Vinod Kone]

> On May 19, 2015, 4:53 p.m., Niklas Nielsen wrote:
> > src/linux/sched.hpp, line 51
> > 
> >
> > Want to add some context to which pid that you couldn't get the policy 
> > for? For example: "Failed to get scheduler policy for pid: 1234"
> 
> Vinod Kone wrote:
> Nik, the general rule is that we don't include stuff in the log message 
> that the caller already has access to, because it makes it hard to chain 
> errors (e.g., the caller has no idea whether to include pid in its own log 
> message or not).
> 
> Niklas Nielsen wrote:
> Okay - didn't know. Should we put this in the style guide then?
> 
> We don't print the pid in the call site either in: 
> https://reviews.apache.org/r/34310

+1 to updating style guide.

regarding caller not printing pid: it's up to the caller. and yea, it makes 
sense for the caller to print pid in this specific case. cc @idownes.


- Vinod


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


On May 19, 2015, 7:58 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 19, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Niklas Nielsen]

> On May 19, 2015, 9:53 a.m., Niklas Nielsen wrote:
> > src/linux/sched.hpp, line 51
> > 
> >
> > Want to add some context to which pid that you couldn't get the policy 
> > for? For example: "Failed to get scheduler policy for pid: 1234"
> 
> Vinod Kone wrote:
> Nik, the general rule is that we don't include stuff in the log message 
> that the caller already has access to, because it makes it hard to chain 
> errors (e.g., the caller has no idea whether to include pid in its own log 
> message or not).

Okay - didn't know. Should we put this in the style guide then?

We don't print the pid in the call site either in: 
https://reviews.apache.org/r/34310


- Niklas


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


On May 19, 2015, 12:58 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 19, 2015, 12:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

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

(Updated May 19, 2015, 12:58 p.m.)


Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone.


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


Repository: mesos


Description
---

Support manipulating scheduler policy on Linux.


Diffs (updated)
-

  src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
  src/linux/sched.hpp PRE-CREATION 
  src/tests/sched_tests.cpp PRE-CREATION 

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


Testing
---

Added test.


Thanks,

Ian Downes

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

> On May 19, 2015, 9:53 a.m., Niklas Nielsen wrote:
> > src/tests/sched_tests.cpp, line 45
> > 
> >
> > Any reason not to use the fork abstraction? If not, want to explicitly 
> > use ::fork()?

I only want a single child so the Fork abstraction is OTT IMHO. Changed to 
::fork to be explicit.


> On May 19, 2015, 9:53 a.m., Niklas Nielsen wrote:
> > src/tests/sched_tests.cpp, line 34
> > 
> >
> > Can you go in a bit more detail about what this test does? What is the 
> > point of starting the children and having them sleep and change the parent 
> > sched policy?

It's testing we can set the scheduling policy for a different process to the 
caller. I split this into a second test to make it clearer.


- Ian


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


On May 18, 2015, 1:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 1:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

> On May 19, 2015, 11:49 a.m., Vinod Kone wrote:
> > can you please make sure to run 'make check' on OSX?

It makes and tests clean on OSX; did you see something different?
```
$ make check

[--] Global test environment tear-down
[==] 570 tests from 86 test cases ran. (123273 ms total)
[  PASSED  ] 570 tests.

  YOU HAVE 7 DISABLED TESTS

Making check in ec2
make[1]: Nothing to be done for `check'.
$ uname -v
Darwin Kernel Version 13.4.0: Wed Mar 18 16:20:14 PDT 2015; 
root:xnu-2422.115.14~1/RELEASE_X86_64
```


> On May 19, 2015, 11:49 a.m., Vinod Kone wrote:
> > src/Makefile.am, line 539
> > 
> >
> > should be inside "if OS_LINUX" above at #459?

That's for source only? We've put all the other linux/*.hpp here.


- Ian


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


On May 18, 2015, 1:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 1:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Joris Van Remoortere]

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



src/linux/sched.hpp


Can we actually rename this to `getPolicy` or something similar, so that we 
can augment this file with other scheduler specific functions? (e.g. affinity)


- Joris Van Remoortere


On May 18, 2015, 8:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 8:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Joris Van Remoortere]

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


Hey Ian, looks good.

I have a prototype for testing the pre-emption. I will add some reviews that 
depend on this patch set, as I will need to introduce sched_affinity first.


src/linux/sched.hpp


Would this be easier to read if we flipped the logic?
```
if ((policy != FIFO && policy != RR) && priority != 0) {
```
Feel free to ignore, either way works :-)


- Joris Van Remoortere


On May 18, 2015, 8:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 8:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Vinod Kone]

> On May 19, 2015, 4:53 p.m., Niklas Nielsen wrote:
> > src/linux/sched.hpp, line 51
> > 
> >
> > Want to add some context to which pid that you couldn't get the policy 
> > for? For example: "Failed to get scheduler policy for pid: 1234"

Nik, the general rule is that we don't include stuff in the log message that 
the caller already has access to, because it makes it hard to chain errors 
(e.g., the caller has no idea whether to include pid in its own log message or 
not).


- Vinod


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


On May 18, 2015, 8:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 8:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Vinod Kone]

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


can you please make sure to run 'make check' on OSX?


src/Makefile.am


should be inside "if OS_LINUX" above at #459?



src/tests/sched_tests.cpp


Why do we have to assume the current policy is OTHER? Can't we just grab 
the current policy and reset it after testing changing policy?


- Vinod Kone


On May 18, 2015, 8:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 8:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Niklas Nielsen]

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



src/linux/sched.hpp


Do we need to have a global invariant, that this isn't included on mac?



src/linux/sched.hpp


Does it make sense to const this?



src/linux/sched.hpp


Want to add some context to which pid that you couldn't get the policy for? 
For example: "Failed to get scheduler policy for pid: 1234"



src/linux/sched.hpp


How about adding the pid here? :)



src/linux/sched.hpp


Same here



src/tests/sched_tests.cpp


Can you go in a bit more detail about what this test does? What is the 
point of starting the children and having them sleep and change the parent 
sched policy?



src/tests/sched_tests.cpp


Any reason not to use the fork abstraction? If not, want to explicitly use 
::fork()?



src/tests/sched_tests.cpp


Do you want a return here, to make sure the children doesn't continue into 
the parent code?



src/tests/sched_tests.cpp


It would be great to have a reap abstraction, so we don't have to fiddle 
too much with the masked types. Could we have used the reaper?


- Niklas Nielsen


On May 18, 2015, 1:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 1:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

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

(Updated May 18, 2015, 1:48 p.m.)


Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Support manipulating scheduler policy on Linux.


Diffs (updated)
-

  src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
  src/linux/sched.hpp PRE-CREATION 
  src/tests/sched_tests.cpp PRE-CREATION 

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


Testing
---

Added test.


Thanks,

Ian Downes

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

> On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> > src/tests/sched_tests.cpp, line 40
> > 
> >
> > Could you elaborate on this comment and explain what you are testing in 
> > the child? I don't think it's clear.
> > 
> > Should we also test the inheritance? (i.e. fork while in the IDLE 
> > policy, and then verify in the child?)

It's testing that we can set the policy for a child/different process, not that 
a child process can set its own policy. I amended the comment to make that 
clearer.

Regarding inheritance, this is specified for sched_setscheduler() and I'm not 
sure I want to get into the business of testing every system call ;-) ?

"Child processes inherit the scheduling policy and parameters across a fork(2). 
 The scheduling policy and parameters are preserved across execve(2)."


> On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> > src/tests/sched_tests.cpp, line 30
> > 
> >
> > Do you want to add a TODO for some tests that:
> > 1) Actually test that the priority / interruption behavior is as 
> > expected
> > 2) Shows people how to use this / explains how it works
> > 
> > I think 1) and 2) can be done in the same test with nice comments :-)

Added a TODO.

Testing priority is easy. Do you have suggestions on how to test preemption 
behavior?


> On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> > src/linux/sched.hpp, line 53
> > 
> >
> > Should we add the pid for which this failed to the error message?

I think the caller should be responsible for this?


> On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> > src/linux/sched.hpp, line 73
> > 
> >
> > Should we add the pid for which this failed to the error message?

I think the caller should be responsible for this?


- Ian


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


On May 18, 2015, 10:33 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 18, 2015, 10:33 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

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

(Updated May 18, 2015, 10:33 a.m.)


Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone.


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


Repository: mesos


Description
---

Support manipulating scheduler policy on Linux.


Diffs
-

  src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
  src/linux/sched.hpp PRE-CREATION 
  src/tests/sched_tests.cpp PRE-CREATION 

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


Testing
---

Added test.


Thanks,

Ian Downes

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

[Joris Van Remoortere]

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


Great work Ian!

Could you add the dependent reviews for this chain? I'm missing some dependent 
code when trying to apply 34309 + 34310 on master right now.

Should we put in any guards in for the POSIX / LINUX mismatch bug mentioned 
here: http://linux.die.net/man/2/sched_getscheduler
If we don't need any guards, should we mention it at the call site or in a 
top-level comment in the header?

In previous projects I have used RAII to wrap these kinds of utilities. The 
advantage of that is that we always "clean up" back to the original policy upon 
destruction, rather than relying on manual calls to reset to the original 
policy. This is a larger pattern change for more than just this utility, but I 
thought I would start the conversation. Maybe we can discuss it further at one 
of the community meetings?


src/linux/sched.hpp


unused?



src/linux/sched.hpp


In the comment below you use a `,` to split the `isSome` case from the 
`isNone` case. Let's be consistent? Maybe we can clarify the `caller` part as 
well:

`Return the current scheduling policy for the specified pid, or for the 
caller if pid is None() or zero.`



src/linux/sched.hpp


Should we add the pid for which this failed to the error message?



src/linux/sched.hpp


Can we add some checks for the condition mentioned in the comment? (i.e. 
priority only making sense for the FIFO and RR policy). The EINVAL error code 
from sched_setscheduler() is rather over-loaded, so I think this is a great 
opportunity to help our users and give them a more helpful error message! :-)



src/linux/sched.hpp


Should we add the pid for which this failed to the error message?



src/linux/sched.hpp


missing trailing `{` for namespace.



src/tests/sched_tests.cpp


Do you want to add a TODO for some tests that:
1) Actually test that the priority / interruption behavior is as expected
2) Shows people how to use this / explains how it works

I think 1) and 2) can be done in the same test with nice comments :-)



src/tests/sched_tests.cpp


1) This test requires elevated (CAP_SYS) permissions. We can use the 
`ROOT_XXX` prefix to signify this so it doesn't fail during non-priveleged user 
test runs? I don't know if we want to introduce more filters for specific 
permission requirements (like `CAPSYS_XXX`? That's probably a wishlist thing :-)

2) You have a trailing `;` here. This doesn't compile for me.



src/tests/sched_tests.cpp


Could you elaborate on this comment and explain what you are testing in the 
child? I don't think it's clear.

Should we also test the inheritance? (i.e. fork while in the IDLE policy, 
and then verify in the child?)


- Joris Van Remoortere


On May 16, 2015, 1:14 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34309/
> ---
> 
> (Updated May 16, 2015, 1:14 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2652
> https://issues.apache.org/jira/browse/MESOS-2652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support manipulating scheduler policy on Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/linux/sched.hpp PRE-CREATION 
>   src/tests/sched_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34309/diff/
> 
> 
> Testing
> ---
> 
> Added test.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Review Request 34309: Support manipulating scheduler policy on Linux.

[Ian Downes]

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

Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone.


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


Repository: mesos


Description
---

Support manipulating scheduler policy on Linux.


Diffs
-

  src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
  src/linux/sched.hpp PRE-CREATION 
  src/tests/sched_tests.cpp PRE-CREATION 

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


Testing
---

Added test.


Thanks,

Ian Downes