[
https://issues.apache.org/jira/browse/HDFS-4403?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13554484#comment-13554484
]
Todd Lipcon commented on HDFS-4403:
-----------------------------------
I don't really follow...
'optional' is generally used in protobuf for adding new features compatibly.
Old clients don't know about it, so they don't care. New clients already have
to deal with old servers which don't set the field, so they have back-compat
paths (like the one I'm adding here).
If there's an optional field, and a client assumes it's always set, then it's a
bug, I agree. That's why the patch is calling hasChecksumType() here and going
to a fallback (compatibility) path.
In this case, the options are:
- old client, old server: there's a bug. We can't go back in time and change
the code.
- new client, new server: new server sets checksum type. new client reads it.
Everything works great. (HDFS-3177)
- new client, old server: old server doesn't set the type. HDFS-3177 set a
default to CRC32, which wasn't correct. This patch instead fixes that to
fallback to another (compatible) method to determine the type, rather than
defaulting to some arbitrary choice.
- old client, new server: new server sets type. old client ignores it. Again,
can't go back in time to change the old client, but nothing breaks.
If in the future we change the server to not set a type, then it's our own
fault that old clients wouldn't know what to do. That's just life. This is how
you do versioning with protobuf: see the Language Guide at
https://developers.google.com/protocol-buffers/docs/proto#updating :
{quote}
Any new fields that you add should be optional or repeated. This means that any
messages serialized by code using your "old" message format can be parsed by
your new generated code, as they won't be missing any required elements. You
should set up sensible default values for these elements so that new code can
properly interact with messages generated by old code. Similarly, messages
created by your new code can be parsed by your old code: old binaries simply
ignore the new field when parsing. However, the unknown fields are not
discarded, and if the message is later serialized, the unknown fields are
serialized along with it – so if the message is passed on to new code, the new
fields are still available. Note that preservation of unknown fields is
currently not available for Python.
{quote}
Here there is no "sensible default value", so we use a "sensible fallback code
path" instead.
> DFSClient can infer checksum type when not provided by reading first byte
> -------------------------------------------------------------------------
>
> Key: HDFS-4403
> URL: https://issues.apache.org/jira/browse/HDFS-4403
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: hdfs-client
> Affects Versions: 2.0.2-alpha
> Reporter: Todd Lipcon
> Assignee: Todd Lipcon
> Priority: Minor
> Attachments: hdfs-4403.txt
>
>
> HDFS-3177 added the checksum type to OpBlockChecksumResponseProto, but the
> new protobuf field is optional, with a default of CRC32. This means that this
> API, when used against an older cluster (like earlier 0.23 releases) will
> falsely return CRC32 even if that cluster has written files with CRC32C. This
> can cause issues for distcp, for example.
> Instead of defaulting the protobuf field to CRC32, we can leave it with no
> default, and if the OpBlockChecksumResponseProto has no checksum type set,
> the client can send OP_READ_BLOCK to read the first byte of the block, then
> grab the checksum type out of that response (which has always been present)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
