mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 72098: Converted ACCEPT to synchronous authorization.
Date Thu, 20 Feb 2020 15:59:35 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message