You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2012/06/18 20:28:25 UTC
Review Request: Add APIs for event notification in cgroups.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
> On June 28, 2012, 7:11 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 347
> > <https://reviews.apache.org/r/5395/diff/8/?file=117045#file117045line347>
> >
> > Why not do this in the call to eventfd?
Done.
> On June 28, 2012, 7:11 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 353
> > <https://reviews.apache.org/r/5395/diff/8/?file=117045#file117045line353>
> >
> > Also, why not do this in the call to eventfd?
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8723
-----------------------------------------------------------
On June 28, 2012, 7:40 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 28, 2012, 7:40 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8723
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18398>
Why not do this in the call to eventfd?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18399>
Also, why not do this in the call to eventfd?
- Benjamin Hindman
On June 28, 2012, 1:36 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 28, 2012, 1:36 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8734
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18427>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18428>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18429>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18424>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18423>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18422>
Done.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18421>
Done.
- Jie Yu
On June 28, 2012, 7:40 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 28, 2012, 7:40 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8726
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18411>
No need to make this public, you can keep it protected.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18408>
You should put all of the 'start' logic into 'initialize' and all of the 'stop' logic into 'finalize'. Then just pass a "lambda" to terminate self when the future is discarded (a few lines above).
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18410>
Just 'terminate(self())' after putting the logic into finalize.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18403>
sizeof(data)
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18402>
Kill newline.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18405>
Why not just return Future?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18406>
You can set Future.failed here.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18404>
While unlikely to be exposed, there is actually a race here ... it's possible listener has been garbage collected before doing 'listener->future()'.
Two options: Pull the future out before you spawn, or just pass a promise (allocated on the heap) into the process.
- Benjamin Hindman
On June 28, 2012, 7:40 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 28, 2012, 7:40 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
> On July 2, 2012, 5:06 a.m., Benjamin Hindman wrote:
> > src/tests/cgroups_tests.cpp, line 401
> > <https://reviews.apache.org/r/5395/diff/10/?file=117203#file117203line401>
> >
> > Seeing this used again makes me think that there should really be a second test fixture, one where the body of 'prepare' is inside SetUp and one where it doesn't exist. Thoughts?
Done. Refactored.
> On July 2, 2012, 5:06 a.m., Benjamin Hindman wrote:
> > src/tests/cgroups_tests.cpp, line 426
> > <https://reviews.apache.org/r/5395/diff/10/?file=117203#file117203line426>
> >
> > Put the value expected first (here and others below).
Done.
> On July 2, 2012, 5:06 a.m., Benjamin Hindman wrote:
> > src/tests/cgroups_tests.cpp, line 440
> > <https://reviews.apache.org/r/5395/diff/10/?file=117203#file117203line440>
> >
> > Kill newline.
Done.
> On July 2, 2012, 5:06 a.m., Benjamin Hindman wrote:
> > src/tests/cgroups_tests.cpp, line 450
> > <https://reviews.apache.org/r/5395/diff/10/?file=117203#file117203line450>
> >
> > Making 'size' be in terms of 'limit' is more robust.
Done.
> On July 2, 2012, 5:06 a.m., Benjamin Hindman wrote:
> > src/tests/cgroups_tests.cpp, line 451
> > <https://reviews.apache.org/r/5395/diff/10/?file=117203#file117203line451>
> >
> > Space after '(char*)' please.
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8780
-----------------------------------------------------------
On June 29, 2012, 12:47 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 29, 2012, 12:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8780
-----------------------------------------------------------
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/5395/#comment18582>
Seeing this used again makes me think that there should really be a second test fixture, one where the body of 'prepare' is inside SetUp and one where it doesn't exist. Thoughts?
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/5395/#comment18580>
Put the value expected first (here and others below).
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/5395/#comment18579>
Kill newline.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/5395/#comment18584>
Making 'size' be in terms of 'limit' is more robust.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/5395/#comment18583>
Space after '(char*)' please.
- Benjamin Hindman
On June 29, 2012, 12:47 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 29, 2012, 12:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
> On July 24, 2012, 6:17 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 812
> > <https://reviews.apache.org/r/5395/diff/12/?file=123388#file123388line812>
> >
> > We've typically called these variables "out" instead of ss.
Done.
> On July 24, 2012, 6:17 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, lines 892-893
> > <https://reviews.apache.org/r/5395/diff/12/?file=123388#file123388line892>
> >
> > How about instead of making reading an Option, you just discard the future in 'finalize' all the time? It doesn't hurt to call discard (even if the future was never actually associated with anything) and that way you don't have to have the extra '.get()' everywhere or the need to reason about when it's "okay" to call '.get()' because the Option isSome and when it's not.
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review9398
-----------------------------------------------------------
On July 24, 2012, 9:03 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated July 24, 2012, 9:03 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp cec9e36
> src/linux/cgroups.cpp 93c140f
> src/tests/cgroups_tests.cpp 7f0f901
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review9398
-----------------------------------------------------------
Ship it!
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment20127>
We've typically called these variables "out" instead of ss.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment20128>
How about instead of making reading an Option, you just discard the future in 'finalize' all the time? It doesn't hurt to call discard (even if the future was never actually associated with anything) and that way you don't have to have the extra '.get()' everywhere or the need to reason about when it's "okay" to call '.get()' because the Option isSome and when it's not.
- Benjamin Hindman
On July 16, 2012, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated July 16, 2012, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp cec9e36
> src/linux/cgroups.cpp 93c140f
> src/tests/cgroups_tests.cpp 7f0f901
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated July 24, 2012, 9:03 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Addressed Ben's review comments.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp cec9e36
src/linux/cgroups.cpp 93c140f
src/tests/cgroups_tests.cpp 7f0f901
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated July 16, 2012, 6:32 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Updated to fit the new trunk.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp cec9e36
src/linux/cgroups.cpp 93c140f
src/tests/cgroups_tests.cpp 7f0f901
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated July 3, 2012, 12:14 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Addressed review feedback for the unit test.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 29, 2012, 12:47 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Addressed Ben's feedback.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 28, 2012, 7:40 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Eventfd wrapper.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 28, 2012, 1:36 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
eventfd issue.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 27, 2012, 11:58 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Adapt to the new process::io interface.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 26, 2012, 6:21 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Address Ben's review feedback. This patch is based on the process::io::read (nonblocking read) patch (https://reviews.apache.org/r/5576/).
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 858
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line858>
> >
> > This is great, but can we think of any other abstractions this can use to make this stuff even simpler? In particular, what about a process::io::read which handles spurious events? See this: https://reviews.apache.org/r/5567
Done.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 870
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line870>
> >
> > virtual
Done.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 872
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line872>
> >
> > virtual
Done.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 933
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line933>
> >
> > Is this supposed to account for spurious events? Or couldn't "none" here mean EOF?
Done.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 383
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line383>
> >
> > This is so similar to just utils::os::read that we should create another utils version that does the conversion for you (probably via std::stringstream or boost::lexical_cast). The big difference here is returning 'none' for EAGAIN, but see my comment below.
Done.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 328
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line328>
> >
> > Older versions of glibc don't have eventfd. We need to wrap it if necessary.
Maybe we should add a check in the autotool to check the glibc version. But anyway, cgroups is not available in the kernel if eventfd is not.
Therefore, I think there are two possibilities if eventfd is not available:
1) if you try to compile on that platform, compilation will not succeed
2) if you compile on other machine and try to run the binary on a machine without eventfd, the linking will fail
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8426
-----------------------------------------------------------
On June 26, 2012, 6:21 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 26, 2012, 6:21 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On June 25, 2012, 8:01 p.m., Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 328
> > <https://reviews.apache.org/r/5395/diff/5/?file=112400#file112400line328>
> >
> > Older versions of glibc don't have eventfd. We need to wrap it if necessary.
>
> Jie Yu wrote:
> Maybe we should add a check in the autotool to check the glibc version. But anyway, cgroups is not available in the kernel if eventfd is not.
>
> Therefore, I think there are two possibilities if eventfd is not available:
>
> 1) if you try to compile on that platform, compilation will not succeed
> 2) if you compile on other machine and try to run the binary on a machine without eventfd, the linking will fail
I'm not worried about not having eventfd, I'm worried about not having the glibc wrapper for eventfd. You'll need to check if you have a version of glibc that has that wrapper, and if not, include something like:
#define EFD_SEMAPHORE 1
#define EFD_CLOEXEC O_CLOEXEC
#define EFD_NONBLOCK O_NONBLOCK
int eventfd(unsigned int count, int flags)
{
return syscall(__NR_eventfd2, count, flags);
}
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8426
-----------------------------------------------------------
On June 27, 2012, 11:58 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 27, 2012, 11:58 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/#review8426
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18108>
Older versions of glibc don't have eventfd. We need to wrap it if necessary.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18235>
This is so similar to just utils::os::read that we should create another utils version that does the conversion for you (probably via std::stringstream or boost::lexical_cast). The big difference here is returning 'none' for EAGAIN, but see my comment below.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18238>
This is great, but can we think of any other abstractions this can use to make this stuff even simpler? In particular, what about a process::io::read which handles spurious events? See this: https://reviews.apache.org/r/5567
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18110>
virtual
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18111>
virtual
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5395/#comment18237>
Is this supposed to account for spurious events? Or couldn't "none" here mean EOF?
- Benjamin Hindman
On June 20, 2012, 6:51 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5395/
> -----------------------------------------------------------
>
> (Updated June 20, 2012, 6:51 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
>
> This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
>
> This interface is general to all the events in cgroups.
>
>
> Diffs
> -----
>
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5395/diff/
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 20, 2012, 6:51 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Adapt to the new cgroups basic APIs.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 19, 2012, 4:56 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Rename test name to adapt to the new gtest hack.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 18, 2012, 10:41 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
The newest version.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu
Re: Review Request: Add APIs for event notification in cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5395/
-----------------------------------------------------------
(Updated June 18, 2012, 9 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Reformat a little bit. Remove the extra promise argument in EventListener.
Description
-------
In cgroups, sometimes you want to be notified if some certain events happen. For example, users may want to be notified when an out-of-memory event happens.
This patch introduce a function called "listenEvent" which returns an instance of Future so that you can check the status of this future to see whether any event happens.
This interface is general to all the events in cgroups.
Diffs (updated)
-----
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5395/diff/
Testing
-------
On Linux machines, make check.
Thanks,
Jie Yu