----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72019/#review219389 -----------------------------------------------------------
Fix it, then Ship it! src/master/master.hpp Lines 1306 (patched) <https://reviews.apache.org/r/72019/#comment307559> Hmm... to me, "caching of responses" sounds misleading here. Yes, `ReadOnlyHandler` is caching - in a sense - but it is not caching what already exists, it is *caching the future*. Given that you are touching this comment now, probably it is time to replace this phrase with something more precise. I would suggest s/caching of responses/deduplicating requests/; if you know a better word, feel free to use it;) src/master/master.hpp Lines 1308-1311 (original), 1327-1333 (patched) <https://reviews.apache.org/r/72019/#comment307558> Makes me wonder whether it is possible, instead of using `<Option>` to simply add `Nothing` to the variant, like ``` struct PostProcessing { ... Variant<Nothing, Subscribe> state; }; ... pair<Response, Master::ReadOnlyHandler::PostProcessing> Master::ReadOnlyHandler::frameworks(...) { ... return {OK(jsonify(frameworks), query.get("jsonp")), {Nothing()}}; } ``` If there are reasons to have this variant wrapped into `Option`, I think they deserve a comment in the code. - Andrei Sekretenko On Jan. 21, 2020, 7:26 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72019/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2020, 7:26 p.m.) > > > Review request for mesos, Andrei Sekretenko and Greg Mann. > > > Bugs: MESOS-9497 > https://issues.apache.org/jira/browse/MESOS-9497 > > > Repository: mesos > > > Description > ------- > > This call is not entirely read-only, unlike the other GET_* v1 master > calls, and therefore it warranted its own patch. > > The approach used is to add a post-processing "write" step to the > handler return type. The post-processing step gets executed > synchronously. In order to deal with different potential post- > processing steps, we use a Variant. > > Note that SUBSCRIBE cannot asynchronously register the subscriber > after the read-only state is served, because it will miss events > in the interim! > > > Diffs > ----- > > src/common/http.hpp 47a4d6a1ad4897155448a6ba64e789b15a78c7a2 > src/master/http.cpp 8a588635e688eb52cd7b8320426dc412e7b44e18 > src/master/master.hpp 3074918d677430b588c7765f5ed82f4e324eeff4 > src/master/readonly_handler.cpp fbe748d99c2520b520f56afa50dc0b9bd809778d > src/tests/master_load_tests.cpp 6cee2488413b6a4f9a69092a8b06cf6eb79f360b > > > Diff: https://reviews.apache.org/r/72019/diff/2/ > > > Testing > ------- > > > Thanks, > > Benjamin Mahler > >
