[
https://issues.apache.org/jira/browse/MESOS-4997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16094803#comment-16094803
]
Qian Zhang commented on MESOS-4997:
-----------------------------------
[~bmahler] I have a question for this ticket: when a framework subscribes with
Mesos master, there are two ways to send the encoded data in HTTP request:
{{Content-Type: application/x-protobuf}} and {{Content-Type:
application/json}}. For the former, I think it will have the issue mentioned
in this ticket and it can be resolved by using optional enum field and
including an UNKNOWN value as the first entry in the enum. But for the later,
even we use that solution, there will still be an error like {{Failed to find
enum for 'xxx'}} when {{protobuf::parse()}} is called. So I think we may need a
solution to handle the later case?
> 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
> 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)
