mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 69563: Made master process authorization results properly for offer operations.
Date Fri, 14 Dec 2018 08:58:42 GMT

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


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