You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/07/02 00:45:59 UTC
Review Request 12215: Fixed a crash due to invalid utime / stime values
coming from /proc/pid/stat.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Brenden Matthews, and Vinod Kone.
Repository: mesos
Description
-------
This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
Diff: https://reviews.apache.org/r/12215/diff/
Testing
-------
make check
Also ran the isolator tests with 100 iterations.
Thanks,
Ben Mahler
Re: Review Request 12215: Fixed a crash due to invalid utime / stime values
coming from /proc/pid/stat.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 1, 2013, 10:54 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, lines 59-60
> > <https://reviews.apache.org/r/12215/diff/1/?file=314823#file314823line59>
> >
> > Would it make more sense to just return an error if the times are invalid?
> >
> > With the current logic it looks the callers of process() need to know that a Process could have invalid (-1) utime and stime and deal with it accordingly.
>
> Ben Mahler wrote:
> There are a few options, on OS X, we currently return -1 when we cannot obtain timing information so this mimics that behavior.
>
> I could alternatively return 0. The main issue with returning an error is that killtree and others want process information, regardless of whether the Process cpu times are available.
>
> One option is to make utime and stime with Try's, this enforces explicit handling of this Linux bug and the OS X issue. While also allowing those who are interested in pid, ppid, etc to still obtain the information. Thoughts?
utime and stime as Try sgtm.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/#review22631
-----------------------------------------------------------
On July 1, 2013, 10:45 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12215/
> -----------------------------------------------------------
>
> (Updated July 1, 2013, 10:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brenden Matthews, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
>
> Diff: https://reviews.apache.org/r/12215/diff/
>
>
> Testing
> -------
>
> make check
>
> Also ran the isolator tests with 100 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 12215: Fixed a crash due to invalid utime / stime values
coming from /proc/pid/stat.
Posted by Ben Mahler <be...@gmail.com>.
> On July 1, 2013, 10:54 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, lines 59-60
> > <https://reviews.apache.org/r/12215/diff/1/?file=314823#file314823line59>
> >
> > Would it make more sense to just return an error if the times are invalid?
> >
> > With the current logic it looks the callers of process() need to know that a Process could have invalid (-1) utime and stime and deal with it accordingly.
There are a few options, on OS X, we currently return -1 when we cannot obtain timing information so this mimics that behavior.
I could alternatively return 0. The main issue with returning an error is that killtree and others want process information, regardless of whether the Process cpu times are available.
One option is to make utime and stime with Try's, this enforces explicit handling of this Linux bug and the OS X issue. While also allowing those who are interested in pid, ppid, etc to still obtain the information. Thoughts?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/#review22631
-----------------------------------------------------------
On July 1, 2013, 10:45 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12215/
> -----------------------------------------------------------
>
> (Updated July 1, 2013, 10:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brenden Matthews, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
>
> Diff: https://reviews.apache.org/r/12215/diff/
>
>
> Testing
> -------
>
> make check
>
> Also ran the isolator tests with 100 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 12215: Fixed a crash due to invalid utime / stime values
coming from /proc/pid/stat.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/#review22631
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
<https://reviews.apache.org/r/12215/#comment46319>
Would it make more sense to just return an error if the times are invalid?
With the current logic it looks the callers of process() need to know that a Process could have invalid (-1) utime and stime and deal with it accordingly.
- Vinod Kone
On July 1, 2013, 10:45 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12215/
> -----------------------------------------------------------
>
> (Updated July 1, 2013, 10:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Brenden Matthews, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
>
> Diff: https://reviews.apache.org/r/12215/diff/
>
>
> Testing
> -------
>
> make check
>
> Also ran the isolator tests with 100 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 12215: Refactored os::Process to use Options for memory
and CPU information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/
-----------------------------------------------------------
(Updated July 2, 2013, 7:21 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Changed from Try to Option in os::Process.
Summary (updated)
-----------------
Refactored os::Process to use Options for memory and CPU information.
Repository: mesos
Description
-------
This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 3f4e9d4c64bf5a9027aa26aab24c6f02d76b8bfc
3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp 806949bb29ad15e97851697dcdeeb17396ebe459
3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp c5ad25bd03c573119494fb41e2cf075ecb67b3d8
Diff: https://reviews.apache.org/r/12215/diff/
Testing
-------
make check
Also ran the isolator tests with 100 iterations.
Thanks,
Ben Mahler
Re: Review Request 12215: Refactored os::Process to use Try's for memory and
CPU information.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/#review22671
-----------------------------------------------------------
Ship it!
+1 for s/Try/Option/.
- Vinod Kone
On July 2, 2013, 2:46 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12215/
> -----------------------------------------------------------
>
> (Updated July 2, 2013, 2:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 3f4e9d4c64bf5a9027aa26aab24c6f02d76b8bfc
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp 806949bb29ad15e97851697dcdeeb17396ebe459
> 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp c5ad25bd03c573119494fb41e2cf075ecb67b3d8
>
> Diff: https://reviews.apache.org/r/12215/diff/
>
>
> Testing
> -------
>
> make check
>
> Also ran the isolator tests with 100 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 12215: Refactored os::Process to use Try's for memory and
CPU information.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/#review22660
-----------------------------------------------------------
Ship it!
I like wrapping those values in a Try, alternatively you could just use an Option (which makes it look less like a half broken Process and more like a Process with no resource statistics). The important part is that users will need to do something like 'process.rss.isSome()'.
- Benjamin Hindman
On July 2, 2013, 2:46 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12215/
> -----------------------------------------------------------
>
> (Updated July 2, 2013, 2:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 3f4e9d4c64bf5a9027aa26aab24c6f02d76b8bfc
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp 806949bb29ad15e97851697dcdeeb17396ebe459
> 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp c5ad25bd03c573119494fb41e2cf075ecb67b3d8
>
> Diff: https://reviews.apache.org/r/12215/diff/
>
>
> Testing
> -------
>
> make check
>
> Also ran the isolator tests with 100 iterations.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 12215: Refactored os::Process to use Try's for memory and
CPU information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12215/
-----------------------------------------------------------
(Updated July 2, 2013, 2:46 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Refactored the memory / cpu information to be wrapped in a Try.
Summary (updated)
-----------------
Refactored os::Process to use Try's for memory and CPU information.
Repository: mesos
Description
-------
This was reported by Brenden Matthews on the mailing list. It appears that on some Linux systems the utime / stime values coming from /proc are corrupt.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp ef6c6c8f7839baaddcfd20249f2dbd25a7c3d25a
3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 3f4e9d4c64bf5a9027aa26aab24c6f02d76b8bfc
3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp 806949bb29ad15e97851697dcdeeb17396ebe459
3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp c5ad25bd03c573119494fb41e2cf075ecb67b3d8
Diff: https://reviews.apache.org/r/12215/diff/
Testing
-------
make check
Also ran the isolator tests with 100 iterations.
Thanks,
Ben Mahler