You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2019/06/21 16:00:53 UTC
Review Request 70923: Added a registry operation for the DRAIN_AGENT
call.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
Bugs: MESOS-9814
https://issues.apache.org/jira/browse/MESOS-9814
Repository: mesos
Description
-------
This adds the associated registry operation and fields for the
DRAIN_AGENT master call. This call affects admitted or unreachable
agents and thus has data fields into two parts of the registry.
When agents transition from reachable to unreachable, or vice
versa, the draining info is now copied too.
Because this feature is not downgrade compatible, a minimum capability
is added when the draining feature is used.
Diffs
-----
src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
Diff: https://reviews.apache.org/r/70923/diff/1/
Testing
-------
make check
Thanks,
Joseph Wu
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On June 24, 2019, 11:47 a.m., Meng Zhu wrote:
> > src/master/registry_operations.cpp
> > Lines 422-433 (patched)
> > <https://reviews.apache.org/r/70923/diff/2/?file=2151646#file2151646line426>
> >
> > I do not see the capability removed anywhere. But in the previous patch, we said:
> >
> > ```
> > To remove this minimum capability requirement:
> >
> > ```
> >
> > I am OK with not removing any minimum capability (even they are not needed anymore). But we should mention that in the doc.
Ah, see the next patch. DrainAgent covers a single transition (Normal -> Draining). The (Draining/ed -> Normal) transition (ReactivateAgent) is where we can remove the minimum capability.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216102
-----------------------------------------------------------
On June 21, 2019, 9 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 9 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents and thus has data fields into two parts of the registry.
>
> When agents transition from reachable to unreachable, or vice
> versa, the draining info is now copied too.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
>
>
> Diff: https://reviews.apache.org/r/70923/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216102
-----------------------------------------------------------
src/master/registry_operations.cpp
Lines 422-433 (patched)
<https://reviews.apache.org/r/70923/#comment303094>
I do not see the capability removed anywhere. But in the previous patch, we said:
```
To remove this minimum capability requirement:
```
I am OK with not removing any minimum capability (even they are not needed anymore). But we should mention that in the doc.
- Meng Zhu
On June 21, 2019, 9 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 9 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents and thus has data fields into two parts of the registry.
>
> When agents transition from reachable to unreachable, or vice
> versa, the draining info is now copied too.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
>
>
> Diff: https://reviews.apache.org/r/70923/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216065
-----------------------------------------------------------
Fix it, then Ship it!
src/master/registry_operations.cpp
Lines 128 (patched)
<https://reviews.apache.org/r/70923/#comment303036>
s/any //
- Greg Mann
On June 21, 2019, 4 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 4 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents and thus has data fields into two parts of the registry.
>
> When agents transition from reachable to unreachable, or vice
> versa, the draining info is now copied too.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
>
>
> Diff: https://reviews.apache.org/r/70923/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216230
-----------------------------------------------------------
src/master/registry_operations.cpp
Lines 387-403 (patched)
<https://reviews.apache.org/r/70923/#comment303356>
This also came up in the quota changes, my comment from there:
--
Let's pull something out for this, since it's going to come up again.
E.g.
```
protobuf::master::Capabilities capabilities = registry->minimum_capabilities();
capabilities.agentDraining = true;
*registry->mutable_minimum_capabilities() = capabilities.toStrings();
```
For this to work, we have to:
-Update protobuf::master::Capabilities to take in repeated strings (in addition to the existing repeated proto constructor)
-Carry the unknown capabilities as a member of protobuf::master::Capabilities (we can just tackle the strings for now and leave the existing TODO on the protobuf constructor)
-Add a toStrings() that goes back to repeated string
This will make this logic very simple and easy to read, and leave very little room for error across any operations that modify the minimum capabilities. We can also unit test the C++ wrapper to make sure it's correct!
- Benjamin Mahler
On June 26, 2019, 8:44 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated June 26, 2019, 8:44 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents, but this commit only deals with admitted agents.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
>
>
> Diff: https://reviews.apache.org/r/70923/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216288
-----------------------------------------------------------
src/master/registry_operations.cpp
Lines 388-402 (patched)
<https://reviews.apache.org/r/70923/#comment303434>
For the time being, we can use:
https://github.com/apache/mesos/blob/3720e4cf5f7cb0d8e98afacea39528bd41c767b4/src/common/protobuf_utils.hpp#L501-L513
- Meng Zhu
On June 26, 2019, 1:44 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated June 26, 2019, 1:44 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents, but this commit only deals with admitted agents.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
>
>
> Diff: https://reviews.apache.org/r/70923/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/#review216661
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On July 8, 2019, 6:38 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70923/
> -----------------------------------------------------------
>
> (Updated July 8, 2019, 6:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-9814
> https://issues.apache.org/jira/browse/MESOS-9814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This adds the associated registry operation and fields for the
> DRAIN_AGENT master call. This call affects admitted or unreachable
> agents, but this commit only deals with admitted agents.
>
> Because this feature is not downgrade compatible, a minimum capability
> is added when the draining feature is used.
>
>
> Diffs
> -----
>
> src/master/registry.proto 67904edf73235a4007f7c59c05dc4f1abab1cd08
> src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
> src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
> src/tests/registrar_tests.cpp 5f0de523705d124ddd2d21ad355f06633d68c141
>
>
> Diff: https://reviews.apache.org/r/70923/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/
-----------------------------------------------------------
(Updated July 8, 2019, 11:38 a.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
Changes
-------
Updated to use the minimum capability helper.
Bugs: MESOS-9814
https://issues.apache.org/jira/browse/MESOS-9814
Repository: mesos
Description
-------
This adds the associated registry operation and fields for the
DRAIN_AGENT master call. This call affects admitted or unreachable
agents, but this commit only deals with admitted agents.
Because this feature is not downgrade compatible, a minimum capability
is added when the draining feature is used.
Diffs (updated)
-----
src/master/registry.proto 67904edf73235a4007f7c59c05dc4f1abab1cd08
src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
src/tests/registrar_tests.cpp 5f0de523705d124ddd2d21ad355f06633d68c141
Diff: https://reviews.apache.org/r/70923/diff/4/
Changes: https://reviews.apache.org/r/70923/diff/3-4/
Testing
-------
make check
Thanks,
Joseph Wu
Re: Review Request 70923: Added a registry operation for the
DRAIN_AGENT call.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70923/
-----------------------------------------------------------
(Updated June 26, 2019, 1:44 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and Vinod Kone.
Changes
-------
Rebased on newer protobufs, which added the `State` field into the Registry.
Also split out the modifications of unreachable operations into another review: https://reviews.apache.org/r/70956/
Bugs: MESOS-9814
https://issues.apache.org/jira/browse/MESOS-9814
Repository: mesos
Description (updated)
-------
This adds the associated registry operation and fields for the
DRAIN_AGENT master call. This call affects admitted or unreachable
agents, but this commit only deals with admitted agents.
Because this feature is not downgrade compatible, a minimum capability
is added when the draining feature is used.
Diffs (updated)
-----
src/master/registry.proto 239789322c45c6a8346332f50661a98b1851b685
src/master/registry_operations.hpp 5a3010d46c4eddb0d660eb7368ec836903477a9b
src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4
src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8
Diff: https://reviews.apache.org/r/70923/diff/3/
Changes: https://reviews.apache.org/r/70923/diff/2-3/
Testing
-------
make check
Thanks,
Joseph Wu