You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2017/11/29 17:54:03 UTC

Review Request 64146: Added offer operation update acknowledgement to the agent.

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

Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.


Bugs: MESOS-8186
    https://issues.apache.org/jira/browse/MESOS-8186


Repository: mesos


Description
-------

The agent's offerOperationUpdateAcknowlegement handler is
updated to pass acknowledgements to the resource provider
manager.


Diffs
-----

  src/slave/slave.cpp cd716476dfad619dc6cf7449a32a27842efbe7fb 


Diff: https://reviews.apache.org/r/64146/diff/1/


Testing
-------

make check


Thanks,

Greg Mann


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/
-----------------------------------------------------------

(Updated Dec. 7, 2017, 3:11 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.


Bugs: MESOS-8186
    https://issues.apache.org/jira/browse/MESOS-8186


Repository: mesos


Description
-------

The agent's offerOperationUpdateAcknowlegement handler is
updated to pass acknowledgements to the resource provider
manager.

The agent's resource provider message handler is also
updated to avoid removing offer operations, since this
should actually be done upon acknowledgement of the update.


Diffs (updated)
-----

  src/slave/slave.cpp dd830d93f8a08720231e3d4a6421f7a058ba6093 


Diff: https://reviews.apache.org/r/64146/diff/4/

Changes: https://reviews.apache.org/r/64146/diff/3-4/


Testing
-------

make check


Thanks,

Greg Mann


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/#review192893
-----------------------------------------------------------


Fix it, then Ship it!





src/slave/slave.cpp
Lines 3860 (patched)
<https://reviews.apache.org/r/64146/#comment271237>

    `operation != nullptr`


- Jie Yu


On Dec. 5, 2017, 7:07 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64146/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 7:07 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8186
>     https://issues.apache.org/jira/browse/MESOS-8186
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The agent's offerOperationUpdateAcknowlegement handler is
> updated to pass acknowledgements to the resource provider
> manager.
> 
> The agent's resource provider message handler is also
> updated to avoid removing offer operations, since this
> should actually be done upon acknowledgement of the update.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp c07e25f668bef6b6fa3cae4b69ba90de3eb3bdcc 
> 
> 
> Diff: https://reviews.apache.org/r/64146/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/
-----------------------------------------------------------

(Updated Dec. 5, 2017, 7:07 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.


Bugs: MESOS-8186
    https://issues.apache.org/jira/browse/MESOS-8186


Repository: mesos


Description
-------

The agent's offerOperationUpdateAcknowlegement handler is
updated to pass acknowledgements to the resource provider
manager.

The agent's resource provider message handler is also
updated to avoid removing offer operations, since this
should actually be done upon acknowledgement of the update.


Diffs (updated)
-----

  src/slave/slave.cpp c07e25f668bef6b6fa3cae4b69ba90de3eb3bdcc 


Diff: https://reviews.apache.org/r/64146/diff/3/

Changes: https://reviews.apache.org/r/64146/diff/2-3/


Testing
-------

make check


Thanks,

Greg Mann


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/
-----------------------------------------------------------

(Updated Dec. 4, 2017, 10:24 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.


Bugs: MESOS-8186
    https://issues.apache.org/jira/browse/MESOS-8186


Repository: mesos


Description
-------

The agent's offerOperationUpdateAcknowlegement handler is
updated to pass acknowledgements to the resource provider
manager.

The agent's resource provider message handler is also
updated to avoid removing offer operations, since this
should actually be done upon acknowledgement of the update.


Diffs (updated)
-----

  src/slave/slave.cpp c07e25f668bef6b6fa3cae4b69ba90de3eb3bdcc 


Diff: https://reviews.apache.org/r/64146/diff/2/

Changes: https://reviews.apache.org/r/64146/diff/1-2/


Testing
-------

make check


Thanks,

Greg Mann


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Nov. 30, 2017, 11:33 p.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 3851 (patched)
> > <https://reviews.apache.org/r/64146/diff/1/?file=1903841#file1903841line3852>
> >
> >     Currently, for agent default resources, we call `removeOfferOperation` right after the operation is processed (given that we don't have status update manager or checkpointing for operations), and send best effort status update to the master.
> >     
> >     I am wondering if we need ack at all for agent default resources operations. That mean, we should probably remove the offer operation for agent default resources in `offerOperationStatusUpdate`.
> >     
> >     If there is a fail over, and the update is lost, the agent will reconcile with the master during re-registration and master will notice that and call removeOfferOperation there (in updateSlave handler).

The current master code will only acknowledge updates which have the `status_uuid` field set (updates which are being retried), and since the agent doesn't set that field for its best-effort updates, I don't think we need to worry about agent default resources in this handler right now.


- Greg


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


On Nov. 29, 2017, 5:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64146/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2017, 5:56 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8186
>     https://issues.apache.org/jira/browse/MESOS-8186
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The agent's offerOperationUpdateAcknowlegement handler is
> updated to pass acknowledgements to the resource provider
> manager.
> 
> The agent's resource provider message handler is also
> updated to avoid removing offer operations, since this
> should actually be done upon acknowledgement of the update.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp cd716476dfad619dc6cf7449a32a27842efbe7fb 
> 
> 
> Diff: https://reviews.apache.org/r/64146/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/#review192391
-----------------------------------------------------------




src/slave/slave.cpp
Lines 3850 (patched)
<https://reviews.apache.org/r/64146/#comment270503>

    Ditto. I don't think we can remove the operation based on `latest_status`.
    
    Also, I would comment here saying that: If the ack is dropped (RP disconnected), during RP re-registration, an UPDATE_STATE will be triggered to tell the agent about the removed operation, and agent will add it back.



src/slave/slave.cpp
Lines 3851 (patched)
<https://reviews.apache.org/r/64146/#comment270507>

    Currently, for agent default resources, we call `removeOfferOperation` right after the operation is processed (given that we don't have status update manager or checkpointing for operations), and send best effort status update to the master.
    
    I am wondering if we need ack at all for agent default resources operations. That mean, we should probably remove the offer operation for agent default resources in `offerOperationStatusUpdate`.
    
    If there is a fail over, and the update is lost, the agent will reconcile with the master during re-registration and master will notice that and call removeOfferOperation there (in updateSlave handler).


- Jie Yu


On Nov. 29, 2017, 5:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64146/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2017, 5:56 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8186
>     https://issues.apache.org/jira/browse/MESOS-8186
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The agent's offerOperationUpdateAcknowlegement handler is
> updated to pass acknowledgements to the resource provider
> manager.
> 
> The agent's resource provider message handler is also
> updated to avoid removing offer operations, since this
> should actually be done upon acknowledgement of the update.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp cd716476dfad619dc6cf7449a32a27842efbe7fb 
> 
> 
> Diff: https://reviews.apache.org/r/64146/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 64146: Added offer operation update acknowledgement to the agent.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64146/
-----------------------------------------------------------

(Updated Nov. 29, 2017, 5:56 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Jie Yu.


Bugs: MESOS-8186
    https://issues.apache.org/jira/browse/MESOS-8186


Repository: mesos


Description (updated)
-------

The agent's offerOperationUpdateAcknowlegement handler is
updated to pass acknowledgements to the resource provider
manager.

The agent's resource provider message handler is also
updated to avoid removing offer operations, since this
should actually be done upon acknowledgement of the update.


Diffs
-----

  src/slave/slave.cpp cd716476dfad619dc6cf7449a32a27842efbe7fb 


Diff: https://reviews.apache.org/r/64146/diff/1/


Testing
-------

make check


Thanks,

Greg Mann