You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2016/03/22 03:29:34 UTC

Usage of protobuf 'enum' fields

Hi folks,

I wanted to surface the following ticket to our attention:
https://issues.apache.org/jira/browse/MESOS-4997

The issue is that when enum fields are deserialized, unknown enum values
are _stripped_. This means that if an enum field is 'required' and a new
value is added, old clients cannot deserialize messages with the new enum
value set: the message is considered to have a missing required field and
is dropped.

The suggested approach to ensure new enum values can be safely added is the
following:

-Enum fields should be optional.
-The first entry in an enum list should be UNKNOWN (and/or we set [default
= UNKNOWN]).

Having them as optional ensures that the protobuf deserialization considers
messages with stripped enum fields to be initialized. Also, if our code
calls the getter unconditionally it is safer to get UNKNOWN rather than an
arbitrary enum value (whatever happens to be the first in the list).

I will follow up and ensure we fix this for FrameworkInfo::Capability,
where we added a TASK_KILLING_STATE capability in 0.28. Frameworks that try
to set this new capability but talk to a 0.27 (or earlier) master will not
be able to register because the message will be dropped.

Ben

Re: Usage of protobuf 'enum' fields

Posted by Zameer Manji <zm...@apache.org>.
+1

I have run into this issue before and it was very confusing.

On Tue, Mar 22, 2016 at 1:37 AM, tommy xiao <xi...@gmail.com> wrote:

> yes, following apache upgrade doc guide, the step is master update firstly,
> than upgrade slave. it can't support slave firstly. so this is rule on our
> ops step.
>
> 2016-03-22 10:29 GMT+08:00 Benjamin Mahler <bm...@apache.org>:
>
> > Hi folks,
> >
> > I wanted to surface the following ticket to our attention:
> > https://issues.apache.org/jira/browse/MESOS-4997
> >
> > The issue is that when enum fields are deserialized, unknown enum values
> > are _stripped_. This means that if an enum field is 'required' and a new
> > value is added, old clients cannot deserialize messages with the new enum
> > value set: the message is considered to have a missing required field and
> > is dropped.
> >
> > The suggested approach to ensure new enum values can be safely added is
> the
> > following:
> >
> > -Enum fields should be optional.
> > -The first entry in an enum list should be UNKNOWN (and/or we set
> [default
> > = UNKNOWN]).
> >
> > Having them as optional ensures that the protobuf deserialization
> considers
> > messages with stripped enum fields to be initialized. Also, if our code
> > calls the getter unconditionally it is safer to get UNKNOWN rather than
> an
> > arbitrary enum value (whatever happens to be the first in the list).
> >
> > I will follow up and ensure we fix this for FrameworkInfo::Capability,
> > where we added a TASK_KILLING_STATE capability in 0.28. Frameworks that
> try
> > to set this new capability but talk to a 0.27 (or earlier) master will
> not
> > be able to register because the message will be dropped.
> >
> > Ben
> >
>
>
>
> --
> Deshi Xiao
> Twitter: xds2000
> E-mail: xiaods(AT)gmail.com
>
> --
> Zameer Manji
>
> <http://gmail.com>

Re: Usage of protobuf 'enum' fields

Posted by tommy xiao <xi...@gmail.com>.
yes, following apache upgrade doc guide, the step is master update firstly,
than upgrade slave. it can't support slave firstly. so this is rule on our
ops step.

2016-03-22 10:29 GMT+08:00 Benjamin Mahler <bm...@apache.org>:

> Hi folks,
>
> I wanted to surface the following ticket to our attention:
> https://issues.apache.org/jira/browse/MESOS-4997
>
> The issue is that when enum fields are deserialized, unknown enum values
> are _stripped_. This means that if an enum field is 'required' and a new
> value is added, old clients cannot deserialize messages with the new enum
> value set: the message is considered to have a missing required field and
> is dropped.
>
> The suggested approach to ensure new enum values can be safely added is the
> following:
>
> -Enum fields should be optional.
> -The first entry in an enum list should be UNKNOWN (and/or we set [default
> = UNKNOWN]).
>
> Having them as optional ensures that the protobuf deserialization considers
> messages with stripped enum fields to be initialized. Also, if our code
> calls the getter unconditionally it is safer to get UNKNOWN rather than an
> arbitrary enum value (whatever happens to be the first in the list).
>
> I will follow up and ensure we fix this for FrameworkInfo::Capability,
> where we added a TASK_KILLING_STATE capability in 0.28. Frameworks that try
> to set this new capability but talk to a 0.27 (or earlier) master will not
> be able to register because the message will be dropped.
>
> Ben
>



-- 
Deshi Xiao
Twitter: xds2000
E-mail: xiaods(AT)gmail.com