----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72098/#review219620 -----------------------------------------------------------
Fix it, then Ship it! src/master/master.cpp Line 4290 (original), 4292 (patched) <https://reviews.apache.org/r/72098/#comment307816> Maybe a bit of context for the reader about why `_accept` exists? (given that the reader won't know about the previous state of the code) src/master/master.cpp Lines 4497-4499 (patched) <https://reviews.apache.org/r/72098/#comment307817> is it possible to not wrap it here and use `authorized_` directly instead? src/master/master.cpp Lines 4570-4572 (patched) <https://reviews.apache.org/r/72098/#comment307818> Feel free to make the logging consistent and add the agent logging in all of these. src/master/master.cpp Lines 5320-5322 (original), 5085-5087 (patched) <https://reviews.apache.org/r/72098/#comment307819> This comment still refers to authz futures? src/master/master.cpp Lines 5335-5338 (original), 5099-5102 (patched) <https://reviews.apache.org/r/72098/#comment307820> Looks like we don't need this anymore? src/master/master.cpp Lines 5278-5280 (patched) <https://reviews.apache.org/r/72098/#comment307821> different wrapping here than the others? src/master/master.cpp Lines 5321-5324 (patched) <https://reviews.apache.org/r/72098/#comment307822> Ditto here - Benjamin Mahler On Feb. 13, 2020, 4:42 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72098/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2020, 4:42 p.m.) > > > Review request for mesos, Benjamin Mahler and Greg Mann. > > > Bugs: MESOS-10023, MESOS-10056 and MESOS-10083 > https://issues.apache.org/jira/browse/MESOS-10023 > https://issues.apache.org/jira/browse/MESOS-10056 > https://issues.apache.org/jira/browse/MESOS-10083 > > > Repository: mesos > > > Description > ------- > > This patch converts ACCEPT call to synchronous authorization > (see MESOS-10056), thus fixing race between ACCEPT and REVIVE > (MESOS-10023) and removing potential for other similar races. > > It also moves authorization of scheduler API operations after their > validation (thus fixing MESOS-10083) and effectively gets rid of the > concept of a "task pending authorization". > > Tests are converted from mocking `Authorizer::authorized(...)` > to mocking `Authorizer::provideObjectApprover(...)` as necessary. > > > Diffs > ----- > > src/master/authorization.cpp 77719eb079b2a19e0841573f639437aa3bb0fe54 > src/master/framework.cpp a9318a9d33122610960e01a184b568a8ea18b514 > src/master/master.hpp f1aa40fb45c693bd992b50cffca11020a1fe4433 > src/master/master.cpp 6d45c4e56432cb997769f7c6d0c8f71bdc8f8005 > src/tests/master_authorization_tests.cpp > bc8155b97c9078eaa151cc4a3e5bc6ea0d7ac9fa > src/tests/master_tests.cpp 9688f5f0266f7c7142b54d488f2c13b427e542c0 > src/tests/reconciliation_tests.cpp cdff370c5871ded0cb10b8b782bd669e092eb741 > > > Diff: https://reviews.apache.org/r/72098/diff/4/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
