> 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 > >
