hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sangjin Lee (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3049) [Storage Implementation] Implement storage reader interface to fetch raw data from HBase backend
Date Fri, 31 Jul 2015 19:03:05 GMT

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

Sangjin Lee commented on YARN-3049:

Sorry [~zjshen] it took me a while to get to this. The patch looks pretty good actually. I
have one high level point I'd like to discuss with you, and several smaller comments.

I see that you added a new boolean argument in {{TimelineCollector.putEntity()}}, {{TimelineCollector.putEntities()}},
and {{TimelineWriter.write()}} to indicate we're dealing with a new app (and thus writing
to the app-to-flow table). I'm not sure whether that is really what we want to do. Can we
not detect and leverage the fact that we're dealing with an "application created" event and
trigger those actions instead of having an explicit argument that gets passed down all the
way from the clients? First, in this approach we would be completely relying on the client
code to specify this correctly. Secondly, I would argue that the fact that we need to detect
that we're introducing a new application and write to these tables is somewhat of an "implementation
detail" of the HBase writer. For example, other writers may not even care about that and have
no need for it. The fact that this detail leaks all the way to the callers is awkward at best.

My initial thinking of how to do this was inside {{HBaseTimelineWriterImpl}} on detecting
the application created event to trigger this action. What do you think?

- l.138: it might be better to use the type {{SortedSet}} or {{NavigableSet}} to make it explicit
we want ordering

- l.93: What does it mean to indicate newApp for a set of entities? What if the set of entities
contains bunch of different applications?

- See comments above; rather than relying on the boolean flag in the arguments, can we detect
the case of the application created event and do it?

- l.67: nit: I think the word "from" is needed there. It's just that the space was missing
between "result" and "from".

- l.33: nit: "both matches" -> "both match"

> [Storage Implementation] Implement storage reader interface to fetch raw data from HBase
> ------------------------------------------------------------------------------------------------
>                 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

View raw message