[ 
https://issues.apache.org/jira/browse/YARN-6064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15805717#comment-15805717
 ] 

Varun Saxena commented on YARN-6064:
------------------------------------

Thanks [~rohithsharma] for the patch. Had a glance at the patch. Few comments.

# We are querying flow context while querying flow apps. Is extra peek in app 
to flow table required ? Cant we supply flow run ID as well from client ?
# I think the tests require APPLICATION_CREATED_EVENT to be passed for an entry 
in app to flow table to be made. We are relying on it in code.
# Are changes in GenericEntityReader required ? 
# In ApplicationEntityReader, below piece of code has been added. Here, we 
check for getFilters() being null and then inside if block dereference filters 
by calling getFilters().getFromId(). What if getFilters() is null ? It may not 
be null in code flow (havent checked) but this should still be fixed. I do not 
think we need to pass fromId to ApplicationRowKeyPrefix in this case.  
{code}
369         if (getFilters() == null || getFilters().getFromId() == null) {
370           applicationRowKeyPrefix = new ApplicationRowKeyPrefix(
371               context.getClusterId(), context.getUserId(), 
context.getFlowName(),
372               context.getFlowRunId(), getFilters().getFromId());
{code}

An observation : fromId is inclusive as start row in HBase is inclusive. Let us 
say for pagination case I first get app100-app91. Then from UI side you will 
send fromId as app91 or you expect at the client side to find out the next app 
as app90 and query ? Or client needs to drop the extra record ? We need to make 
this clear in the documentation whenever we write it.

I will invoke the build after committing YARN-5585 and do a more detailed 
review after QA report.

> Support fromId for flowRuns and flow/flowRun apps REST API's
> ------------------------------------------------------------
>
>                 Key: YARN-6064
>                 URL: https://issues.apache.org/jira/browse/YARN-6064
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelinereader
>            Reporter: Rohith Sharma K S
>            Assignee: Rohith Sharma K S
>              Labels: yarn-5355-merge-blocker
>         Attachments: YARN-6064-YARN-5355.0001.patch
>
>
> Splitting out JIRA YARN-6027 for pagination support for flowRuns, flow apps 
> and flow run apps. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to