[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15127812#comment-15127812
]
Varun Saxena commented on YARN-4446:
------------------------------------
bq. I think the rule of thumb is, the impact of any changes to filters or data
to retrieve should be limited to TimelineReaderWebServices and *EntityReaders,
and should not result in changes in any other place
Yes. Changes will be in WebServices, TimelineReaderManager and Entity readers.
{quote}
(TimelineContext.java)
nit: it might be slightly better to call the other constructor
(TimelineReaderContext.java)
nit: same as TimelineContext
{quote}
Ok.
bq. Regarding common default behavior we have throughout the code base, should
we move them to TimelineEntityFilters and TimelineDataToRetrieve respectively
and encapsulate them? The examples include things like DEFAULT_LIMIT and
behavior around default data to retrieve if the user didn't specify any.
That's a good suggestion. We can set default values in constructor of
TimelineEntityFilters if value is null.
bq. why is this copy necessary?
We are using context to fill UID after we have retrieved entities from backend
storage implementation. The storage implementation can modify the context
within and if we use the same reference, the code to fill UID would rely on
storage implementation not changing it in a manner where it breaks UID filling
code. Or the storage layer may not change it at all which can create
inconsistency in result based on storage implementation. For instance, HBase
implementation modifies the context(fills flow context) but FS implementation
does not. This would mean that when we create UID, it will be with/without flow
depending on storage implementation
So for this separation of UID creation code and storage, I created a copy.
However, with HBase and FS implementations, this does not cause any functional
impact. The code will work(albeit with different UIDs' formed) even if we do
not create a copy.
Thoughts ?
bq. The javadoc that's added here is great. My only question is, where should
we have the detailed description of the filters and data to retrieve? Should we
put them in TimelineDataToRetrieve and TimelineEntityFilters instead?
I did think about it. Went with this because a user who wants to implement for
another storage will refer to TimelineReader API and will get all the info
there itself.
We can however in the javadoc ask them to refer to TimelineDataToRetrieve and
TimelineEntityFilters classes as well.
Where should we add it in these classes though because we have private fields ?
So should we have it documented against getters of these classes.
Or probably class javadoc for TimelineDataToRetrieve and TimelineEntityFilters
can contain the whole info.
Thoughts ?
> Refactor reader API for better extensibility
> --------------------------------------------
>
> Key: YARN-4446
> URL: https://issues.apache.org/jira/browse/YARN-4446
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Affects Versions: YARN-2928
> Reporter: Varun Saxena
> Assignee: Varun Saxena
> Labels: yarn-2928-1st-milestone
> Attachments: YARN-4446-YARN-2928.01.patch,
> YARN-4446-YARN-2928.02.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
