[
https://issues.apache.org/jira/browse/MESOS-4997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16097170#comment-16097170
]
Qian Zhang commented on MESOS-4997:
-----------------------------------
I have verified the issue mentioned in my previous comment by accessing Mesos
master endpoint:
{code}
$ curl -X POST -H "Content-Type: application/json" -d '{"type": "xxx"}'
127.0.0.1:5050/api/v1
Failed to convert JSON into Call protobuf: Failed to find enum for 'xxx'%
{code}
In the Call protobuf message, the enum {{Type}} already has a default value
{{UNKNOWN}} (see
[here|https://github.com/apache/mesos/blob/1.3.0/include/mesos/v1/master/master.proto#L45]
for details) and the field {{Call.type}} is optional, but the above curl
command will still fail. The root cause is, in the code
[here|https://github.com/apache/mesos/blob/1.3.0/3rdparty/stout/include/stout/protobuf.hpp#L449:L454]
when we try to get the enum value for the string "xxx", it will fail since
there is no any enum value corresponding to "xxx".
My proposal is, when we fail to get an enum for a string, check if it is an
optional enum field, if yes, call {{FieldDescriptor::default_value_enum()}} to
get the default value (in this case it is {{UNKNOWN}}), and use that default
value.
> 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)
