You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/06/27 20:26:15 UTC
Review Request 67762: Added minimum capability check during master
recovery.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Joseph Wu.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description
-------
Added minimum capability check during master registry
recovery to guard against incompatible downgrade.
See MESOS-8878.
Also added a dedicated test.
Diffs
-----
src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
Diff: https://reviews.apache.org/r/67762/diff/1/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205487
-----------------------------------------------------------
PASS: Mesos patch 67762 was successfully built and tested.
Reviews applied: `['67761', '67762']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67762
- Mesos Reviewbot Windows
On June 27, 2018, 8:26 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 8:26 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
> src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205485
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 67762]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On June 27, 2018, 8:26 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 8:26 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
> src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205601
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 67762]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On June 29, 2018, 11:37 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 29, 2018, 11:37 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224
> src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review212978
-----------------------------------------------------------
PASS: Mesos patch 67762 was successfully built and tested.
Reviews applied: `['67761', '67762']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2908/mesos-review-67762
- Mesos Reviewbot Windows
On Feb. 20, 2019, 10:46 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2019, 10:46 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp ccd117f607747d49e5259d9ba6645fed61811adf
> src/master/master.cpp 015da54583448a8d102d8e401e48bd228baf6dd6
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213323
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 70077, 67762]
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 Feb. 28, 2019, 11:08 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2019, 11:08 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new registry field `minimum_capabilities`
> to faciliatate safe downgrade of masters. This is a list
> of strings which will be written whenever the master uses a
> feature that may be backwards incompatible. If an older master
> sees an unknown feature in the list, the older master will
> refuse to recover and try to provide remediation steps instead.
>
> Also see: MESOS-8878.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
> src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213454
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 70077, 67762]
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 March 5, 2019, 6:53 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated March 5, 2019, 6:53 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Upon recovery, the master will compare its own capabilities against the
> list of strings read from the registry. If the master is missing any
> of the capabilities read from the registry, the master will refuse to
> recover and try to provide remediation steps instead.
>
> Modifications to the registry's list will be added in future, within
> the logic for other RegistryOperations. For example, if support for
> Quota limit bursting (MESOS-8068) is added to a future master,
> setting a quota limit should cause the UpdateQuota RegistryOperation
> to add the appropriate minimum capability. A RemoveQuota
> RegistryOperation would remove the minimum capability if there are no
> other quota limits in use.
>
> Also adds a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
> src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213496
-----------------------------------------------------------
Ship it!
I can tweak the below before committing.
src/master/master.cpp
Lines 1684 (patched)
<https://reviews.apache.org/r/67762/#comment299431>
s/compatibiliy/downgrades/
^ This is the doc name you chose for the previous patch in the chain.
- Joseph Wu
On March 5, 2019, 10:53 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated March 5, 2019, 10:53 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Upon recovery, the master will compare its own capabilities against the
> list of strings read from the registry. If the master is missing any
> of the capabilities read from the registry, the master will refuse to
> recover and try to provide remediation steps instead.
>
> Modifications to the registry's list will be added in future, within
> the logic for other RegistryOperations. For example, if support for
> Quota limit bursting (MESOS-8068) is added to a future master,
> setting a quota limit should cause the UpdateQuota RegistryOperation
> to add the appropriate minimum capability. A RemoveQuota
> RegistryOperation would remove the minimum capability if there are no
> other quota limits in use.
>
> Also adds a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
> src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
(Updated March 5, 2019, 10:53 a.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Fixed the commit message.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description (updated)
-------
Upon recovery, the master will compare its own capabilities against the
list of strings read from the registry. If the master is missing any
of the capabilities read from the registry, the master will refuse to
recover and try to provide remediation steps instead.
Modifications to the registry's list will be added in future, within
the logic for other RegistryOperations. For example, if support for
Quota limit bursting (MESOS-8068) is added to a future master,
setting a quota limit should cause the UpdateQuota RegistryOperation
to add the appropriate minimum capability. A RemoveQuota
RegistryOperation would remove the minimum capability if there are no
other quota limits in use.
Also adds a dedicated test.
Diffs (updated)
-----
include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
Diff: https://reviews.apache.org/r/67762/diff/6/
Changes: https://reviews.apache.org/r/67762/diff/5-6/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213322
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 70077, 67762]
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 March 1, 2019, 7:08 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated March 1, 2019, 7:08 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new registry field `minimum_capabilities`
> to faciliatate safe downgrade of masters. This is a list
> of strings which will be written whenever the master uses a
> feature that may be backwards incompatible. If an older master
> sees an unknown feature in the list, the older master will
> refuse to recover and try to provide remediation steps instead.
>
> Also see: MESOS-8878.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
> src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
(Updated Feb. 28, 2019, 3:08 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Addressed kaysoky's comments.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description (updated)
-------
This patch adds a new registry field `minimum_capabilities`
to faciliatate safe downgrade of masters. This is a list
of strings which will be written whenever the master uses a
feature that may be backwards incompatible. If an older master
sees an unknown feature in the list, the older master will
refuse to recover and try to provide remediation steps instead.
Also see: MESOS-8878.
Diffs (updated)
-----
include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
Diff: https://reviews.apache.org/r/67762/diff/5/
Changes: https://reviews.apache.org/r/67762/diff/4-5/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On Feb. 26, 2019, 6:04 p.m., Joseph Wu wrote:
> > src/master/master.cpp
> > Lines 1680 (patched)
> > <https://reviews.apache.org/r/67762/diff/4/?file=2125976#file2125976line1680>
> >
> > A forced recovery doesn't really make sense, if we're trying to prevent undefined behavior. If a recovery must be forced, and lacking any other context, my recommendation would be purging the registry and starting the cluster anew.
Removed.
> On Feb. 26, 2019, 6:04 p.m., Joseph Wu wrote:
> > src/master/master.cpp
> > Lines 1682-1684 (patched)
> > <https://reviews.apache.org/r/67762/diff/4/?file=2125976#file2125976line1682>
> >
> > I wonder if we should start a document (even blank?) which lists each minimum capability, some background, and how to recover from each.
> >
> > Using quota limits as an example, the recovery steps might be to launch the newer master, and send operator calls modifying all quotas into a backwards compatible form.
> >
> > ---
> >
> > We can then link the document in the error message as a fallback.
Done. Added a doc in https://reviews.apache.org/r/70077/.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213241
-----------------------------------------------------------
On Feb. 28, 2019, 3:08 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2019, 3:08 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new registry field `minimum_capabilities`
> to faciliatate safe downgrade of masters. This is a list
> of strings which will be written whenever the master uses a
> feature that may be backwards incompatible. If an older master
> sees an unknown feature in the list, the older master will
> refuse to recover and try to provide remediation steps instead.
>
> Also see: MESOS-8878.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp aceab3455adfdf7ff6e168e033316997a4d36fb5
> src/master/master.cpp 665c1c7979a5ae4ecec0d5a68e59c5419049a4d5
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213241
-----------------------------------------------------------
Commit description could use some extra context. Not sure if you want to use another example (instead of quota limits) to illustrate how the list of minimum capabilities should be updated.
Suggestion:
```
Upon recovery, the master will compare its own capabilities against the
list of strings read from the registry. If the master is missing any
of the capabilities read from the registry, the master will refuse to
recover and try to provide remediation steps instead.
Modifications to the registry's list will be added in future, within
the logic for other RegistryOperations. For example, if support for
Quota limit bursting (MESOS-8068) is added to a future master,
setting a quota limit should cause the UpdateQuota RegistryOperation
to add the appropriate minimum capability. A RemoveQuota
RegistryOperation would remove the minimum capability if there are no
other quota limits in use.
Also adds a dedicated test.
```
src/master/master.hpp
Lines 348 (patched)
<https://reviews.apache.org/r/67762/#comment299142>
Nit: s/an hashset of missing minimum capabilities in string/the set of capabilities missing from this master/
src/master/master.cpp
Lines 1680 (patched)
<https://reviews.apache.org/r/67762/#comment299144>
A forced recovery doesn't really make sense, if we're trying to prevent undefined behavior. If a recovery must be forced, and lacking any other context, my recommendation would be purging the registry and starting the cluster anew.
src/master/master.cpp
Lines 1682-1684 (patched)
<https://reviews.apache.org/r/67762/#comment299145>
I wonder if we should start a document (even blank?) which lists each minimum capability, some background, and how to recover from each.
Using quota limits as an example, the recovery steps might be to launch the newer master, and send operator calls modifying all quotas into a backwards compatible form.
---
We can then link the document in the error message as a fallback.
src/master/master.cpp
Lines 1685-1687 (patched)
<https://reviews.apache.org/r/67762/#comment299146>
Nit: Suggestion:
```
<< "Master is missing the following minimum capabilities: "
<< strings::join<hashset<string>>(", ", missingCapabilities)
<< ". See the following documentation for steps to safely "
<< "recover from this state: "
<< "http://mesos.apache.org/documentation/latest/compatibility"; // <-- Just a possible link.
```
- Joseph Wu
On Feb. 20, 2019, 10:46 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2019, 10:46 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp ccd117f607747d49e5259d9ba6645fed61811adf
> src/master/master.cpp 015da54583448a8d102d8e401e48bd228baf6dd6
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review213106
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 67762]
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 Feb. 20, 2019, 6:46 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2019, 6:46 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
> include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
> src/master/master.hpp ccd117f607747d49e5259d9ba6645fed61811adf
> src/master/master.cpp 015da54583448a8d102d8e401e48bd228baf6dd6
> src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
>
>
> Diff: https://reviews.apache.org/r/67762/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
(Updated Feb. 20, 2019, 10:46 a.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Rebased.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description
-------
Added minimum capability check during master registry
recovery to guard against incompatible downgrade.
See MESOS-8878.
Also added a dedicated test.
Diffs (updated)
-----
include/mesos/mesos.proto 48f30b3f41f3440245c1912becc9c2c3d572aff9
include/mesos/v1/mesos.proto e07dd9e42939fddcff5d15072a143d9c4c44dd3d
src/master/master.hpp ccd117f607747d49e5259d9ba6645fed61811adf
src/master/master.cpp 015da54583448a8d102d8e401e48bd228baf6dd6
src/tests/master_tests.cpp 5ae8e1cea3ca87551093bd63d744ac807ac7797a
Diff: https://reviews.apache.org/r/67762/diff/4/
Changes: https://reviews.apache.org/r/67762/diff/3-4/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205602
-----------------------------------------------------------
PASS: Mesos patch 67762 was successfully built and tested.
Reviews applied: `['67761', '67762']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67762
- Mesos Reviewbot Windows
On June 29, 2018, 4:37 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 29, 2018, 4:37 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224
> src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
(Updated June 29, 2018, 4:37 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Thanks for the review! Patch updated.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description
-------
Added minimum capability check during master registry
recovery to guard against incompatible downgrade.
See MESOS-8878.
Also added a dedicated test.
Diffs (updated)
-----
include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224
src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
Diff: https://reviews.apache.org/r/67762/diff/3/
Changes: https://reviews.apache.org/r/67762/diff/2-3/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On June 29, 2018, 2:33 p.m., Joseph Wu wrote:
> > src/master/master.cpp
> > Lines 406-418 (patched)
> > <https://reviews.apache.org/r/67762/diff/2/?file=2046750#file2046750line406>
> >
> > Consider defining a `hash<MasterInfo::Capability::Type>` and performing a hashset subtraction instead of these loops.
Done.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205583
-----------------------------------------------------------
On June 29, 2018, 4:37 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 29, 2018, 4:37 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224
> src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205583
-----------------------------------------------------------
src/master/master.hpp
Lines 443-444 (patched)
<https://reviews.apache.org/r/67762/#comment288459>
You could simplify the return type to `vector<string>`, as the empty `vector` is basically the same as returning `None`.
src/master/master.cpp
Lines 399-403 (patched)
<https://reviews.apache.org/r/67762/#comment288457>
Is there any particular reason why you've wrapped this logic inside a lambda? You'd get roughly the same result if you unwrapped the logic.
src/master/master.cpp
Lines 406-418 (patched)
<https://reviews.apache.org/r/67762/#comment288460>
Consider defining a `hash<MasterInfo::Capability::Type>` and performing a hashset subtraction instead of these loops.
- Joseph Wu
On June 27, 2018, 10:01 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 10:01 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
> src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205490
-----------------------------------------------------------
PASS: Mesos patch 67762 was successfully built and tested.
Reviews applied: `['67761', '67762']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67762
- Mesos Reviewbot Windows
On June 28, 2018, 5:01 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 28, 2018, 5:01 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
> src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/#review205491
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67761, 67762]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On June 28, 2018, 1:01 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67762/
> -----------------------------------------------------------
>
> (Updated June 28, 2018, 1:01 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-8880
> https://issues.apache.org/jira/browse/MESOS-8880
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added minimum capability check during master registry
> recovery to guard against incompatible downgrade.
> See MESOS-8878.
>
> Also added a dedicated test.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
> include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
> src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
> src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
> src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
>
>
> Diff: https://reviews.apache.org/r/67762/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 67762: Added minimum capability check during master
recovery.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67762/
-----------------------------------------------------------
(Updated June 27, 2018, 10:01 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Added a reminder in the `MasterInfo::Capability`.
Bugs: MESOS-8880
https://issues.apache.org/jira/browse/MESOS-8880
Repository: mesos
Description
-------
Added minimum capability check during master registry
recovery to guard against incompatible downgrade.
See MESOS-8878.
Also added a dedicated test.
Diffs (updated)
-----
include/mesos/mesos.proto 463e6adcdf14115b2f21270bb1fd9c45e1b67cc3
include/mesos/v1/mesos.proto 8eaad9c4b2a9cdd527922a004b0f7dd0dc58a7f1
src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c
src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a
src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb
Diff: https://reviews.apache.org/r/67762/diff/2/
Changes: https://reviews.apache.org/r/67762/diff/1-2/
Testing
-------
make check
Thanks,
Meng Zhu