You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Charlie Carson <cc...@twitter.com> on 2014/01/23 20:09:05 UTC
Review Request 17255: Add observe endpoint to master.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Jeff Currier.
Bugs: https://issues.apache.org/jira/browse/MESOS-880
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
Repository: mesos-git
Description
-------
Add observe endpoint to master.
This changes adds a new HTTP endpoint of observe to master.
This allows clients to report health via an HTTP POST.
Values are:
MONITOR = Monitor for which health is being reported.
HOSTS = Comma seperated list of hosts.
LEVEL = OK for healthy, anything else for unhealthy.
This also contains a small fix to alphabetize the existing
endpoints / help strings.
SEE: https://issues.apache.org/jira/browse/MESOS-880
Review: https://reviews.apache.org/r/17255
Diffs
-----
src/master/http.cpp 546e91dbb9c8ee1014bb4f0b3be2714ad6a2d520
src/master/master.hpp 99b81811d596a777ee83dce7b157c1b11c064fab
src/master/master.cpp c7d9186b9c22fb40b05aa6d5bc6f2d38fb7f73ea
Diff: https://reviews.apache.org/r/17255/diff/
Testing
-------
make check
manually testing - I'll add unit tests when the repair coordinator is introduced in the next checkin.
Thanks,
Charlie Carson
Re: Review Request 17255: Add observe endpoint to master.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 363
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line363>
> >
> > Do you want to check for 'jsonp'?
>
> Charlie Carson wrote:
> I'm not opposed, but not 100% it's useful either. jsonp is typically so that you can invoke a local call back when the response comes back, right? ex:
>
> <script type="application/javascript"
> src="http://server2.example.com/Users/1234?jsonp=parseResponse">
> </script>
>
> would invoke parseResponse w/ the result of the get to the resource.
>
> the reason i'm not sure it makes sense is that this upload is a post, which you can't do with a script tag.
>
> OTOH, I might be ignorant of some other valid use cases. dropping for now - happy to reopen :-)
Makes sense to ignore for a POST!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review33087
-----------------------------------------------------------
On Jan. 30, 2014, 10:13 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:13 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
> src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
> src/master/master.cpp 77872ece66601043ddeb280cceba5a7676e8a6be
> src/tests/health_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new health_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 297
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line297>
> >
> > You guys have likely already discussed this, but 'monitor' or 'metric'?
1) snapping to koalabird, just so we don't need to change something there and/or create a translation layer
2) to me, monitor implies something like "Disk space is low" vs. metric implies "Available disk space". I think we want the former, b/c that allows the evaluation to be distributed (either by the external alerting system or on the host itself) rather than force it to take place on the host. That's especially good here, b/c a given host / service might have very different ideas of how much available disk space is enough. I DEFINITELY don't think we want master having knowledge of different classes of clients and what constitutes "good" or "bad" for a given metric / host.
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 299
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line299>
> >
> > Likewise, 'level' makes it sound like there will be a spectrum of health. Is that planned for the future? Will it be a spectrum or more like a collection? If so, does s/level/status/ make more sense? These "interface" questions are harder to change later. ;)
again, koalabird terminology, and we can get either OK, WARN, or CRITICAL.
personally, I would definitely prefer normalized degrees of failure, for things like disk space or memory free. In my perfect world this would be a 0-N metric where 0 is healthy and the bigger the number the more unhealthy something is. it would be up to the host / external system to map "2GB free" to 0, 1, or 2 in terms of health. this would let you prioritize repairs without having to do the evaluation. OTOH, for the type of repairs we are expecting to do (hardware failure basically), it is pretty likely that binary will suffice (Jeff's experience).
I guess I'm trying to balance 3 different things:
1) snap to koalabird since that's what we need
2) keep it open ended, i.e. even if we thought it was binary, I would take 0 or 1, not true or false, so that we allow degrees in the future if need be
3) internally, be strongly typed, b/c we can change that going forward.
but agreed there is lots of room for debate here.
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 308
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line308>
> >
> > Please wrap lines longer than 80 characters. In this case, please wrap after the '=', indent by 2 on the newline.
fixed and I added this to my .vimrc so that it won't keep happening:
augroup vimrc_autocmds
autocmd BufEnter * highlight ExtraWhitespace ctermbg=red guibg=red
autocmd BufEnter * call matchadd('ExtraWhitespace', '\s\+$', 11)
autocmd BufEnter * highlight OverLength ctermbg=red guibg=red
autocmd BufEnter * call matchadd('OverLength', '\%>80v.\+')
augroup END
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 350
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line350>
> >
> > Did you need this explicit assignment?
no, I've been coding in Java for too long :-)
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 363
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line363>
> >
> > Do you want to check for 'jsonp'?
I'm not opposed, but not 100% it's useful either. jsonp is typically so that you can invoke a local call back when the response comes back, right? ex:
<script type="application/javascript"
src="http://server2.example.com/Users/1234?jsonp=parseResponse">
</script>
would invoke parseResponse w/ the result of the get to the resource.
the reason i'm not sure it makes sense is that this upload is a post, which you can't do with a script tag.
OTOH, I might be ignorant of some other valid use cases. dropping for now - happy to reopen :-)
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 361
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line361>
> >
> > Yeah, a JSON::Boolean would be great!
there is another CR out to fix it :-)
> On Jan. 29, 2014, 3:26 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 365
> > <https://reviews.apache.org/r/17255/diff/1/?file=436254#file436254line365>
> >
> > Do you want to keep this in post testing? Assuming you don't include this, I think you can take two tacks for dealing with missing values here: (1) show all missing values and (2) show a missing value when you encounter it. In the latter case I'd prefer to see code above which explicitly checks for each key and returns BadRequest("Missing '" + KEY + "'.") when it is missing something. In addition, the key/value might be present but the value isn't decodable (http::decode fails) or is empty, both of which could be presented immediately as errors. It's not clear to me that there is that much benefit in determining all the ways in which the endpoint was misused (i.e., all the errors) rather than just returning after you encounter the first error.
I'll remove this and simplify.
- Charlie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review33087
-----------------------------------------------------------
On Jan. 30, 2014, 10:13 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:13 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
> src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
> src/master/master.cpp 77872ece66601043ddeb280cceba5a7676e8a6be
> src/tests/health_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new health_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review33087
-----------------------------------------------------------
Stoked to see a HELP included from the very beginning! In addition to the comments I made below it would be great to see a simple test. We've got this great little utility in libprocess called 'http::post' which should help you. ;)
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62340>
You guys have likely already discussed this, but 'monitor' or 'metric'?
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62341>
Likewise, 'level' makes it sound like there will be a spectrum of health. Is that planned for the future? Will it be a spectrum or more like a collection? If so, does s/level/status/ make more sense? These "interface" questions are harder to change later. ;)
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62342>
Please wrap lines longer than 80 characters. In this case, please wrap after the '=', indent by 2 on the newline.
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62343>
s/string &/string& /g (here and elsewhere please)
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62344>
s/http/HTTP/
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62345>
s/if(/if (/
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62346>
s/json/JSON/g (although, it looks like you capitalized it everywhere else)
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62347>
FYI, I prefer going through each key explicitly down here versus the loop of keys above.
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62348>
Did you need this explicit assignment?
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62351>
What about grouping each 'response.values[KEY]' block? Something like:
JSON::Object response;
// Add 'monitor'.
response.values[MONITOR_KEY] = values[MONITOR_KEY];
// Add 'hosts'.
vector<string> hosts = strings::split(values[HOSTS_KEY], ",");
JSON::Array array;
array.values.assign(hosts.begin(), hosts.end());
response.values[HOSTS_KEY] = array;
// Add 'level'.
bool isHealthy = strings::upper(values[LEVEL_KEY]) == "OK";
// TODO(ccarson): This is a workaround ...
response.values[LEVEL_KEY] =
(isHealthy ? JSON::Value(JSON::True()) : JSON::False());
return OK(response ...
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62352>
s/workaroudn/workaround/
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62349>
Yeah, a JSON::Boolean would be great!
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62350>
Do you want to check for 'jsonp'?
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment62353>
Do you want to keep this in post testing? Assuming you don't include this, I think you can take two tacks for dealing with missing values here: (1) show all missing values and (2) show a missing value when you encounter it. In the latter case I'd prefer to see code above which explicitly checks for each key and returns BadRequest("Missing '" + KEY + "'.") when it is missing something. In addition, the key/value might be present but the value isn't decodable (http::decode fails) or is empty, both of which could be presented immediately as errors. It's not clear to me that there is that much benefit in determining all the ways in which the endpoint was misused (i.e., all the errors) rather than just returning after you encounter the first error.
- Benjamin Hindman
On Jan. 23, 2014, 7:09 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2014, 7:09 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/master/http.cpp 546e91dbb9c8ee1014bb4f0b3be2714ad6a2d520
> src/master/master.hpp 99b81811d596a777ee83dce7b157c1b11c064fab
> src/master/master.cpp c7d9186b9c22fb40b05aa6d5bc6f2d38fb7f73ea
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> manually testing - I'll add unit tests when the repair coordinator is introduced in the next checkin.
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
> On Feb. 10, 2014, 7:10 a.m., Benjamin Hindman wrote:
> > src/Makefile.am, line 848
> > <https://reviews.apache.org/r/17255/diff/2/?file=456965#file456965line848>
> >
> > Maybe s/health_tests/observe_tests/ ... although I assume you want to add more tests beyond just the observe endpoint? Since we have a 'health' endpoint there is a little ambiguity here.
renamed to repair_tests since the rest of the tests will be on the RepairCoordinator
- Charlie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review34049
-----------------------------------------------------------
On Jan. 30, 2014, 10:13 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:13 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
> src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
> src/master/master.cpp 77872ece66601043ddeb280cceba5a7676e8a6be
> src/tests/health_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new health_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review34049
-----------------------------------------------------------
Ship it!
src/Makefile.am
<https://reviews.apache.org/r/17255/#comment64043>
Maybe s/health_tests/observe_tests/ ... although I assume you want to add more tests beyond just the observe endpoint? Since we have a 'health' endpoint there is a little ambiguity here.
src/tests/health_tests.cpp
<https://reviews.apache.org/r/17255/#comment64041>
{ on newline please.
src/tests/health_tests.cpp
<https://reviews.apache.org/r/17255/#comment64042>
{ on newline please. Also, const& on JsonResponse?
src/tests/health_tests.cpp
<https://reviews.apache.org/r/17255/#comment64044>
These macros look great!
- Benjamin Hindman
On Jan. 30, 2014, 10:13 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:13 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
> src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
> src/master/master.cpp 77872ece66601043ddeb280cceba5a7676e8a6be
> src/tests/health_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new health_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
> On March 20, 2014, 6:45 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 551
> > <https://reviews.apache.org/r/17255/diff/4/?file=484418#file484418line551>
> >
> > Could you follow up with a fix for this? It should be bound to Http::stats, not roles.
opened https://issues.apache.org/jira/browse/MESOS-1129 to track this since the review is closed and code is in
- Charlie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review37941
-----------------------------------------------------------
On Feb. 13, 2014, 6:55 p.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2014, 6:55 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am aa8bb2b492e4d37b14db294f48c074e631143b68
> src/master/http.cpp 966eed6d8340038265ef799f1b6149502ccc606e
> src/master/master.hpp 737bd8b5a1cd88a98a7f094795c50547079921ba
> src/master/master.cpp a4e1b1f7327f01eb16f14c3fa08fc7a62048d36c
> src/tests/repair_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new repair_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review37941
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/17255/#comment69756>
Could you follow up with a fix for this? It should be bound to Http::stats, not roles.
- Dominic Hamon
On Feb. 13, 2014, 10:55 a.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2014, 10:55 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am aa8bb2b492e4d37b14db294f48c074e631143b68
> src/master/http.cpp 966eed6d8340038265ef799f1b6149502ccc606e
> src/master/master.hpp 737bd8b5a1cd88a98a7f094795c50547079921ba
> src/master/master.cpp a4e1b1f7327f01eb16f14c3fa08fc7a62048d36c
> src/tests/repair_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new repair_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/
-----------------------------------------------------------
(Updated Feb. 13, 2014, 6:55 p.m.)
Review request for mesos, Benjamin Hindman and Jeff Currier.
Bugs: https://issues.apache.org/jira/browse/MESOS-880
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
Repository: mesos-git
Description
-------
Add observe endpoint to master.
This changes adds a new HTTP endpoint of observe to master.
This allows clients to report health via an HTTP POST.
Values are:
MONITOR = Monitor for which health is being reported.
HOSTS = Comma seperated list of hosts.
LEVEL = OK for healthy, anything else for unhealthy.
This also contains a small fix to alphabetize the existing
endpoints / help strings.
SEE: https://issues.apache.org/jira/browse/MESOS-880
Review: https://reviews.apache.org/r/17255
Diffs (updated)
-----
src/Makefile.am aa8bb2b492e4d37b14db294f48c074e631143b68
src/master/http.cpp 966eed6d8340038265ef799f1b6149502ccc606e
src/master/master.hpp 737bd8b5a1cd88a98a7f094795c50547079921ba
src/master/master.cpp a4e1b1f7327f01eb16f14c3fa08fc7a62048d36c
src/tests/repair_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/17255/diff/
Testing
-------
make check
added unit tests in a new repair_test.cpp file
Thanks,
Charlie Carson
Re: Review Request 17255: Add observe endpoint to master.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/#review34161
-----------------------------------------------------------
Ship it!
Nice test.
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment64173>
We always use braces for if/for/while loops.
if () {
}
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment64174>
ditto.
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment64175>
ditto.
src/master/http.cpp
<https://reviews.apache.org/r/17255/#comment64176>
s/either/of/ ?
src/tests/repair_tests.cpp
<https://reviews.apache.org/r/17255/#comment64177>
s/r/response/
src/tests/repair_tests.cpp
<https://reviews.apache.org/r/17255/#comment64178>
s/o/object/
src/tests/repair_tests.cpp
<https://reviews.apache.org/r/17255/#comment64179>
How about using a ternary operator for conciseness?
src/tests/repair_tests.cpp
<https://reviews.apache.org/r/17255/#comment64180>
s/!ok/level != OK/ ?
- Vinod Kone
On Feb. 11, 2014, 12:10 a.m., Charlie Carson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17255/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2014, 12:10 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Jeff Currier.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add observe endpoint to master.
>
> This changes adds a new HTTP endpoint of observe to master.
> This allows clients to report health via an HTTP POST.
> Values are:
> MONITOR = Monitor for which health is being reported.
> HOSTS = Comma seperated list of hosts.
> LEVEL = OK for healthy, anything else for unhealthy.
>
> This also contains a small fix to alphabetize the existing
> endpoints / help strings.
>
> SEE: https://issues.apache.org/jira/browse/MESOS-880
>
> Review: https://reviews.apache.org/r/17255
>
>
> Diffs
> -----
>
> src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
> src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
> src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
> src/master/master.cpp 241b0069902cd125fdd933096ddd7e1c841d1559
> src/tests/repair_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17255/diff/
>
>
> Testing
> -------
>
> make check
> added unit tests in a new repair_test.cpp file
>
>
> Thanks,
>
> Charlie Carson
>
>
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/
-----------------------------------------------------------
(Updated Feb. 11, 2014, 12:10 a.m.)
Review request for mesos, Benjamin Hindman and Jeff Currier.
Bugs: https://issues.apache.org/jira/browse/MESOS-880
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
Repository: mesos-git
Description
-------
Add observe endpoint to master.
This changes adds a new HTTP endpoint of observe to master.
This allows clients to report health via an HTTP POST.
Values are:
MONITOR = Monitor for which health is being reported.
HOSTS = Comma seperated list of hosts.
LEVEL = OK for healthy, anything else for unhealthy.
This also contains a small fix to alphabetize the existing
endpoints / help strings.
SEE: https://issues.apache.org/jira/browse/MESOS-880
Review: https://reviews.apache.org/r/17255
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
src/master/master.cpp 241b0069902cd125fdd933096ddd7e1c841d1559
src/tests/repair_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/17255/diff/
Testing (updated)
-------
make check
added unit tests in a new repair_test.cpp file
Thanks,
Charlie Carson
Re: Review Request 17255: Add observe endpoint to master.
Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17255/
-----------------------------------------------------------
(Updated Jan. 30, 2014, 10:13 p.m.)
Review request for mesos, Benjamin Hindman and Jeff Currier.
Bugs: https://issues.apache.org/jira/browse/MESOS-880
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
Repository: mesos-git
Description
-------
Add observe endpoint to master.
This changes adds a new HTTP endpoint of observe to master.
This allows clients to report health via an HTTP POST.
Values are:
MONITOR = Monitor for which health is being reported.
HOSTS = Comma seperated list of hosts.
LEVEL = OK for healthy, anything else for unhealthy.
This also contains a small fix to alphabetize the existing
endpoints / help strings.
SEE: https://issues.apache.org/jira/browse/MESOS-880
Review: https://reviews.apache.org/r/17255
Diffs (updated)
-----
src/Makefile.am c30706846bca1fa3287291e39f46a23713ad1ba4
src/master/http.cpp fb15483953a593bfec4e60884219dc8c4e8d565c
src/master/master.hpp 7649737283f5f5d786ac40504e943a3be4c1d62b
src/master/master.cpp 77872ece66601043ddeb280cceba5a7676e8a6be
src/tests/health_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/17255/diff/
Testing (updated)
-------
make check
added unit tests in a new health_test.cpp file
Thanks,
Charlie Carson