mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@apache.org>
Subject Re: Review Request 64011: Updated master behaviour to update agent state on reregistration.
Date Thu, 23 Nov 2017 00:52:43 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64011/#review191767
-----------------------------------------------------------




src/master/master.hpp
Lines 1105 (patched)
<https://reviews.apache.org/r/64011/#comment269647>

    s/agentCapabilities/capabilities/



src/master/master.cpp
Lines 6320 (patched)
<https://reviews.apache.org/r/64011/#comment269649>

    s/reregistration/re-registration/



src/master/master.cpp
Lines 6321 (patched)
<https://reviews.apache.org/r/64011/#comment269648>

    s/. (/(/
    s/)/)./
    s/state/SlaveInfo/



src/master/master.cpp
Lines 6323 (patched)
<https://reviews.apache.org/r/64011/#comment269650>

    Can you file a JIRA for this?



src/master/master.cpp
Line 6478 (original), 6484 (patched)
<https://reviews.apache.org/r/64011/#comment269655>

    Can you make sure to reject re-registration here if the slave is in `markingGone` or `gone`
maps.



src/master/master.cpp
Lines 6489-6490 (patched)
<https://reviews.apache.org/r/64011/#comment269651>

    s/in cases 1 and 2//



src/master/master.cpp
Line 6489 (original), 6501 (patched)
<https://reviews.apache.org/r/64011/#comment269652>

    log the pid and hostname.



src/master/master.cpp
Line 6610 (original), 6543 (patched)
<https://reviews.apache.org/r/64011/#comment269653>

    log the pid and hostname.



src/master/master.cpp
Lines 6626-6627 (original), 6559-6560 (patched)
<https://reviews.apache.org/r/64011/#comment269654>

    Remove the first sentence. 
    
    s/In the common case,/In the common case, we are here if/



src/master/master.cpp
Line 6659 (original), 6594 (patched)
<https://reviews.apache.org/r/64011/#comment269656>

    s/success/future/



src/master/master.cpp
Line 6672 (original), 6609 (patched)
<https://reviews.apache.org/r/64011/#comment269657>

    Reject if agent is being marked gone or already marked gone. Can you do this and the previous
similar change in a separate review?



src/master/master.cpp
Lines 6847 (patched)
<https://reviews.apache.org/r/64011/#comment269658>

    Reject if slave is marking gone or marked gone.



src/master/master.cpp
Lines 6854 (patched)
<https://reviews.apache.org/r/64011/#comment269659>

    log the pid and hostname.



src/master/master.cpp
Lines 6861-6871 (patched)
<https://reviews.apache.org/r/64011/#comment269662>

    Change this to:
    ```
    CHECK_READY(updated);
    CHECK(updated.get());
    
    VLOG(1) << Registry updated for slave " < slaveInfo.id << " at " <<
pid << " (" slaveInfo.hostname() << ")";
    
    ```



src/master/master.cpp
Lines 6873 (patched)
<https://reviews.apache.org/r/64011/#comment269663>

    s/failoverSlave()/`failoverSlave()`/
    
    Can you expand a bit on the comment for future reference?



src/master/master.cpp
Lines 6901-6902 (patched)
<https://reviews.apache.org/r/64011/#comment269664>

    indentation?
    
    can you update the comment to not say anything about checkpoint resources. say instead
that you would recover the framework info if necessary.



src/master/master.cpp
Line 6943 (original), 6997 (patched)
<https://reviews.apache.org/r/64011/#comment269666>

    just inline it.



src/master/master.cpp
Line 11091 (original), 11146 (patched)
<https://reviews.apache.org/r/64011/#comment269667>

    2 blank lines.



src/master/master.cpp
Lines 11147 (patched)
<https://reviews.apache.org/r/64011/#comment269668>

    return `Try<Nothing>` here instead and reject re-registration if it's an error.



src/master/master.cpp
Lines 11170 (patched)
<https://reviews.apache.org/r/64011/#comment269669>

    s/   slave/slave/


- Vinod Kone


On Nov. 22, 2017, 11:31 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64011/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2017, 11:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When an agent reregisters, the master will now always update
> the agent information it holds in memory, and will write any
> changes back to the registry if necessary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 
>   src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f 
>   src/tests/master_tests.cpp 9c450b9f592d9e09a468f537d9b500e97acc636b 
> 
> 
> Diff: https://reviews.apache.org/r/64011/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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