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 2012/10/30 20:39:00 UTC
Review Request: Added an ABORT function to stout.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
This is part 1, where I'd like to make sure this is the right way function to have.
This way, we have a nice helper for aborting, like CHECK() or LOG(FATAL), however these two:
1. Print out a stack trace, which we may not want sometimes.
2. Perform locking, and can't be used within a fork.
Part 2, will refactor usage of:
std::cerr << "Cgroups can't be enabled." << std::endl;
abort();
to become:
ABORT("Cgroups can't be enabled.");
Part 3, will kill the old files:
stout/fatal.hpp
libprocess/src/fatal.hpp
libprocess/src/fatal.cpp
P.S. I looked at how glog implements it's style of CHECK() << "Foo" << "bar";
It appears to call an abort immediately, but they have signal handlers that then catch the abort and appear to wait if someone else is logging, and flush the logs to ensure the remaining stuff sent to the check stream are flushed. Needless to say, that's way more complexity than I'd like to have if we want this to be standalone in stout.
Diffs
-----
third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
third_party/libprocess/include/stout/abort.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/7781/diff/
Testing
-------
CentOS: make check
Thanks,
Ben Mahler
Re: Review Request: Added an ABORT function to stout.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/#review13048
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Oct. 30, 2012, 7:39 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7781/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2012, 7:39 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This is part 1, where I'd like to make sure this is the right way function to have.
>
> This way, we have a nice helper for aborting, like CHECK() or LOG(FATAL), however these two:
> 1. Print out a stack trace, which we may not want sometimes.
> 2. Perform locking, and can't be used within a fork.
>
> Part 2, will refactor usage of:
> std::cerr << "Cgroups can't be enabled." << std::endl;
> abort();
> to become:
> ABORT("Cgroups can't be enabled.");
>
> Part 3, will kill the old files:
> stout/fatal.hpp
> libprocess/src/fatal.hpp
> libprocess/src/fatal.cpp
>
>
> P.S. I looked at how glog implements it's style of CHECK() << "Foo" << "bar";
> It appears to call an abort immediately, but they have signal handlers that then catch the abort and appear to wait if someone else is logging, and flush the logs to ensure the remaining stuff sent to the check stream are flushed. Needless to say, that's way more complexity than I'd like to have if we want this to be standalone in stout.
>
>
> Diffs
> -----
>
> third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
> third_party/libprocess/include/stout/abort.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7781/diff/
>
>
> Testing
> -------
>
> CentOS: make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added an ABORT function to stout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/#review13118
-----------------------------------------------------------
This function requires the use of strings::format to create abort messages. I'd prefer if ABORT worked more like glog (i.e., ABORT() << "Some message with an int " << i << " here";
third_party/libprocess/include/stout/abort.hpp
<https://reviews.apache.org/r/7781/#comment28278>
const &
- Benjamin Hindman
On Oct. 30, 2012, 7:39 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7781/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2012, 7:39 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This is part 1, where I'd like to make sure this is the right way function to have.
>
> This way, we have a nice helper for aborting, like CHECK() or LOG(FATAL), however these two:
> 1. Print out a stack trace, which we may not want sometimes.
> 2. Perform locking, and can't be used within a fork.
>
> Part 2, will refactor usage of:
> std::cerr << "Cgroups can't be enabled." << std::endl;
> abort();
> to become:
> ABORT("Cgroups can't be enabled.");
>
> Part 3, will kill the old files:
> stout/fatal.hpp
> libprocess/src/fatal.hpp
> libprocess/src/fatal.cpp
>
>
> P.S. I looked at how glog implements it's style of CHECK() << "Foo" << "bar";
> It appears to call an abort immediately, but they have signal handlers that then catch the abort and appear to wait if someone else is logging, and flush the logs to ensure the remaining stuff sent to the check stream are flushed. Needless to say, that's way more complexity than I'd like to have if we want this to be standalone in stout.
>
>
> Diffs
> -----
>
> third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
> third_party/libprocess/include/stout/abort.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7781/diff/
>
>
> Testing
> -------
>
> CentOS: make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added an EXIT function to stout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/#review13450
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Nov. 7, 2012, 12:18 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7781/
> -----------------------------------------------------------
>
> (Updated Nov. 7, 2012, 12:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a consistent way for us to exit without alarming the user with a stack trace. E.g.
> EXIT(1) << "Cgroups cannot be enabled because you run an old kernel";
>
> Part 2, will refactor usage of:
> std::cerr << "Cgroups can't be enabled." << std::endl;
> abort();
> to become:
> EXIT(1) << "Cgroups can't be enabled.";
>
> Part 3, will kill the old files (if all the usages can be switched):
> stout/fatal.hpp
> libprocess/src/fatal.hpp
> libprocess/src/fatal.cpp
>
>
> Diffs
> -----
>
> third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
> third_party/libprocess/include/stout/exit.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7781/diff/
>
>
> Testing
> -------
>
> CentOS: make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added an EXIT function to stout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/
-----------------------------------------------------------
(Updated Nov. 7, 2012, 12:18 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Renamed the Exit struct to __Exit and moved it down.
Description
-------
This adds a consistent way for us to exit without alarming the user with a stack trace. E.g.
EXIT(1) << "Cgroups cannot be enabled because you run an old kernel";
Part 2, will refactor usage of:
std::cerr << "Cgroups can't be enabled." << std::endl;
abort();
to become:
EXIT(1) << "Cgroups can't be enabled.";
Part 3, will kill the old files (if all the usages can be switched):
stout/fatal.hpp
libprocess/src/fatal.hpp
libprocess/src/fatal.cpp
Diffs (updated)
-----
third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
third_party/libprocess/include/stout/exit.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/7781/diff/
Testing
-------
CentOS: make check
Thanks,
Ben Mahler
Re: Review Request: Added an EXIT function to stout.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/#review13128
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Nov. 6, 2012, 4:09 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7781/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2012, 4:09 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a consistent way for us to exit without alarming the user with a stack trace. E.g.
> EXIT(1) << "Cgroups cannot be enabled because you run an old kernel";
>
> Part 2, will refactor usage of:
> std::cerr << "Cgroups can't be enabled." << std::endl;
> abort();
> to become:
> EXIT(1) << "Cgroups can't be enabled.";
>
> Part 3, will kill the old files (if all the usages can be switched):
> stout/fatal.hpp
> libprocess/src/fatal.hpp
> libprocess/src/fatal.cpp
>
>
> Diffs
> -----
>
> third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
> third_party/libprocess/include/stout/exit.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7781/diff/
>
>
> Testing
> -------
>
> CentOS: make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Added an EXIT function to stout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7781/
-----------------------------------------------------------
(Updated Nov. 6, 2012, 4:09 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Done glog style now, thanks for the C++ trickery help ben!
EXIT(0) << "Foo " << 42;
Summary (updated)
-----------------
Added an EXIT function to stout.
Description (updated)
-------
This adds a consistent way for us to exit without alarming the user with a stack trace. E.g.
EXIT(1) << "Cgroups cannot be enabled because you run an old kernel";
Part 2, will refactor usage of:
std::cerr << "Cgroups can't be enabled." << std::endl;
abort();
to become:
EXIT(1) << "Cgroups can't be enabled.";
Part 3, will kill the old files (if all the usages can be switched):
stout/fatal.hpp
libprocess/src/fatal.hpp
libprocess/src/fatal.cpp
Diffs (updated)
-----
third_party/libprocess/Makefile.am f32c184ca8cc8852b6c90f3c85a8f99e10ed781b
third_party/libprocess/include/stout/exit.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/7781/diff/
Testing
-------
CentOS: make check
Thanks,
Ben Mahler