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/01/24 10:02:16 UTC
Review Request: Resource Monitoring 2: Adding global statistics.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9090/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Jan. 28, 2013, 7:54 p.m., Vinod Kone wrote:
> > third_party/libprocess/src/process.cpp, line 1415
> > <https://reviews.apache.org/r/9090/diff/1/?file=251527#file251527line1415>
> >
> > We don't want libraries to automatically spit log messages.
> >
> > In other words, we prefer VLOG so that users can specifically request them (by setting GLOG_v) if they are so interested.
Gotcha, I see now that there's only VLOG in process.cpp. It's too bad that it will require the user to re-run with GLOG_v if we need information from libprocess logging.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review15761
-----------------------------------------------------------
On Jan. 24, 2013, 9:02 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2013, 9:02 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Vinod Kone <vi...@gmail.com>.
> On Jan. 28, 2013, 7:54 p.m., Vinod Kone wrote:
> > third_party/libprocess/src/process.cpp, line 1415
> > <https://reviews.apache.org/r/9090/diff/1/?file=251527#file251527line1415>
> >
> > We don't want libraries to automatically spit log messages.
> >
> > In other words, we prefer VLOG so that users can specifically request them (by setting GLOG_v) if they are so interested.
>
> Ben Mahler wrote:
> Gotcha, I see now that there's only VLOG in process.cpp. It's too bad that it will require the user to re-run with GLOG_v if we need information from libprocess logging.
well, there is an endpoint (/logging?) to toggle logging at run time.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review15761
-----------------------------------------------------------
On Jan. 28, 2013, 10:49 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 10:49 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review15761
-----------------------------------------------------------
Ship it!
third_party/libprocess/src/process.cpp
<https://reviews.apache.org/r/9090/#comment33921>
We don't want libraries to automatically spit log messages.
In other words, we prefer VLOG so that users can specifically request them (by setting GLOG_v) if they are so interested.
- Vinod Kone
On Jan. 24, 2013, 9:02 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2013, 9:02 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review16807
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Feb. 6, 2013, 1:59 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2013, 1:59 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/process.hpp 0a6371813fd5fa585bca2b578404b52a59fc45f0
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Added global statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/
-----------------------------------------------------------
(Updated Feb. 22, 2013, 12:31 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs
-----
third_party/libprocess/include/process/process.hpp 0a6371813fd5fa585bca2b578404b52a59fc45f0
third_party/libprocess/include/process/statistics.hpp faefad2e374e08e4e0f88750c4c4eee74bce62d7
third_party/libprocess/src/process.cpp 878f2e866bcff8ecd743f16b1fd6f211bc5bf78a
third_party/libprocess/src/statistics.cpp 29d08f05c6ab7b69d587a44aab860b4dfe645b89
Diff: https://reviews.apache.org/r/9090/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 2: Added global statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/
-----------------------------------------------------------
(Updated Feb. 22, 2013, 12:31 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Changed summary.
Rebased off trunk.
Summary (updated)
-----------------
Resource Monitoring 2: Added global statistics.
Description
-------
This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/process.hpp 0a6371813fd5fa585bca2b578404b52a59fc45f0
third_party/libprocess/include/process/statistics.hpp faefad2e374e08e4e0f88750c4c4eee74bce62d7
third_party/libprocess/src/process.cpp 878f2e866bcff8ecd743f16b1fd6f211bc5bf78a
third_party/libprocess/src/statistics.cpp 29d08f05c6ab7b69d587a44aab860b4dfe645b89
Diff: https://reviews.apache.org/r/9090/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/
-----------------------------------------------------------
(Updated Feb. 6, 2013, 1:59 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
benh review
Description
-------
This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/process.hpp 0a6371813fd5fa585bca2b578404b52a59fc45f0
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9090/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 4, 2013, 9:31 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/src/statistics.cpp, lines 60-61
> > <https://reviews.apache.org/r/9090/diff/2/?file=252492#file252492line60>
> >
> > This is a departure from our standard style ... what caused you to add the CHECKs?
I don't need these CHECKs anymore, as originally without the leading '/' these were not routing! (Returning false).
As a note, we may want to make this a Try or have all callers use CHECK. As long as callers are not ignoring the return value.
> On Feb. 4, 2013, 9:31 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/statistics.hpp, line 14
> > <https://reviews.apache.org/r/9090/diff/2/?file=252490#file252490line14>
> >
> > I'd prefer to namespace statistics. I had imagined just an instance called 'statistics' within the process namespace. Two wins I see here. First, people see usages like 'process::statistics.set(...)' and they see it's clearly provided by libprocess. Likewise, they might do a 'using process::statistics' to just bring in the 'statistics' object from the process namespace. I recognize the desire for a dynamically allocated instance of Statistics in order to avoid cleanup issues (are there any?). Hence, a macro provides a nice means to handle that level of indirection, but maybe it's not so bad if people have to do 'statistics->set()'. On the flip side, one could imagine adding a statistics namespace that mirrors the Statistics interface yielding usage like 'process::statistics::set()'. I probably like this best, but recognize the unfortunate need to replicate the interface (and keep it in sync, etc).
As discussed, added a process::statistics pointer to a heap allocated Statistics object.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review16066
-----------------------------------------------------------
On Jan. 28, 2013, 10:49 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 10:49 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review16066
-----------------------------------------------------------
third_party/libprocess/include/process/statistics.hpp
<https://reviews.apache.org/r/9090/#comment34372>
I'd prefer to namespace statistics. I had imagined just an instance called 'statistics' within the process namespace. Two wins I see here. First, people see usages like 'process::statistics.set(...)' and they see it's clearly provided by libprocess. Likewise, they might do a 'using process::statistics' to just bring in the 'statistics' object from the process namespace. I recognize the desire for a dynamically allocated instance of Statistics in order to avoid cleanup issues (are there any?). Hence, a macro provides a nice means to handle that level of indirection, but maybe it's not so bad if people have to do 'statistics->set()'. On the flip side, one could imagine adding a statistics namespace that mirrors the Statistics interface yielding usage like 'process::statistics::set()'. I probably like this best, but recognize the unfortunate need to replicate the interface (and keep it in sync, etc).
third_party/libprocess/src/statistics.cpp
<https://reviews.apache.org/r/9090/#comment34370>
This is a departure from our standard style ... what caused you to add the CHECKs?
- Benjamin Hindman
On Jan. 28, 2013, 10:49 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 10:49 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/#review15819
-----------------------------------------------------------
third_party/libprocess/src/process.cpp
<https://reviews.apache.org/r/9090/#comment34074>
Right, but that won't ever catch the initialization logging. I've reverted this back to VLOG(1) anyway.
- Ben Mahler
On Jan. 28, 2013, 10:49 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9090/
> -----------------------------------------------------------
>
> (Updated Jan. 28, 2013, 10:49 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
>
> I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
>
>
> This addresses bug MESOS-324.
> https://issues.apache.org/jira/browse/MESOS-324
>
>
> Diffs
> -----
>
> third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
> third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
> third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
>
> Diff: https://reviews.apache.org/r/9090/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Resource Monitoring 2: Adding global statistics.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9090/
-----------------------------------------------------------
(Updated Jan. 28, 2013, 10:49 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Vinod's review.
Description
-------
This adds a global Statistics object for libprocess, which enables centralized stats across Processes.
I'd love to eventually plumb stats into libprocess and export to the webui! (MESOS-320)
This addresses bug MESOS-324.
https://issues.apache.org/jira/browse/MESOS-324
Diffs (updated)
-----
third_party/libprocess/include/process/statistics.hpp 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9
third_party/libprocess/src/process.cpp 72e437862ee0b35126c16d32bec79ef76a4e2b23
third_party/libprocess/src/statistics.cpp 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f
Diff: https://reviews.apache.org/r/9090/diff/
Testing
-------
make check
Thanks,
Ben Mahler