> On May 4, 2018, 8:19 a.m., Jiang Yan Xu wrote: > > src/master/http.cpp > > Lines 4163-4165 (patched) > > <https://reviews.apache.org/r/66919/diff/1/?file=2016039#file2016039line4163> > > > > The following will be more idiomatic. > > > > ``` > > .onAny([](const Future<bool>& result) { > > CHECK_READY(result); > > }) > > ``` > > > > Note the spaces and const ref. > > > > > > It'll be helpful to get familiarize with > > > > https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md > > > > which in turn references > > > > https://google.github.io/styleguide/cppguide.html > > Jiang Yan Xu wrote: > It's also helpful to attach a message to the check failure: > > > ``` > CHECK_READY(result) > << "Failed to update maintenance schedule in the registry"; > ```
Also let's add a TODO above the `CHECK_READY`: ``` TODO(fiu): Consider changing/refactoring the registrar itself so the individual call sites don't need to handle this separately. All registrar failures that cause it to abort should instead abort the process. ``` - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66919/#review202416 ----------------------------------------------------------- On May 4, 2018, 12:46 p.m., Xudong Ni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66919/ > ----------------------------------------------------------- > > (Updated May 4, 2018, 12:46 p.m.) > > > Review request for mesos, Joseph Wu and Jiang Yan Xu. > > > Bugs: MESOS-8630 > https://issues.apache.org/jira/browse/MESOS-8630 > > > Repository: mesos > > > Description > ------- > > When the registrar fails to update the registry it would abort the > actor and fail all future operations,However when the registrar > updates is requested by an operator API such as maintenance update, > the master process doesn't shut down (a 500 error is returned to the > client instead)and all subsequent operations will fail. > > Review: https://reviews.apache.org/r/66919 > > > Diffs > ----- > > src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad > > > Diff: https://reviews.apache.org/r/66919/diff/1/ > > > Testing > ------- > > No error found by running all unit tests(make check) > > > Thanks, > > Xudong Ni > >
