mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 64009: Added new UpdateSlave registry operation.
Date Thu, 30 Nov 2017 23:11:07 GMT


> On Nov. 29, 2017, 4:30 p.m., Michael Park wrote:
> > I think the fact that `UpdateSlave` gets a `SlaveInfo` in the old format is due
> > to the fact that we don't upgrade those resources in the master until we get to
> > the `Slave` struct ctors, as well as the fact that the agents *always* send
> > messages in the old format today (due to lack of master capabilities). Once we
> > update the code such that new agents send messages in the new format, we'll
> > need to add code to *downgrade* for the registrar.
> > 
> > Given this, it would be awesome if we upgrade the resources earlier in
> > the master and downgrade for the registrar correspondingly now. This means that
> > we only need to upgrade the `SlaveInfo` coming out of the registry to perform
> > the comparison, rather than introducing a `equalAfterConversion` function which
> > feels off to me.
> > 
> > Having said all of that, I don't know the urgency nor the scope of the work
> > being done here. The current solution works for now, so if you want and/or need
> > to proceed with this, I won't stop it.
> 
> Vinod Kone wrote:
>     sounds like a plan. we will do that in a separate review and make this review depend
on that.

One problem with that is the question of where to store the updated SlaveInfo. Right now,
it is the copy stored in each `Master::Slave` struct that gets updated, but with this change
we would like to upgrade the resources before that is created/accessed.

Ideally we would just directly modify the protobuf message coming from the agent, but I didn't
find any precedence for doing such a thing within mesos.

Alternatively, we could change e.g. `Master.registering` to contain the updated SlaveInfo,
or introduce a new field for that, but that seems to be a bit too much to be worth it.


- Benno


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


On Nov. 29, 2017, 6:06 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64009/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This operation can be used to update the stored state
> of an existing, admitted slave.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.hpp 6f991e86e46512d5a2bc4e67e160189fccb77f6a 
>   src/common/protobuf_utils.cpp c0ff306ae6c16cbba6fd08469b639b9f906c672b 
>   src/master/registry_operations.hpp PRE-CREATION 
>   src/master/registry_operations.cpp PRE-CREATION 
>   src/tests/registrar_tests.cpp b0fc5f64dbaf841a548b1e19c90ed47bd1248872 
> 
> 
> Diff: https://reviews.apache.org/r/64009/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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