Use of required keyword in protobuf messages

[Chris Trezzo <ct...@gmail.com>]

Hi All,

I was looking through the .proto files and noticed there are a lot of
fields that are marked as required. I am by no means a protobuf expert, but
I was wondering what advantage do we actually get in making fields required?

I understand that if we don't use the required keyword we would have to
implement custom application logic, but the flexibility we gain from having
all the fields optional seems to outweigh that work. In addition, we will
already have to add logic to HBase to handle version compatibility, so it
seems natural to implement the required logic as part of that layer. This
would allow us to change or delete any message field and maintain wire
compatibility.

Quote from the protobuf language guide (
https://developers.google.com/protocol-buffers/docs/proto):

"*Required Is Forever* You should be very careful about marking fields as
required. If at some point you wish to stop writing or sending a required
field, it will be problematic to change the field to an optional field –
old readers will consider messages without this field to be incomplete and
may reject or drop them unintentionally. You should consider writing
application-specific custom validation routines for your buffers instead.
Some engineers at Google have come to the conclusion that using
requireddoes more harm than good; they prefer to use only
optional and repeated. However, this view is not universal."

Thoughts?

Thanks,
Chris Trezzo

Re: Use of required keyword in protobuf messages

[Todd Lipcon <to...@cloudera.com>]

Yea, my recollection is that we decided to leave rolling upgrade
across major versions out of scope for now, but like Greg said, try to
do it between any two major versions where it looks "easy enough".

-Todd

On Wed, Aug 8, 2012 at 12:39 PM, Gregory Chanan <gc...@cloudera.com> wrote:
> Good question, Lars.
>
> I'm going off of the proposal (
> http://wiki.apache.org/hadoop/Hbase/HBaseWireCompatibility
> ) and the slides at the meetup (
> http://files.meetup.com/1350427/wire-compat%20%281%29.pptx).  If I missed
> anything, I apologize.
>
> Those list the minimum requirements to support the use cases and goals on
> slides 4-6.  So, as written, there is no *guarantee* of support for rolling
> upgrade between major versions.
>
> Of course, there is nothing stopping us from adding that guarantee.  Or
> from just guaranteeing it for specific versions (e.g. 0.96 to 0.98 in the
> same manner as 0.92 to 0.94).  It should be much easier to achieve the
> latter than in the past, once everything has been protobufed.  Perhaps we
> should discuss in another thread?
>
> So I'll withdraw my point that perhaps we could treat server-server and
> client-server communication differently, but still submit the point that we
> don't need to keep required fields around forever.
>
> Greg
>
> On Tue, Aug 7, 2012 at 5:23 PM, lars hofhansl <lh...@yahoo.com> wrote:
>
>> Huh?
>>
>> I thought we're allowing for a rolling upgrade between major versions.
>> If server/server is only wire compatible across a minor version that won't
>> be possible.
>>
>> (this might have been mentioned in the initial discussion we all had about
>> this a while back and I might have just forgotten)
>>
>>
>> -- Lars
>>
>>
>> ----- Original Message -----
>> From: Gregory Chanan <gc...@cloudera.com>
>> To: dev@hbase.apache.org
>> Cc:
>> Sent: Tuesday, August 7, 2012 1:33 PM
>> Subject: Re: Use of required keyword in protobuf messages
>>
>> I agree with most of the comments here, particularly that at some point we
>> should go through and review all the "required" fields.
>>
>> Another thing to keep in mind is that we have only guaranteed the following
>> are compatible:
>> - Client/Server across 1 major version
>> - Server/Server different minor versions
>>
>> So we don't need to keep required fields for eons, necessarily.  And it may
>> make sense to be looser about allowing "required" for server/server
>> communication than for client/server.
>>
>> Greg
>>
>> On Tue, Aug 7, 2012 at 11:39 AM, Devaraj Das <dd...@hortonworks.com> wrote:
>>
>> > I too tend to make fields optional unless I am really convinced that the
>> > field would live for eons. I agree with Google's philosophy in that
>> regard.
>> >
>> > On Aug 6, 2012, at 3:39 PM, Chris Trezzo wrote:
>> >
>> > > Hi All,
>> > >
>> > > I was looking through the .proto files and noticed there are a lot of
>> > > fields that are marked as required. I am by no means a protobuf expert,
>> > but
>> > > I was wondering what advantage do we actually get in making fields
>> > required?
>> > >
>> > > I understand that if we don't use the required keyword we would have to
>> > > implement custom application logic, but the flexibility we gain from
>> > having
>> > > all the fields optional seems to outweigh that work. In addition, we
>> will
>> > > already have to add logic to HBase to handle version compatibility, so
>> it
>> > > seems natural to implement the required logic as part of that layer.
>> This
>> > > would allow us to change or delete any message field and maintain wire
>> > > compatibility.
>> > >
>> > > Quote from the protobuf language guide (
>> > > https://developers.google.com/protocol-buffers/docs/proto):
>> > >
>> > > "*Required Is Forever* You should be very careful about marking fields
>> as
>> > > required. If at some point you wish to stop writing or sending a
>> required
>> > > field, it will be problematic to change the field to an optional field
>> –
>> > > old readers will consider messages without this field to be incomplete
>> > and
>> > > may reject or drop them unintentionally. You should consider writing
>> > > application-specific custom validation routines for your buffers
>> instead.
>> > > Some engineers at Google have come to the conclusion that using
>> > > requireddoes more harm than good; they prefer to use only
>> > > optional and repeated. However, this view is not universal."
>> > >
>> > > Thoughts?
>> > >
>> > > Thanks,
>> > > Chris Trezzo
>> >
>> >
>>



-- 
Todd Lipcon
Software Engineer, Cloudera

Re: Use of required keyword in protobuf messages

[Gregory Chanan <gc...@cloudera.com>]

Good question, Lars.

I'm going off of the proposal (
http://wiki.apache.org/hadoop/Hbase/HBaseWireCompatibility
) and the slides at the meetup (
http://files.meetup.com/1350427/wire-compat%20%281%29.pptx).  If I missed
anything, I apologize.

Those list the minimum requirements to support the use cases and goals on
slides 4-6.  So, as written, there is no *guarantee* of support for rolling
upgrade between major versions.

Of course, there is nothing stopping us from adding that guarantee.  Or
from just guaranteeing it for specific versions (e.g. 0.96 to 0.98 in the
same manner as 0.92 to 0.94).  It should be much easier to achieve the
latter than in the past, once everything has been protobufed.  Perhaps we
should discuss in another thread?

So I'll withdraw my point that perhaps we could treat server-server and
client-server communication differently, but still submit the point that we
don't need to keep required fields around forever.

Greg

On Tue, Aug 7, 2012 at 5:23 PM, lars hofhansl <lh...@yahoo.com> wrote:

> Huh?
>
> I thought we're allowing for a rolling upgrade between major versions.
> If server/server is only wire compatible across a minor version that won't
> be possible.
>
> (this might have been mentioned in the initial discussion we all had about
> this a while back and I might have just forgotten)
>
>
> -- Lars
>
>
> ----- Original Message -----
> From: Gregory Chanan <gc...@cloudera.com>
> To: dev@hbase.apache.org
> Cc:
> Sent: Tuesday, August 7, 2012 1:33 PM
> Subject: Re: Use of required keyword in protobuf messages
>
> I agree with most of the comments here, particularly that at some point we
> should go through and review all the "required" fields.
>
> Another thing to keep in mind is that we have only guaranteed the following
> are compatible:
> - Client/Server across 1 major version
> - Server/Server different minor versions
>
> So we don't need to keep required fields for eons, necessarily.  And it may
> make sense to be looser about allowing "required" for server/server
> communication than for client/server.
>
> Greg
>
> On Tue, Aug 7, 2012 at 11:39 AM, Devaraj Das <dd...@hortonworks.com> wrote:
>
> > I too tend to make fields optional unless I am really convinced that the
> > field would live for eons. I agree with Google's philosophy in that
> regard.
> >
> > On Aug 6, 2012, at 3:39 PM, Chris Trezzo wrote:
> >
> > > Hi All,
> > >
> > > I was looking through the .proto files and noticed there are a lot of
> > > fields that are marked as required. I am by no means a protobuf expert,
> > but
> > > I was wondering what advantage do we actually get in making fields
> > required?
> > >
> > > I understand that if we don't use the required keyword we would have to
> > > implement custom application logic, but the flexibility we gain from
> > having
> > > all the fields optional seems to outweigh that work. In addition, we
> will
> > > already have to add logic to HBase to handle version compatibility, so
> it
> > > seems natural to implement the required logic as part of that layer.
> This
> > > would allow us to change or delete any message field and maintain wire
> > > compatibility.
> > >
> > > Quote from the protobuf language guide (
> > > https://developers.google.com/protocol-buffers/docs/proto):
> > >
> > > "*Required Is Forever* You should be very careful about marking fields
> as
> > > required. If at some point you wish to stop writing or sending a
> required
> > > field, it will be problematic to change the field to an optional field
> –
> > > old readers will consider messages without this field to be incomplete
> > and
> > > may reject or drop them unintentionally. You should consider writing
> > > application-specific custom validation routines for your buffers
> instead.
> > > Some engineers at Google have come to the conclusion that using
> > > requireddoes more harm than good; they prefer to use only
> > > optional and repeated. However, this view is not universal."
> > >
> > > Thoughts?
> > >
> > > Thanks,
> > > Chris Trezzo
> >
> >
>

Re: Use of required keyword in protobuf messages

[lars hofhansl <lh...@yahoo.com>]

Huh?

I thought we're allowing for a rolling upgrade between major versions.
If server/server is only wire compatible across a minor version that won't be possible.

(this might have been mentioned in the initial discussion we all had about this a while back and I might have just forgotten)


-- Lars


----- Original Message -----
From: Gregory Chanan <gc...@cloudera.com>
To: dev@hbase.apache.org
Cc: 
Sent: Tuesday, August 7, 2012 1:33 PM
Subject: Re: Use of required keyword in protobuf messages

I agree with most of the comments here, particularly that at some point we
should go through and review all the "required" fields.

Another thing to keep in mind is that we have only guaranteed the following
are compatible:
- Client/Server across 1 major version
- Server/Server different minor versions

So we don't need to keep required fields for eons, necessarily.  And it may
make sense to be looser about allowing "required" for server/server
communication than for client/server.

Greg

On Tue, Aug 7, 2012 at 11:39 AM, Devaraj Das <dd...@hortonworks.com> wrote:

> I too tend to make fields optional unless I am really convinced that the
> field would live for eons. I agree with Google's philosophy in that regard.
>
> On Aug 6, 2012, at 3:39 PM, Chris Trezzo wrote:
>
> > Hi All,
> >
> > I was looking through the .proto files and noticed there are a lot of
> > fields that are marked as required. I am by no means a protobuf expert,
> but
> > I was wondering what advantage do we actually get in making fields
> required?
> >
> > I understand that if we don't use the required keyword we would have to
> > implement custom application logic, but the flexibility we gain from
> having
> > all the fields optional seems to outweigh that work. In addition, we will
> > already have to add logic to HBase to handle version compatibility, so it
> > seems natural to implement the required logic as part of that layer. This
> > would allow us to change or delete any message field and maintain wire
> > compatibility.
> >
> > Quote from the protobuf language guide (
> > https://developers.google.com/protocol-buffers/docs/proto):
> >
> > "*Required Is Forever* You should be very careful about marking fields as
> > required. If at some point you wish to stop writing or sending a required
> > field, it will be problematic to change the field to an optional field –
> > old readers will consider messages without this field to be incomplete
> and
> > may reject or drop them unintentionally. You should consider writing
> > application-specific custom validation routines for your buffers instead.
> > Some engineers at Google have come to the conclusion that using
> > requireddoes more harm than good; they prefer to use only
> > optional and repeated. However, this view is not universal."
> >
> > Thoughts?
> >
> > Thanks,
> > Chris Trezzo
>
>

Re: Use of required keyword in protobuf messages

[Gregory Chanan <gc...@cloudera.com>]

I agree with most of the comments here, particularly that at some point we
should go through and review all the "required" fields.

Another thing to keep in mind is that we have only guaranteed the following
are compatible:
- Client/Server across 1 major version
- Server/Server different minor versions

So we don't need to keep required fields for eons, necessarily.  And it may
make sense to be looser about allowing "required" for server/server
communication than for client/server.

Greg

On Tue, Aug 7, 2012 at 11:39 AM, Devaraj Das <dd...@hortonworks.com> wrote:

> I too tend to make fields optional unless I am really convinced that the
> field would live for eons. I agree with Google's philosophy in that regard.
>
> On Aug 6, 2012, at 3:39 PM, Chris Trezzo wrote:
>
> > Hi All,
> >
> > I was looking through the .proto files and noticed there are a lot of
> > fields that are marked as required. I am by no means a protobuf expert,
> but
> > I was wondering what advantage do we actually get in making fields
> required?
> >
> > I understand that if we don't use the required keyword we would have to
> > implement custom application logic, but the flexibility we gain from
> having
> > all the fields optional seems to outweigh that work. In addition, we will
> > already have to add logic to HBase to handle version compatibility, so it
> > seems natural to implement the required logic as part of that layer. This
> > would allow us to change or delete any message field and maintain wire
> > compatibility.
> >
> > Quote from the protobuf language guide (
> > https://developers.google.com/protocol-buffers/docs/proto):
> >
> > "*Required Is Forever* You should be very careful about marking fields as
> > required. If at some point you wish to stop writing or sending a required
> > field, it will be problematic to change the field to an optional field –
> > old readers will consider messages without this field to be incomplete
> and
> > may reject or drop them unintentionally. You should consider writing
> > application-specific custom validation routines for your buffers instead.
> > Some engineers at Google have come to the conclusion that using
> > requireddoes more harm than good; they prefer to use only
> > optional and repeated. However, this view is not universal."
> >
> > Thoughts?
> >
> > Thanks,
> > Chris Trezzo
>
>

Re: Use of required keyword in protobuf messages

[Devaraj Das <dd...@hortonworks.com>]

I too tend to make fields optional unless I am really convinced that the field would live for eons. I agree with Google's philosophy in that regard.

On Aug 6, 2012, at 3:39 PM, Chris Trezzo wrote:

> Hi All,
> 
> I was looking through the .proto files and noticed there are a lot of
> fields that are marked as required. I am by no means a protobuf expert, but
> I was wondering what advantage do we actually get in making fields required?
> 
> I understand that if we don't use the required keyword we would have to
> implement custom application logic, but the flexibility we gain from having
> all the fields optional seems to outweigh that work. In addition, we will
> already have to add logic to HBase to handle version compatibility, so it
> seems natural to implement the required logic as part of that layer. This
> would allow us to change or delete any message field and maintain wire
> compatibility.
> 
> Quote from the protobuf language guide (
> https://developers.google.com/protocol-buffers/docs/proto):
> 
> "*Required Is Forever* You should be very careful about marking fields as
> required. If at some point you wish to stop writing or sending a required
> field, it will be problematic to change the field to an optional field –
> old readers will consider messages without this field to be incomplete and
> may reject or drop them unintentionally. You should consider writing
> application-specific custom validation routines for your buffers instead.
> Some engineers at Google have come to the conclusion that using
> requireddoes more harm than good; they prefer to use only
> optional and repeated. However, this view is not universal."
> 
> Thoughts?
> 
> Thanks,
> Chris Trezzo

Re: Use of required keyword in protobuf messages

[Jesse Yates <je...@gmail.com>]

On Tue, Aug 7, 2012 at 12:47 AM, Stack <st...@duboce.net> wrote:

> On Mon, Aug 6, 2012 at 11:39 PM, Chris Trezzo <ct...@gmail.com> wrote:
> > Some engineers at Google have come to the conclusion that using
> > requireddoes more harm than good; they prefer to use only
> > optional and repeated. However, this view is not universal."
> >
> > Thoughts?
> >
>
> How hard adding the logic Chris?  Any 'costs' associated.
>
> St.Ack
>

I've found (in my limited experience) that it ends up being a single
helper/util function per message, though at worst it can be one helper per
element in the message. The latter comes about if you have an optional
element that also has some optional elements. In that case you need to make
sure that (1) the message content there and (2) enough of the content is
there.

Fortunately, this is usually a pretty fast check, just checking the
presence of fields per RPC (and each object has generated has___() methods
for this checking), so its not too terrible since the boolean checks are
pretty fast (and using generalized RPC already has a bunch of overhead too
:).

This can theoretically lead to a proliferation of helper functions that
validate message/message contents for lots of different cases. In reality
though, I think this ends up being at most 2-3 per message type (at
_worst_) given then relative simplicity of our RPCs. If we go with this, we
will probably have to a have a helper/validator cleanup when its gets out
of control.

-------------------
Jesse Yates
@jesse_yates
jyates.github.com

Re: Use of required keyword in protobuf messages

[Andrew Purtell <ap...@apache.org>]

On Mon, Aug 6, 2012 at 11:39 PM, Chris Trezzo <ct...@gmail.com> wrote:
> Some engineers at Google have come to the conclusion that using
> required does more harm than good; they prefer to use only
> optional and repeated. However, this view is not universal."
>
> Thoughts?

I agree we should review the use of 'required'. Now's the time to make
modifications before we're locked in for cross-version compatibility.

Best regards,

   - Andy

Problems worthy of attack prove their worth by hitting back. - Piet
Hein (via Tom White)

Re: Use of required keyword in protobuf messages

[Stack <st...@duboce.net>]

On Mon, Aug 6, 2012 at 11:39 PM, Chris Trezzo <ct...@gmail.com> wrote:
> Some engineers at Google have come to the conclusion that using
> requireddoes more harm than good; they prefer to use only
> optional and repeated. However, this view is not universal."
>
> Thoughts?
>

How hard adding the logic Chris?  Any 'costs' associated.

St.Ack

Re: Use of required keyword in protobuf messages

[Jimmy Xiang <jx...@cloudera.com>]

Hi Chris,

At first, I agree. If a field is not absolutely required, we should
make it optional.

However, some fields may be required.  For example, to open a region,
you need to specify the region to be opened.

In case a required field is not required in the future any more, due
to some big architecture/design change,
we can use a deprecation window.

Thanks,
Jimmy

On Mon, Aug 6, 2012 at 3:39 PM, Chris Trezzo <ct...@gmail.com> wrote:
> Hi All,
>
> I was looking through the .proto files and noticed there are a lot of
> fields that are marked as required. I am by no means a protobuf expert, but
> I was wondering what advantage do we actually get in making fields required?
>
> I understand that if we don't use the required keyword we would have to
> implement custom application logic, but the flexibility we gain from having
> all the fields optional seems to outweigh that work. In addition, we will
> already have to add logic to HBase to handle version compatibility, so it
> seems natural to implement the required logic as part of that layer. This
> would allow us to change or delete any message field and maintain wire
> compatibility.
>
> Quote from the protobuf language guide (
> https://developers.google.com/protocol-buffers/docs/proto):
>
> "*Required Is Forever* You should be very careful about marking fields as
> required. If at some point you wish to stop writing or sending a required
> field, it will be problematic to change the field to an optional field –
> old readers will consider messages without this field to be incomplete and
> may reject or drop them unintentionally. You should consider writing
> application-specific custom validation routines for your buffers instead.
> Some engineers at Google have come to the conclusion that using
> requireddoes more harm than good; they prefer to use only
> optional and repeated. However, this view is not universal."
>
> Thoughts?
>
> Thanks,
> Chris Trezzo