mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 64618: Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.
Date Tue, 30 Jan 2018 19:18:48 GMT

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




src/master/master.cpp
Lines 5892-5893 (patched)
<https://reviews.apache.org/r/64618/#comment276212>

    Since the UUID is associated with the status, what do you think about the following wording:
    
    "Processing ACKNOWLEDGE_OPERATION_STATUS call for status <uuid> of operation '<operation_id>'
etc...."



src/master/master.cpp
Lines 5898-5906 (patched)
<https://reviews.apache.org/r/64618/#comment276214>

    Hmm... will the operation's list of statuses ever be empty when a valid ACK is received?
    
    Also, do we need to update the latest status here? We already do it upon receipt of the
status update, via `updateOperation()`.
    
    I think the correct logic here might be something like:
    ```
    if (protobuf::isTerminalState(
            (*operation->statuses().rbegin()).state())) {
      removeOperation(operation);
    }
    ```



src/master/master.cpp
Lines 5903 (patched)
<https://reviews.apache.org/r/64618/#comment276215>

    Rather than just conditionally updating master state if the status UUID is correct, should
we validate that the UUID in the call is correct, and drop the message if not? Seems strange
to send an ACK containing an invalid status UUID to the agent.


- Greg Mann


On Jan. 26, 2018, 10:09 p.m., Gaston Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64618/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2018, 10:09 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
>     https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
>   src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7 
> 
> 
> Diff: https://reviews.apache.org/r/64618/diff/3/
> 
> 
> Testing
> -------
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>


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