mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 67761: Added a new registry field `minimum_capabilities`.
Date Fri, 29 Jun 2018 20:51:52 GMT


> 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.
> 
> Meng Zhu wrote:
>     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?

Hmm... let's take Quota Limits as an example:

1) Suppose we have a Master that supports Quota Limits.  The Registry entry for MinimalCapabilities
is initially blank because no quotas have been set.
2) The operator uses a V1 API to set a Quota Limit.
---> The master first checks its in-memory state to see what MinimumCapabilities it has
set in the Registry.  Finding no minimum, it runs an `AddMinimumCapability` operation.  Then
master persists the quota in the registry in a separate operation.
---> If we have an `UpdateMinimumCapability` operation, the master still needs to check
its in-memory state regardless.
3) The operator deletes the Quota Limit.
---> The master must check if all Quota Limits are disabled, and then run `RemoveMinimumCapability`.
 After that, the master still needs to remove the Quota itself.
---> Very similar picture for an `UpdateMinimumCapability` operation.

So it looks like having one or two operations won't change much.

---

However, it looks like we might run into problems with the non-atomicity of `Add/Remove-MinimumCapability`
with their associated Quota operations.  What if the process exits between an `AddMinimumCapability`
and an `UpdateQuota`?  Or between `RemoveQuota` and `RemoveMinimumCapability`?

What if the modifications to minimum capabilities take place inside `UpdateQuota` and `RemoveQuota`
instead?  i.e. The operation itself will update the required capabilities according to the
current state of the registry, rather than the Master doing so directly.


- Joseph


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


On June 29, 2018, 1:25 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67761/
> -----------------------------------------------------------
> 
> (Updated June 29, 2018, 1:25 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/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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