[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15369753#comment-15369753 ] Hudson commented on YARN-4446: -- SUCCESS: Integrated in Hadoop-trunk-Commit #10074 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/10074/]) YARN-4446. Refactor reader API for better extensibility (Varun Saxena (sjlee: rev 9cb1287e9b8425f91de925f411c3c2a8fa9fe2a3) * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineReaderContext.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/GenericEntityReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/storage/TestFileSystemTimelineReaderImpl.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineReaderUtils.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineDataToRetrieve.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/ApplicationEntityReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/HBaseTimelineReaderImpl.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineReaderManager.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineReaderWebServicesUtils.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/TimelineEntityReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/FileSystemTimelineReaderImpl.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/FlowActivityEntityReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/TimelineReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/storage/flow/TestHBaseStorageFlowRun.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/TimelineEntityReaderFactory.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/storage/reader/FlowRunEntityReader.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/storage/flow/TestHBaseStorageFlowActivity.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/storage/TestHBaseTimelineStorage.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineReaderWebServices.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/TimelineEntityFilters.java > 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 > Fix For: YARN-2928 > > Attachments: YARN-4446-YARN-2928.01.patch, > YARN-4446-YARN-2928.02.patch, YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsub
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15131564#comment-15131564 ] Varun Saxena commented on YARN-4446: Thanks [~sjlee0] for the review and commit. > 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 > Fix For: YARN-2928 > > Attachments: YARN-4446-YARN-2928.01.patch, > YARN-4446-YARN-2928.02.patch, YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15130974#comment-15130974 ] Sangjin Lee commented on YARN-4446: --- +1. I'll commit it soon. Please let me know now if you have any additional feedback. > 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, YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15129767#comment-15129767 ] Varun Saxena commented on YARN-4446: The 3 checkstyle issues cant be fixed. 2 of them are related to param number and one is with import which is required for javadoc. Will have to ignore them. javadocs do not look to be related. We will fix all these pending javadocs in YARN-4409. > 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, YARN-4446-YARN-2928.03.patch, > YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15129203#comment-15129203 ] Sangjin Lee commented on YARN-4446: --- There seem to be new javadoc errors and a fixable checkstyle violation. Could you please take a look? > 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, YARN-4446-YARN-2928.03.patch, > YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15129140#comment-15129140
]
Hadoop QA commented on YARN-4446:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 0s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 3m 32s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m
58s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 18s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 19s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
16s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 26s
{color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
16s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
34s {color} | {color:green} YARN-2928 passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 15s
{color} | {color:red} hadoop-yarn-server-timelineservice in YARN-2928 failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 18s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
19s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 14s
{color} | {color:green} the patch passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 14s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 16s
{color} | {color:green} the patch passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 16s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 13s
{color} | {color:red}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice:
patch generated 3 new + 52 unchanged - 47 fixed = 55 total (was 99) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 23s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
13s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
42s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 11s
{color} | {color:red} hadoop-yarn-server-timelineservice in the patch failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 38s
{color} | {color:green}
hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-jdk1.7.0_91
with JDK v1.7.0_91 generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 16s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 17s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 22s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} asfli
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15128962#comment-15128962
]
Hadoop QA commented on YARN-4446:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 0s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 4m 55s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
29s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 19s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 21s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
18s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 28s
{color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
17s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
37s {color} | {color:green} YARN-2928 passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 17s
{color} | {color:red} hadoop-yarn-server-timelineservice in YARN-2928 failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 20s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 10s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
23s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 15s
{color} | {color:green} the patch passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 15s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 18s
{color} | {color:green} the patch passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 18s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 16s
{color} | {color:red}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice:
patch generated 9 new + 52 unchanged - 47 fixed = 61 total (was 99) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 26s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
14s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
48s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 15s
{color} | {color:red} hadoop-yarn-server-timelineservice in the patch failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 52s
{color} | {color:green}
hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-jdk1.7.0_91
with JDK v1.7.0_91 generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 23s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 20s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} asfl
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15128870#comment-15128870 ] Varun Saxena commented on YARN-4446: Addressed comments and attached a new patch. > 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, YARN-4446-YARN-2928.03.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15128590#comment-15128590
]
Sangjin Lee commented on YARN-4446:
---
{quote}
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.
{quote}
Understood. I'm fine with the way it is then. I was curious as to the reason
for creating new instances.
{quote}
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 ?
{quote}
That's what I am thinking too. We can document it in the class javadoc for
TimelineDataToRetrieve and TimelineEntityFilters, and in TimelineReader we can
refer to those javadocs.
> 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)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
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)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15126988#comment-15126988
]
Sangjin Lee commented on YARN-4446:
---
Thanks [~varun_saxena] for your patch!
I generally agree with the direction you are taking with this patch. 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. I think that is the case with
the patch, but can you also confirm?
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.
(TimelineContext.java)
- nit: it might be slightly better to call the other constructor
(TimelineReaderContext.java)
- nit: same as TimelineContext
(TimelineReaderManager.java)
- l.127: why is this copy necessary?
(TimelineReader.java)
- 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?
> 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)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15126552#comment-15126552 ] Varun Saxena commented on YARN-4446: Kindly review. 3 of the checkstyle issues cant be fixed. > 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)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15124465#comment-15124465
]
Hadoop QA commented on YARN-4446:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 0s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
55s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 19s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 21s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
18s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 30s
{color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
18s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
43s {color} | {color:green} YARN-2928 passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 18s
{color} | {color:red} hadoop-yarn-server-timelineservice in YARN-2928 failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 20s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
21s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 15s
{color} | {color:green} the patch passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 15s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 18s
{color} | {color:green} the patch passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 18s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 14s
{color} | {color:red}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice:
patch generated 3 new + 53 unchanged - 46 fixed = 56 total (was 99) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 25s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
14s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
45s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 15s
{color} | {color:red} hadoop-yarn-server-timelineservice in the patch failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 47s
{color} | {color:green}
hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-jdk1.7.0_91
with JDK v1.7.0_91 generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 1s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 4s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 22m 41s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoo
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[ https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15124442#comment-15124442 ] Varun Saxena commented on YARN-4446: Fixed one checkstyle issue which could be fixed. > 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)
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15124193#comment-15124193
]
Hadoop QA commented on YARN-4446:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 0s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 12m
19s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 16s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 18s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
19s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 29s
{color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
21s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
37s {color} | {color:green} YARN-2928 passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 16s
{color} | {color:red} hadoop-yarn-server-timelineservice in YARN-2928 failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 18s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
19s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 13s
{color} | {color:green} the patch passed with JDK v1.8.0_66 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 16s
{color} | {color:green} the patch passed with JDK v1.7.0_91 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 16s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 13s
{color} | {color:red}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice:
patch generated 4 new + 53 unchanged - 46 fixed = 57 total (was 99) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 23s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
13s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
42s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 11s
{color} | {color:red} hadoop-yarn-server-timelineservice in the patch failed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 39s
{color} | {color:green}
hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-jdk1.7.0_91
with JDK v1.7.0_91 generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 15s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m 52s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.8.0_66. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m 0s
{color} | {color:green} hadoop-yarn-server-timelineservice in the patch passed
with JDK v1.7.0_91. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
20s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 25m 23s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/had
[jira] [Commented] (YARN-4446) Refactor reader API for better extensibility
[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15054318#comment-15054318
]
Varun Saxena commented on YARN-4446:
After YARN-3863, getEntities API would change to as under :
{code}
Set getEntities(String userId, String clusterId,
String flowId, Long flowRunId, String appId, String entityType,
Long limit, Long createdTimeBegin, Long createdTimeEnd,
Long modifiedTimeBegin, Long modifiedTimeEnd,
TimelineFilterList relatesTo, TimelineFilterList isRelatedTo,
TimelineFilterList infoFilters, TimelineFilterList configFilters,
TimelineFilterList metricFilters, TimelineFilterList eventFilters,
TimelineFilterList confsToRetrieve, TimelineFilterList metricsToRetrieve,
EnumSet fieldsToRetrieve) throws IOException;
{code}
The suggestion is to put related parameters into a separate class.
So limit, createdTimeBegin/End, modifiedTimeBegin/End, relatesTo, isRelatedTo,
info/config/metric/event filters can be boxed in one class named
{{TimelineEntityFilters}}.
Better name ?
Also, we can put userId, clusterId, flowId, flowRunId and appId in a class
named {{TimelineReaderContext}}.
metricToRetrieve, confsToRetrieve and fieldToRetrieve can be in a single class
named {{TimelineDataToRetrieve}}(say).
This would allow for better extensibility. Because if we add any other filter
or data to retrieve in future, we need not change the caller until and unless
we want to pass the new information. And hence would be far more easier to
extend.
> 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
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
