----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69563/#review211323 -----------------------------------------------------------
Could you please add a test, and break out refactorings and unrelated cleanups into their own patch? src/master/master.cpp Lines 4757-4758 (original), 4757-4758 (patched) <https://reviews.apache.org/r/69563/#comment296306> Performing this change in this patch makes it hard to review the important change. It should be in a separate patch. I personally am not sure we want to make this change as the reference semantics it introduces make it harder to verify that we e.g., do not access invalid references. This might not be worth the performance improvement this patch might bring. src/master/master.cpp Lines 5513-5519 (patched) <https://reviews.apache.org/r/69563/#comment296307> This patch now first authorizes and then validates. Is this the accepted pattern? In any case, this seems to be a change which would warrent its own patch. src/master/master.cpp Lines 5667 (patched) <https://reviews.apache.org/r/69563/#comment296308> This change and the one below seem in `DESTROY_DISK` seem to be the central pieces to address MESOS-9474. src/master/master.cpp Lines 5805 (patched) <https://reviews.apache.org/r/69563/#comment296309> We should log some descriptive message here. - Benjamin Bannier On Dec. 14, 2018, 2:55 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69563/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2018, 2:55 a.m.) > > > Review request for mesos, Benjamin Bannier, Greg Mann, James DeFelice, and > Jan Schlicht. > > > Bugs: MESOS-9474 > https://issues.apache.org/jira/browse/MESOS-9474 > > > Repository: mesos > > > Description > ------- > > This patch fixes a bug where the master does not complete the > processing of authorization results for `CREATE_DISK`, `DESTROY_DISK` > and `LAUNCH_GROUP`, causing other operations to drop if a remaining > authorization is denied.. > > > Diffs > ----- > > src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c > > > Diff: https://reviews.apache.org/r/69563/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
