[
https://issues.apache.org/jira/browse/YARN-710?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13664665#comment-13664665
]
Siddharth Seth commented on YARN-710:
-------------------------------------
bq. bq. Vinod, Sid, given the discussion, are we OK with the current patch, and
what is missing is what will be the public API for this?
Yep. The public API is missing. Comments on the patch itself.
- The new methods should be marked "@Private"
- Should a runtime exception be thrown in case of a reflection error. Limit the
scope of the IOException to errors reading from / writing to the stream.
- I'm not sure if this is possible, but can the Proto class be figured out in a
different manner - directly constructing the classname or from the return type
of the getProto method, instead of instantiating *PBImpl and relying on what is
returned by getProto.
bq. If we don't want to support compatibility of disk serialization for some
records, how do you identify the supported ones without a tag interface? I
would expect the write call to fail if it is not supported the the given record.
bq. I'm OK with having an additional public API class to do the ser/deser of
'supported' records. Still I think such class should delegate to the
RecordFactory as the record factory knows about the underlaying implementation
and can process the ser/deser leveraging the implementation.
I don't think tagging is required for the public API. That can just be
determined by what is exposed in the helper library. Like you said, this
library can make use of the changes being added in this patch.
Related, PB offers other means to create recrods - such as
Builder.mergeDelimitedFrom. What the patch does is probably what it should be -
i.e. leaves the responsibility of reading/writing multiple records to the user,
but would like to here your thoughts on using mergeFrom / mergeDelimitedFrom.
Alternately, and I'd like to know what others think, does it make sense for
these methods to work with Protos directly. Something like
ApplicationIdProto serialize(ApplicationId)
ApplicationId deserialize(ApplicationIdProto)
> Add to ser/deser methods to RecordFactory
> -----------------------------------------
>
> Key: YARN-710
> URL: https://issues.apache.org/jira/browse/YARN-710
> Project: Hadoop YARN
> Issue Type: Bug
> Components: api
> Affects Versions: 2.0.4-alpha
> Reporter: Alejandro Abdelnur
> Assignee: Alejandro Abdelnur
> Attachments: YARN-710.patch
>
>
> I order to do things like AMs failover and checkpointing I need to serialize
> app IDs, app attempt IDs, containers and/or IDs, resource requests, etc.
> Because we are wrapping/hiding the PB implementation from the APIs, we are
> hiding the built in PB ser/deser capabilities.
--
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
