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 2019/06/10 10:57:58 UTC

Review Request 70822: Made common protobuf changes for agent draining.

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 


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


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review216651
-----------------------------------------------------------




include/mesos/v1/mesos.proto
Lines 3773-3774 (patched)
<https://reviews.apache.org/r/70822/#comment303856>

    > This allows the operator to limit the maximum time it will take the agent to drain.
    
    Since this is a relative time the current implementation does not allow setting an upper bound on the time it will take an agent to drain, but instead the setting of an upper bound on how long one is willing to wait for tasks to terminate when draining.
    
    Consider the following scenario:
    
    * operator starts draining an agent
    * master persists the drain config and sends a request to the agent
    * the agent receives a drain request and persists the drain config
    * the agent fails over before it starts killing any task
    * the agent comes back up, e.g., after a duration > `max_grace_period`
    * it starts killing tasks
    * the agent will only finish draining after `2 * max_grace_period`
    
    If the agent fails over multiple times the duration could be longer.
    
    If we wanted a way for operators to specify a deadline by which an agent should be drained we would need to switch from times relative to the time the request is processed to some absolute timestamp (we could e.g., still accept such a `DrainConfig` from users, but internally translate to an absolute time by adding the period to the current time when processed on the master). This would have different semantics which would match the comment ("time until agent is drained" vs. "duration we are willing for tasks to terminate before taking drastic measures").
    
    Same issue in `mesos.proto`.


- Benjamin Bannier


On June 28, 2019, 9:51 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 28, 2019, 9:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto eb1b09cf9f9c7c102d713170538c2ba210edb351 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 33431777cbc730ddf4b1feb54662b54b8e302e46 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/7/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review216231
-----------------------------------------------------------


Ship it!




Ship It!

- Joseph Wu


On June 28, 2019, 12:51 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 28, 2019, 12:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto eb1b09cf9f9c7c102d713170538c2ba210edb351 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 33431777cbc730ddf4b1feb54662b54b8e302e46 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/7/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 28, 2019, 7:51 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-----

  include/mesos/mesos.proto eb1b09cf9f9c7c102d713170538c2ba210edb351 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto 33431777cbc730ddf4b1feb54662b54b8e302e46 
  src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


Diff: https://reviews.apache.org/r/70822/diff/7/

Changes: https://reviews.apache.org/r/70822/diff/6-7/


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On June 27, 2019, 6:10 a.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 3732-3737 (patched)
> > <https://reviews.apache.org/r/70822/diff/6/?file=2152177#file2152177line3732>
> >
> >     Let's document what happens if these fields are unset.

I have quite the blurb/comment in front of these same objects in the master API protos: https://reviews.apache.org/r/70911/
Feel free to reword those into this file where necessary.

I'll also want to re-order my fields, I guess.  The `max_grace_period` comes before the `mark_gone` in my protos.


- Joseph


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


On June 25, 2019, 12:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 25, 2019, 12:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d2750a634bf58428a152e70aedfc06ff70b3aa84 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 10dc29cd98b4c1fc1016755edeaf8cfae399bd78 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/6/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review216193
-----------------------------------------------------------




include/mesos/mesos.proto
Lines 3732-3737 (patched)
<https://reviews.apache.org/r/70822/#comment303269>

    Let's document what happens if these fields are unset.



include/mesos/v1/mesos.proto
Lines 3725-3730 (patched)
<https://reviews.apache.org/r/70822/#comment303270>

    Let's document what happens if these fields are unset.


- Benjamin Bannier


On June 25, 2019, 9:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 25, 2019, 9:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d2750a634bf58428a152e70aedfc06ff70b3aa84 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 10dc29cd98b4c1fc1016755edeaf8cfae399bd78 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/6/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review216185
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 3734 (patched)
<https://reviews.apache.org/r/70822/#comment303260>

    Let's renumber these.



include/mesos/v1/mesos.proto
Lines 3727 (patched)
<https://reviews.apache.org/r/70822/#comment303261>

    Let's renumber these.


- Benjamin Bannier


On June 25, 2019, 9:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 25, 2019, 9:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d2750a634bf58428a152e70aedfc06ff70b3aa84 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 10dc29cd98b4c1fc1016755edeaf8cfae399bd78 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/6/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 25, 2019, 7:56 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-----

  include/mesos/mesos.proto d2750a634bf58428a152e70aedfc06ff70b3aa84 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto 10dc29cd98b4c1fc1016755edeaf8cfae399bd78 
  src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


Diff: https://reviews.apache.org/r/70822/diff/6/

Changes: https://reviews.apache.org/r/70822/diff/5-6/


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 18, 2019, 11:17 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description (updated)
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


Diff: https://reviews.apache.org/r/70822/diff/5/


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 18, 2019, 11:17 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description (updated)
-------

Added common protobufs for agent draining.


Diffs (updated)
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


Diff: https://reviews.apache.org/r/70822/diff/5/

Changes: https://reviews.apache.org/r/70822/diff/4-5/


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 18, 2019, 10:08 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


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

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


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review215840
-----------------------------------------------------------




include/mesos/type_utils.hpp
Lines 226-230 (patched)
<https://reviews.apache.org/r/70822/#comment302775>

    What do you think about using  `google::protobuf::util::MessageDifferencer::Equals` here to remove potential for errors?



include/mesos/v1/mesos.proto
Lines 2623 (patched)
<https://reviews.apache.org/r/70822/#comment302776>

    Let's insert this after `REASON_AGENT_DISCONNECTED` to keep this somewhat sorted.


- Benjamin Bannier


On June 11, 2019, 7:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 7:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Vinod Kone <vi...@apache.org>.

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
>     Are you proposing having a separate message for inclusion in the agent API outputs?
> 
> Vinod Kone wrote:
>     IIUC, Benjamin is questioning why we are storing both the spec (max grace period, mark_gone) and the status (state) in the same proto (do we do this elsewhere in the code base?). If they were separate protos, say DrainRequest and DrainStatus, master would send DrainRequest to the agent, and show DrainStatus (and possibly DrainRequest) in the operator API response. It looks a bit weird that we would send DrainInfo (as it is currently written  to the agent and do a CHECK on valid state. The fact that an agent got a Drain message is enough for the agent to know that it needs to drain, it doesn't need to look into the `DrainInfo::State`?

Let me repharse my first sentence. I think storing both spec and status in the same proto is probably fine if it makes thing easy to expose in API endpoints and use it internally in the master. But I still think it's weird to send the status in a message to the agent which is not expected to use it. So, if there's a way we can avoid it that would be great.


- Vinod


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


On June 18, 2019, 11:17 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
>     Are you proposing having a separate message for inclusion in the agent API outputs?
> 
> Vinod Kone wrote:
>     IIUC, Benjamin is questioning why we are storing both the spec (max grace period, mark_gone) and the status (state) in the same proto (do we do this elsewhere in the code base?). If they were separate protos, say DrainRequest and DrainStatus, master would send DrainRequest to the agent, and show DrainStatus (and possibly DrainRequest) in the operator API response. It looks a bit weird that we would send DrainInfo (as it is currently written  to the agent and do a CHECK on valid state. The fact that an agent got a Drain message is enough for the agent to know that it needs to drain, it doesn't need to look into the `DrainInfo::State`?
> 
> Vinod Kone wrote:
>     Let me repharse my first sentence. I think storing both spec and status in the same proto is probably fine if it makes thing easy to expose in API endpoints and use it internally in the master. But I still think it's weird to send the status in a message to the agent which is not expected to use it. So, if there's a way we can avoid it that would be great.
> 
> Joseph Wu wrote:
>     From the master's perspective, having the `State` in the DrainInfo complicates what the master needs to do in the registry.  To report the correct state to any endpoints, the master will need to assume all agents are in a `DRAINING` state, until the agent connects, reconciles, and says it has nothing running.  This means the state within the registry will always be `DRAINING` (and there is no reason to transition that state).
>     
>     So moving the `State` field into a separate message would be ideal.

It definitely makes sense to me that the agent doesn't need to receive a drain state, so the `DrainSlaveMessage` shouldn't include that information. But we could just include the max grace period and the mark_gone bit in the `DrainSlaveMessage`, without moving the drain state outside of `DrainInfo`.

I think the question is, will we make use of the drain state outside of the `DrainInfo` message? I don't have any use cases in mind for that currently, but maybe I'm missing something? Or maybe it's best to have a separate `DrainState` enum in case we want to use it outside of `DrainInfo` in the future?


- Greg


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


On June 18, 2019, 11:17 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Vinod Kone <vi...@apache.org>.

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
>     Are you proposing having a separate message for inclusion in the agent API outputs?

IIUC, Benjamin is questioning why we are storing both the spec (max grace period, mark_gone) and the status (state) in the same proto (do we do this elsewhere in the code base?). If they were separate protos, say DrainRequest and DrainStatus, master would send DrainRequest to the agent, and show DrainStatus (and possibly DrainRequest) in the operator API response. It looks a bit weird that we would send DrainInfo (as it is currently written  to the agent and do a CHECK on valid state. The fact that an agent got a Drain message is enough for the agent to know that it needs to drain, it doesn't need to look into the `DrainInfo::State`?


- Vinod


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


On June 18, 2019, 11:17 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
>     Are you proposing having a separate message for inclusion in the agent API outputs?
> 
> Vinod Kone wrote:
>     IIUC, Benjamin is questioning why we are storing both the spec (max grace period, mark_gone) and the status (state) in the same proto (do we do this elsewhere in the code base?). If they were separate protos, say DrainRequest and DrainStatus, master would send DrainRequest to the agent, and show DrainStatus (and possibly DrainRequest) in the operator API response. It looks a bit weird that we would send DrainInfo (as it is currently written  to the agent and do a CHECK on valid state. The fact that an agent got a Drain message is enough for the agent to know that it needs to drain, it doesn't need to look into the `DrainInfo::State`?
> 
> Vinod Kone wrote:
>     Let me repharse my first sentence. I think storing both spec and status in the same proto is probably fine if it makes thing easy to expose in API endpoints and use it internally in the master. But I still think it's weird to send the status in a message to the agent which is not expected to use it. So, if there's a way we can avoid it that would be great.
> 
> Joseph Wu wrote:
>     From the master's perspective, having the `State` in the DrainInfo complicates what the master needs to do in the registry.  To report the correct state to any endpoints, the master will need to assume all agents are in a `DRAINING` state, until the agent connects, reconciles, and says it has nothing running.  This means the state within the registry will always be `DRAINING` (and there is no reason to transition that state).
>     
>     So moving the `State` field into a separate message would be ideal.
> 
> Greg Mann wrote:
>     It definitely makes sense to me that the agent doesn't need to receive a drain state, so the `DrainSlaveMessage` shouldn't include that information. But we could just include the max grace period and the mark_gone bit in the `DrainSlaveMessage`, without moving the drain state outside of `DrainInfo`.
>     
>     I think the question is, will we make use of the drain state outside of the `DrainInfo` message? I don't have any use cases in mind for that currently, but maybe I'm missing something? Or maybe it's best to have a separate `DrainState` enum in case we want to use it outside of `DrainInfo` in the future?

I split out the state into a top-level `DrainState` enum.


- Greg


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


On June 25, 2019, 7:56 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 25, 2019, 7:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d2750a634bf58428a152e70aedfc06ff70b3aa84 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto 10dc29cd98b4c1fc1016755edeaf8cfae399bd78 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/6/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.

Are you proposing having a separate message for inclusion in the agent API outputs?


- Greg


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


On June 18, 2019, 11:17 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.

> On June 14, 2019, 11:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?

The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?


- Benjamin


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


On June 11, 2019, 7:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 7:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?

What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.


- Greg


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


On June 11, 2019, 5:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 5:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.

> On June 14, 2019, 11:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.

Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.


- Benjamin


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


On June 19, 2019, 12:08 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 12:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/4/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On June 14, 2019, 2:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?
> 
> Greg Mann wrote:
>     Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.
>     
>     What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
>     The goal would be to use dedicated messages for triggering draining (master->agent) and to report draining (agent->master). That might not only be conceptually simplier, but would likely also lead to simpler, less redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
>     What do you mean by "report draining"? The design doesn't involve any explicit communication of draining progress between agent and master, the master just receives task status updates.
> 
> Benjamin Bannier wrote:
>     Sorry, there's no process of communicating `DrainInfo` back to master, but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
>     Are you proposing having a separate message for inclusion in the agent API outputs?
> 
> Vinod Kone wrote:
>     IIUC, Benjamin is questioning why we are storing both the spec (max grace period, mark_gone) and the status (state) in the same proto (do we do this elsewhere in the code base?). If they were separate protos, say DrainRequest and DrainStatus, master would send DrainRequest to the agent, and show DrainStatus (and possibly DrainRequest) in the operator API response. It looks a bit weird that we would send DrainInfo (as it is currently written  to the agent and do a CHECK on valid state. The fact that an agent got a Drain message is enough for the agent to know that it needs to drain, it doesn't need to look into the `DrainInfo::State`?
> 
> Vinod Kone wrote:
>     Let me repharse my first sentence. I think storing both spec and status in the same proto is probably fine if it makes thing easy to expose in API endpoints and use it internally in the master. But I still think it's weird to send the status in a message to the agent which is not expected to use it. So, if there's a way we can avoid it that would be great.

From the master's perspective, having the `State` in the DrainInfo complicates what the master needs to do in the registry.  To report the correct state to any endpoints, the master will need to assume all agents are in a `DRAINING` state, until the agent connects, reconciles, and says it has nothing running.  This means the state within the registry will always be `DRAINING` (and there is no reason to transition that state).

So moving the `State` field into a separate message would be ideal.


- Joseph


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


On June 18, 2019, 4:17 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 18, 2019, 4:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > <https://reviews.apache.org/r/70822/diff/3/?file=2148591#file2148591line986>
> >
> >     What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
> >     
> >     Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?

Yes we can have a CHECK on the agent to make sure the master doesn't send DRAINED in a DrainSlaveMessage. That will happen in another patch.

What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?


- Greg


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


On June 11, 2019, 5:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 5:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review215896
-----------------------------------------------------------




src/messages/messages.proto
Lines 986 (patched)
<https://reviews.apache.org/r/70822/#comment302831>

    What does it mean if the master sends a `DRAINED` state to the agent? Is that something we need to reject in validation?
    
    Does it maybe make sense to instead break out `DrainInfo.state` into its own message to use in state reporting?


- Benjamin Bannier


On June 11, 2019, 7:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 7:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review215871
-----------------------------------------------------------




include/mesos/mesos.proto
Lines 3721-3723 (patched)
<https://reviews.apache.org/r/70822/#comment302815>

    ... and all operations have been finished and ACK'd.



include/mesos/mesos.proto
Lines 3730-3731 (patched)
<https://reviews.apache.org/r/70822/#comment302816>

    You can mention that this removal is automatic (performed by the master).


- Joseph Wu


On June 11, 2019, 10:43 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 10:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review215812
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70822]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 11, 2019, 5:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 11, 2019, 5:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
>     https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/3/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 11, 2019, 5:43 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
  src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
  src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
  src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


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

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


Testing
-------

`make`


Thanks,

Greg Mann


Re: Review Request 70822: Added common protobufs for agent draining.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70822/#review215776
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70822]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 10, 2019, 2:24 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> -----------------------------------------------------------
> 
> (Updated June 10, 2019, 2:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/2/
> 
> 
> Testing
> -------
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 70822: Added common protobufs for agent draining.

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

(Updated June 10, 2019, 11:24 a.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, Joseph Wu, and Vinod Kone.


Summary (updated)
-----------------

Added common protobufs for agent draining.


Repository: mesos


Description
-------

This patch makes protobuf message updates which will be used
by both the master and the agent to facilitate automatic
draining of agents.


Diffs (updated)
-----

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 


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

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


Testing
-------

`make`


Thanks,

Greg Mann