> On April 12, 2018, 5:33 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 4986-4994 (patched) > > <https://reviews.apache.org/r/66050/diff/8/?file=1994610#file1994610line4986> > > > > What will happen if these operations are sent to a 1.5 agent which has > > the RESOURCE_PROVIDER capability, but does not support the operation? > > Zhitao Li wrote: > Good catch. > > For a 1.5 agent, I think it will crash at this line: > https://github.com/apache/mesos/blob/1.5.0/src/common/protobuf_utils.cpp#L851 > because it cannot know whether this unknown operation is speculative. I guess > that triggers the agent to reregister with master without changing its > checkpointed resources. > > In fact, this is much better than attempting to apply, because agent > would do a delete followed by recreate and cause data loss on the persistent > volume (the fix is in r/66218). > > It seems like agent version was never present in `AgentInfo` so master > cannot really know this is an old agent version. Do you want to consider > adding that? > > Greg Mann wrote: > Ah, it looks like the version is included in the 'RegisterSlaveMessage' > and 'ReregisterSlaveMessage', and the master places it in the 'Slave' struct: > https://github.com/apache/mesos/blob/bd688e4cf9f6f35c9d75aad50b99e1fdeb8104da/src/master/master.cpp#L6586 > > Perhaps we should check that the version is >= 1.6.0 rather than checking > for the RESOURCE_PROVIDER capability? > > Chun-Hung Hsiao wrote: > In proto2, since the enum is not defined in 1.5 proto, the enum field > will be treated as an unknown field. > See: https://developers.google.com/protocol-buffers/docs/proto#enum
I'm inclined not to add a version check anymore, but just mention that this is not safe to use if operator is using 1.5.x agent with *experimental* `RESOURCE_PROVIDER` feature. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66050/#review201055 ----------------------------------------------------------- On April 17, 2018, 9:58 a.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66050/ > ----------------------------------------------------------- > > (Updated April 17, 2018, 9:58 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > The new offer operations are implemented as speculative operations, but > we will use validation to make them non-speculative on API level so that > we can transition later without a breaking change. > > > Diffs > ----- > > src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 > src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 > src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a > src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf > src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d > > > Diff: https://reviews.apache.org/r/66050/diff/11/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
