You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2019/07/29 21:18:50 UTC

Review Request 71183: Stored last time a drain request was sent in the agent.

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
-------

This information can be used to calculate an approximate time when
draining an agent would be finished, e.g., by comparing
`DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
added here, both obtained from the agent API simultaneously. This
information is necessarily approximate as the agent might e.g., fail
over before it has finished draining which would reset the timeout; for
that specific case the master would send the drain request again so
after some time the value calculated from the agent API would be in
line with the expected true value.


Diffs
-----

  include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
  include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
  src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
  src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
  src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 


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


Testing
-------

`make check`


Thanks,

Benjamin Bannier


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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

> On July 30, 2019, 11:56 a.m., Greg Mann wrote:
> > src/slave/slave.hpp
> > Line 913 (original), 913 (patched)
> > <https://reviews.apache.org/r/71183/diff/1/?file=2158119#file2158119line913>
> >
> >     In the agent API, the agent will no longer have the drain config when it finishes draining; this information is simply lost.

Undid my edit here and also fixed the comment on the member added below.


- Benjamin


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


On July 30, 2019, 2:40 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71183/
> -----------------------------------------------------------
> 
> (Updated July 30, 2019, 2:40 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-9907
>     https://issues.apache.org/jira/browse/MESOS-9907
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This information can be used to calculate an approximate time when
> draining an agent would be finished, e.g., by comparing
> `DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
> added here, both obtained from the agent API simultaneously. This
> information is necessarily approximate as the agent might e.g., fail
> over before it has finished draining which would reset the timeout; for
> that specific case the master would send the drain request again so
> after some time the value calculated from the agent API would be in
> line with the expected true value.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
>   include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
>   src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
>   src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
>   src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 
> 
> 
> Diff: https://reviews.apache.org/r/71183/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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




include/mesos/agent/agent.proto
Lines 574 (patched)
<https://reviews.apache.org/r/71183/#comment304187>

    As in the master patch, let's consider `estimated_drain_start_time` for this field name, with a type of `TimeInfo`.



src/slave/slave.hpp
Line 913 (original), 913 (patched)
<https://reviews.apache.org/r/71183/#comment304189>

    In the agent API, the agent will no longer have the drain config when it finishes draining; this information is simply lost.


- Greg Mann


On July 29, 2019, 9:18 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71183/
> -----------------------------------------------------------
> 
> (Updated July 29, 2019, 9:18 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-9907
>     https://issues.apache.org/jira/browse/MESOS-9907
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This information can be used to calculate an approximate time when
> draining an agent would be finished, e.g., by comparing
> `DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
> added here, both obtained from the agent API simultaneously. This
> information is necessarily approximate as the agent might e.g., fail
> over before it has finished draining which would reset the timeout; for
> that specific case the master would send the drain request again so
> after some time the value calculated from the agent API would be in
> line with the expected true value.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
>   include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
>   src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
>   src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
>   src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 
> 
> 
> Diff: https://reviews.apache.org/r/71183/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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

(Updated Aug. 1, 2019, 9:52 a.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description (updated)
-------

This information can be used to calculate an approximate time when
draining an agent would be finished, e.g., by comparing
`DrainConfig.max_grace_period` and the field
`estimated_drain_start_time` added here, both obtained from the agent
API simultaneously. This information is necessarily approximate as the
agent might e.g., fail over before it has finished draining which would
reset the timeout; for that specific case the master would send the
drain request again so after some time the value calculated from the
agent API would be in line with the expected true value.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
  include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
  src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
  src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
  src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 


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

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


Testing
-------

`make check`


Thanks,

Benjamin Bannier


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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


Ship it!




Ship It!

- Greg Mann


On July 30, 2019, 12:40 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71183/
> -----------------------------------------------------------
> 
> (Updated July 30, 2019, 12:40 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-9907
>     https://issues.apache.org/jira/browse/MESOS-9907
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This information can be used to calculate an approximate time when
> draining an agent would be finished, e.g., by comparing
> `DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
> added here, both obtained from the agent API simultaneously. This
> information is necessarily approximate as the agent might e.g., fail
> over before it has finished draining which would reset the timeout; for
> that specific case the master would send the drain request again so
> after some time the value calculated from the agent API would be in
> line with the expected true value.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
>   include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
>   src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
>   src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
>   src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 
> 
> 
> Diff: https://reviews.apache.org/r/71183/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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




include/mesos/agent/agent.proto
Lines 574 (patched)
<https://reviews.apache.org/r/71183/#comment304233>

    Make sure you update the name of this field in the description.


- Greg Mann


On July 30, 2019, 12:40 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71183/
> -----------------------------------------------------------
> 
> (Updated July 30, 2019, 12:40 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-9907
>     https://issues.apache.org/jira/browse/MESOS-9907
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This information can be used to calculate an approximate time when
> draining an agent would be finished, e.g., by comparing
> `DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
> added here, both obtained from the agent API simultaneously. This
> information is necessarily approximate as the agent might e.g., fail
> over before it has finished draining which would reset the timeout; for
> that specific case the master would send the drain request again so
> after some time the value calculated from the agent API would be in
> line with the expected true value.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
>   include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
>   src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
>   src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
>   src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 
> 
> 
> Diff: https://reviews.apache.org/r/71183/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 71183: Stored last time a drain request was sent in the agent.

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

(Updated July 30, 2019, 2:40 p.m.)


Review request for mesos and Greg Mann.


Changes
-------

Address comments from Greg


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


Repository: mesos


Description
-------

This information can be used to calculate an approximate time when
draining an agent would be finished, e.g., by comparing
`DrainConfig.max_grace_period` and the field `drain_start_time_seconds`
added here, both obtained from the agent API simultaneously. This
information is necessarily approximate as the agent might e.g., fail
over before it has finished draining which would reset the timeout; for
that specific case the master would send the drain request again so
after some time the value calculated from the agent API would be in
line with the expected true value.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 3cb622d5a334c8cc49b1df6bd85d7cbba7dad1b6 
  include/mesos/v1/agent/agent.proto 4324ad64bfd20c2cdba7f053b8b0c0a400363e95 
  src/slave/http.cpp d9f113d82424c468131a549efc32c7b2991e9fc7 
  src/slave/slave.hpp 556d8ea119ec0aec59b341e537cc1b75af72ea79 
  src/slave/slave.cpp 9c14784e140df86fe654d3f83be7ba541d30222f 


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

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


Testing
-------

`make check`


Thanks,

Benjamin Bannier