[
https://issues.apache.org/jira/browse/MESOS-4997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16097438#comment-16097438
]
James Peach commented on MESOS-4997:
------------------------------------
[~qianzhang] The JSON content type is generally more fragile than the protobuf
across API changes. This problem with enums also applies to schedulers
receiving new enum values from the master.
> Current approach to protobuf enums does not support upgrades.
> -------------------------------------------------------------
>
> Key: MESOS-4997
> URL: https://issues.apache.org/jira/browse/MESOS-4997
> Project: Mesos
> Issue Type: Epic
> Reporter: Benjamin Mahler
> Assignee: James Peach
> Priority: Critical
> Labels: tech-debt
>
> Some users were opting in to the recently introduced
> [TASK_KILLING_STATE|https://github.com/apache/mesos/blob/0.28.0/include/mesos/v1/mesos.proto#L259-L272]
> capability introduced in 0.28.0. When the scheduler ties to register with
> the TASK_KILLING_STATE capability against a 0.27.0 master, the master drops
> the message and prints the following:
> {noformat}
> [libprotobuf ERROR google/protobuf/message_lite.cc:123] Can't parse message
> of type "mesos.scheduler.Call" because it is missing required fields:
> subscribe.framework_info.capabilities[0].type
> {noformat}
> It turns out that our approach to enums in general does not allow for
> backwards compatibility. For example:
> {code}
> message Capability {
> enum Type {
> REVOCABLE_RESOURCES = 1;
> TASK_KILLING_STATE = 2; // New!
> }
> required Type type = 1;
> }
> {code}
> Using a required enum is problematic because protobuf will strip unknown enum
> values during de-serialization:
> https://developers.google.com/protocol-buffers/docs/proto#updating
> {quote}
> enum is compatible with int32, uint32, int64, and uint64 in terms of wire
> format (note that values will be truncated if they don't fit), but be aware
> that client code may treat them differently when the message is deserialized.
> Notably, unrecognized enum values are discarded when the message is
> deserialized, which makes the field's has.. accessor return false and its
> getter return the first value listed in the enum definition. However, an
> integer field will always preserve its value. Because of this, you need to be
> very careful when upgrading an integer to an enum in terms of receiving out
> of bounds enum values on the wire.
> {quote}
> The suggestion on the protobuf mailing list is to use optional enum fields
> and include an UNKNOWN value as the first entry in the enum list (and/or
> explicitly specifying it as the default):
> https://groups.google.com/forum/#!msg/protobuf/NhUjBfDyGmY/pf294zMi2bIJ
> The updated version of Capability would be:
> {code}
> message Capability {
> enum Type {
> UNKNOWN = 0;
> REVOCABLE_RESOURCES = 1;
> TASK_KILLING_STATE = 2; // New!
> }
> optional Type type = 1;
> }
> {code}
> Note that the first entry in an enum list is the default value, even if it's
> number is not the lowest (unless {{\[default = <value>\]}} is explicitly
> specified).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
