mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 67761: Added a new registry field `minimum_capabilities`.
Date Fri, 29 Jun 2018 20:25:41 GMT


> On June 29, 2018, 12:28 p.m., Joseph Wu wrote:
> > src/master/registry.proto
> > Lines 88 (patched)
> > <https://reviews.apache.org/r/67761/diff/1/?file=2046454#file2046454line88>
> >
> >     Could use a comment along the lines of:
> >     ```
> >     The minimum set of `MasterInfo::Capability` that the master reading
> >     this registry should be capable of.  If the master is upgraded
> >     or downgraded and no longer satisfies these capabilities, the
> >     master is expected to exit upon recovery.
> >     
> >     We do not use `MasterInfo::Capability` in this field directly
> >     because the enumeration may be extended in later Mesos versions.
> >     If an earlier version is given an enum value from a later version,
> >     the protobuf parser will interpret this as an `UNKNOWN` capability,
> >     and we will be unable to provide remediation text to the operator.
> >     ```

Great comment, thanks!


> On June 29, 2018, 12:28 p.m., Joseph Wu wrote:
> > src/master/registry_operations.hpp
> > Lines 140-163 (patched)
> > <https://reviews.apache.org/r/67761/diff/1/?file=2046455#file2046455line140>
> >
> >     Instead of Add/Remove, how about an `UpdateMinimumCapability` operation instead?
> >     
> >     It would just take the whole list of capabilities and save it.  An "update"
would be less prone to dev error, as we wouldn't need to account for the current set of capabilities
in the registry.

It is intentional that we only support add/remove minimum capability entries one by one. The
idea is that we should only add a minimum capability only if the capability is actually required
and we will remove that once it is deemed no longer necessary.

For example, in the future, we will only add `QUOTA_LIMIT` as minimum capability iff Mesos
needs to set up a quota limit that is different from the quota guarantee. And it will be removed
upon the removal of the last quota entry.

We are hoping devs will have intimate knowledge about the minimum capability entry that relates
to their features and fine-tune the entries as needed.

What do you think?


- Meng


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


On June 27, 2018, 1:26 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67761/
> -----------------------------------------------------------
> 
> (Updated June 27, 2018, 1: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
> -------
> 
> This patch adds a new registry field `minimum_capabilities`
> to faciliatate safe downgrade. See MESOS-8878.
> 
> Also added two accompanying registry operations
> `AddMinimumCapability` and `RemoveMinimumCapability`.
> 
> Also added a test for the new operations.
> 
> 
> Diffs
> -----
> 
>   src/master/registry.proto cac0c3196aed601ba41c337e8d746819ce42f8d9 
>   src/master/registry_operations.hpp c07268ea0a2dd5c95399d860fcbe8ab804ecef92 
>   src/master/registry_operations.cpp c417c4d6c4272eeab15f5313ca5160d4a0aa4da4 
>   src/tests/registrar_tests.cpp d3e4ee0164b8730b39ab05f4880fd0aa034ec8b0 
> 
> 
> Diff: https://reviews.apache.org/r/67761/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message