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

Zhijie Shen commented on YARN-3049:
-----------------------------------

[~gtCarrera9], thanks for review. I've addressed most of your comments in the 
new patch exception followings:

bq. However, I still incline to proceed the changes in this JIRA so that we can 
speed up consolidating our POC patches.

Exactly.

bq. Reader interface: use TimelineCollectorContext to package reader arguments?

Yeah, I can see the rationale behind it, but maybe it's not 
TimelineCollectorContext. As I see a lot of arguments for the reader interface 
(as well as the writer one) and the potential signature change in future (e.g, 
adding newApp in this patch), I start to think of grouping the primitive 
arguments, shielding them in some category object, such as EntityContext, 
EntityFilters, Opts and so on, and using these as the arguments of the 
interface instead. Therefore, if we want to add newApp here, we don't really 
need to change the method signature, but add a getter/setter in Opts. Please 
let me know how you think about the idea. I can file another jira to deal with 
the issue.

bq. We're now performing filters by ourselves in memory. I'm wondering if it 
will be more efficient to translate some of our filter specifications into 
HBase filters?

That sounds a good idea, which should potentially improve the read performance. 
Let me do some investigation how to map our filter into HBase filter and push 
it to the backend. Given it may be a non-trivial work, can we get this patch in 
and follow up the filter change in another jira just in case?

bq. Add a specific test in TestHBaseTimelineWriterImpl for App2FlowTable?

In fact, it has been tested. I change the write path by letting newApp = true, 
and check if we can query the entity successfully without giving the 
flow/flowRun explicitly. However, I didn't do much assertion around the fields 
of retrieved entities, because I consider of deferring this work together with 
rewriting the whole HBase backend unit test. The current tests are too 
preliminary to capture the potential bugs around DB operations.

> [Storage Implementation] Implement storage reader interface to fetch raw data 
> from HBase backend
> ------------------------------------------------------------------------------------------------
>
>                 Key: YARN-3049
>                 URL: https://issues.apache.org/jira/browse/YARN-3049
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Sangjin Lee
>            Assignee: Zhijie Shen
>         Attachments: YARN-3049-WIP.1.patch, YARN-3049-WIP.2.patch, 
> YARN-3049-WIP.3.patch, YARN-3049-YARN-2928.2.patch, 
> YARN-3049-YARN-2928.3.patch
>
>
> Implement existing ATS queries with the new ATS reader design.



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

Reply via email to