[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15858001#comment-15858001
]
Varun Saxena commented on YARN-4675:
Thanks [~Naganarasimha] for the patch. Looks pretty close. Few minor comments
# In TimelineConnector, connectionRetry should be annotated with
VisibleForTesting
# Can't we add a serviceInit method in AMRMClient which stores the value of
timeline service v2 enabled instead of adding an abstract method
registerTimelineV2Client.
# The changes here need to be reflected in timeline v2 documentation as well.
You want to do it here or another JIRA? As the change is very small, maybe we
can do it here itself. Thoughts?
# In YarnClientImpl, we check for YarnConfiguration#timelineServiceV2Enabled
and this will check for timeline service enabled again. We can use
YarnConfiguration#getTimelineServiceVersion instead. This can be done at other
places as well.
{code}
170 if (conf.getBoolean(YarnConfiguration.TIMELINE_SERVICE_ENABLED,
171 YarnConfiguration.DEFAULT_TIMELINE_SERVICE_ENABLED)
172 && !YarnConfiguration.timelineServiceV2Enabled(conf)) {
{code}
By the way can other checkstyle issues be fixed? Few exist from before though
and are appearing in report just because you renamed the variable.
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch,
> YARN-4675.v2.004.patch, YARN-4675.v2.005.patch, YARN-4675.v2.006.patch,
> YARN-4675.v2.007.patch, YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15856360#comment-15856360
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
20s{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 6 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
14s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 15m
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 16m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
12s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 5m
8s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 3m
5s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
20s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 2m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 13m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 13m
58s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 46s{color} | {color:orange} root: The patch generated 11 new + 744 unchanged
- 10 fixed = 755 total (was 754) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
6s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
16s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
50s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
40s{color} | {color:green} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 0 new + 4575 unchanged - 4 fixed = 4575 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
28s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} hadoop-yarn-server-tests in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} hadoop-yarn-applications-distributedshell in the
patch passed. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
40s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m
38s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{col
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15850794#comment-15850794 ] Varun Saxena commented on YARN-4675: Findbugs and test failure of TestTimelineClientV2Impl is related. We need to set timeline service to 2.0 explicitly in the test. Also many checkstyle issues can be fixed as well. Can you fix them ? > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch, > YARN-4675.v2.004.patch, YARN-4675.v2.005.patch, YARN-4675.v2.006.patch, > YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15850686#comment-15850686
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
10s{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 6 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
58s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 14m
46s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 14m
15s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
47s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 1m
57s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m
20s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
17s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 2m
9s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 10m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 10m
35s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 48s{color} | {color:orange} root: The patch generated 20 new + 744 unchanged
- 10 fixed = 764 total (was 754) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 3m
15s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m
0s{color} | {color:red} The patch has 1 line(s) that end in whitespace. Use git
apply --whitespace=fix <>. Refer https://git-scm.com/docs/git-apply
{color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 1m
16s{color} | {color:red} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
39s{color} | {color:green} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 0 new + 4575 unchanged - 4 fixed = 4575 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
17s{color} | {color:green} hadoop-yarn-server-tests in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} hadoop-yarn-applications-distributedshell in the
patch passed. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 2m 37s{color}
| {color:red} hadoop-yarn-common in the patch failed. {color} |
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15834758#comment-15834758
]
Varun Saxena commented on YARN-4675:
# There is a bit of connection related duplicate code in TimelineClientImpl and
TimelineClientV2Impl#serviceInit. I think we can turn TimelineClientHelper into
a service (or a standalone class whose instance can be created) which can be
initialized and stopped (to do the cleanup). We can encapsulate all Jersey
client related stuff and connection retry logic in this class. If timeline
client implementations want to access something from this class, we can provide
relevant getters. I think this would be more cleaner.
# There are a lot of variables in both TimelineClientImpl and
TimelineClientV2Impl which need not be class member variables as they are not
referred to only during initialization.
# I agree in principle with the comment above i.e. to reduce the visibility of
TimelineClient*Impl constructor. I think we can move the classes to same
package as TimelineClient and TimelineV2Client. As TimelineClientImpl is
annotated as Private, this should be fine, backward compatibility wise as well.
# In MRAppMaster#RunningAppContext, the comment over getTimelineClient method
isn't suitable.
# ApplicationMaster Line 302, timelineV2Client need not be made visible for
testing.
# We will reuse TimelineClientConnectionRetry for V2 as well so I think its
fine to move it.
# TimelineV2Client, Line 35, we should say @see TimelineV2Client instead of
TimelineClient
# In TimelineClient we had the below javadoc which has been removed in the
patch. It was wrongly placed over contextAppId before. But the javadoc need not
be removed. It should be placed over method createTimelineClient.
{code}
49/**
50 * Create a timeline client. The current UGI when the user initialize
the
51 * client will be used to do the put and the delegation token
operations. The
52 * current user may use {@link UserGroupInformation#doAs} another
user to
53 * construct and initialize a timeline client if the following
operations are
54 * supposed to be conducted by that user.
55 */
{code}
# TimelineClient Line 42, it should be @see TimelineClient. In the preceding
line, maybe no need of space between time and line.
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch,
> YARN-4675.v2.004.patch, YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15834398#comment-15834398 ] Naganarasimha G R commented on YARN-4675: - test failures seems not related to the patch modifications and few applicable check style comments will address in next patch along with other comments. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch, > YARN-4675.v2.004.patch, YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15834343#comment-15834343
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
11s{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 5 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
14s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 12m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 12m
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
49s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 5m
17s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 9m
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m
18s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
16s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 2m
9s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 11m
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 11m
28s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 45s{color} | {color:orange} root: The patch generated 20 new + 722 unchanged
- 6 fixed = 742 total (was 728) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
2s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
15s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
33s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
38s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 4 new + 4575 unchanged - 4 fixed = 4579 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
29s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m
6s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 37s{color}
| {color:red} hadoop-yarn-server-tests in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m
56s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m
3s{color} | {color:green} hadoop-yarn-applications-distributedshell in the
patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 8m
53s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
37s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color}
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15832483#comment-15832483
]
Sangjin Lee commented on YARN-4675:
---
Thanks [~Naganarasimha]! I went over the latest patch, and have some comments.
Quite a few comments overlap with [~gtCarrera9]'s comments.
(TimelineClient.java)
- l.57: agree with [~gtCarrera9] that we want to make the constructor protected
(TimelineV2Client.java)
- I would move {{contextAppId}} to {{TimelineV2ClientImpl}}. It is used solely
internally by {{TimelineV2ClientImpl}} and it should be defined there.
- As a follow-up, we should drop the {{appId}} argument from the
{{TimelineV2Client}} constructor
- As a follow-up, we can drop the {{setContextAppId}} and {{getContextAppId}}
methods
- l.37: The blurb "Create a timeline client" sounds strange. We should remove
that sentence once it's moved.
- l.58: make this constructor protected too
- l.78,96: nit: let's use imports now that there is no ambiguity
(TimelineClientImpl.java)
- we should do a strong check of the timeline service version in
{{serviceInit()}} not to accept an invalid version
- l.89: this should be removed
- we should remove {{pollTimelineServiceAddress()}}
- we should remove {{initConnConfigurator()}}
- we should remove {{initSslConnConfigurator()}}
- we should remove {{serviceRetryInterval}}
- we should remove {{DEFAULT_TIMEOUT_CONN_CONFIGURATOR}}
- we should remove {{setTimeouts()}}
- we should remove {{DEFAULT_SOCKET_TIMEOUT}}
- {{TimelineClientConnectionRetry}} and {{TimelineJerseyRetryFilter}} are
duplicated with {{TimelineServiceHelper}}; we should determine where we need
them and remove duplication; I would remove them from {{TimelineServiceHelper}}
unless we think we will use them later for v.2
- l.562: we should drop the {{boolean v2}} argument
- we don't need a public {{setTimelineServiceAddress()}} method in v.1.x; I
would simply inline it
- in {{putEntities()}}, we can remove the check if we have it in
{{serviceInit()}}; were we always checking for == 1.5 by the way? So we don't
support 1.0 any more?
(TimelineServiceHelper.java)
- we should remove {{TimelineClientConnectionRetry}} and
{{TimelineJerseyRetryFilter}}
(TimelineV2ClientImpl.java)
- l.88: we should remove {{connectionRetry}}
- l.168: we should drop the {{boolean v2}} argument
(JobHistoryEventHandler.java)
- i believe we need to touch {{TestJobHistoryEventHandler}} too (the
{{JHEvenHandlerForTest}} class)
- l.332, 458: super nit: space before the curly brace
(TestJobHistoryEventHandler.java)
- is this all about testing timeline service v.1? note that
{{mockAppContext()}} now returns a v.1 client only
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch,
> YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830892#comment-15830892
]
Li Lu commented on YARN-4675:
-
Thanks [~Naganarasimha]! I took a look at the 003 patch. Not sure why but I
found some duplicated code in TimelineClientImpl with the newly introduced
helper and v2 impl. Detailed comments:
AMRMClient (and AMRMClientAsync):
- Maybe we'd like to be more clear about registerTimelineV2Client? This is
something new and quite different to v1 clients.
- Do we allow registering timeline clients when the AMRMClient's timeline
version is not set to 2? Maybe we should at least leave a warning/error there?
This can save some time when debugging a misconfigured cluster.
DistributedShell, ApplicationMaster.java:
DS is an app that we demo how to use a lot of YARN features, so maybe we want
to tidy up timeline client related code pieces a little bit...
- Can we unify all {{if}}s for publishing timeline event? We may want to have
centralized methods to dispatch timeline client calls to v1 and v2.
- Also, instead of checking if a timeline client is null, shall we use flags
like limelineServiceV2?
TimelineClient:
- Let's refer to TimelineV2Client in the Java doc for v2 use cases?
{code}
Creates an instance of the timeline v.1.x client.
{code}
- We may also want to update the class's javadoc to reflect API changes over
Hadoop 2. At least mention this class is for timeline v1.x ONLY.
TimelineV2Client:
- Same javadoc issue.
- Shall we close the constructor to protected since we've experienced some
unexpected calls to it in v1? Or at least add a testing only tag?
- Does client users need to know the context app id? If so, we may need to
slightly relax the visibility of getContextAppId?
- Why do we need a setter for context app id? Maybe we want to make this
information immutable for timeline clients? Do we allow reusing timeline v2
clients across multiple applications?
TimelineClientImpl:
- Why do we need RESOURCE_URI_STR_V2? We need to further polish
constructResURI as well.
- serviceRetryInterval is never used.
- Duplicated code for TimelineClientConnectionRetry and JerseyRetryFilter as
in TimelineServiceHelper.
- pollTimelineServiceAddress, initConnConfigurator never called? Duplicates
with V2.
- new ConnectionConfigurator in initSslConnConfigurator duplicates some code
in TimelineServiceHelper.
TimelineServiceHelper:
- There are two {{TimelineServiceHelper}}s in our codebase? One is really
trivial. Shall we merge them or eliminate one of them?
TimelineV2ClientImpl:
- connectionRetry is never used.
Not necessarily addressed in this JIRA, but to bring into attention: We have a
TimelineClient in YarnClientImpl. Shall we do this even though the cluster is
configured with ATS v2?
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch,
> YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830542#comment-15830542
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{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 3 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
16s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 14m
20s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 11m
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
53s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
27s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
51s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m
50s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
25s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 2m
51s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 12m
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 12m
32s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 57s{color} | {color:orange} root: The patch generated 18 new + 555 unchanged
- 2 fixed = 573 total (was 557) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 4m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
50s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 5m
45s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
46s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 4 new + 4575 unchanged - 4 fixed = 4579 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
58s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m
13s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 42s{color}
| {color:red} hadoop-yarn-server-tests in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 16m 28s{color}
| {color:red} hadoop-yarn-client in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m
3s{color} | {color:green} hadoop-yarn-applications-distributedshell in the
patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 8m
51s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
38s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830522#comment-15830522 ] Vrushali C commented on YARN-4675: -- I looked over the patch. In general it looks good to me. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch, > YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830319#comment-15830319 ] Varun Saxena commented on YARN-4675: Seems build has not been invoked. Will invoke it manually. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch, > YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830276#comment-15830276 ] Rohith Sharma K S commented on YARN-4675: - bq. i have tried to maintain V1 client as it is and change only V2 client, so that we can have modifications in trunk as well branch-2. Please share your thoughts on the same. As long as org.apache.hadoop.yarn.client.api.TimelineClient is abstracted and maintain compatibility for users, then we can commit to branch-2 as well. Btw, I have not seen patch yet, I will take a look. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675.v2.003.patch, > YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15829860#comment-15829860
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{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 3 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
17s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 12m
44s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 9m
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
49s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
2s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m
20s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
17s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 2m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 9m
19s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 9m
19s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 42s{color} | {color:orange} root: The patch generated 18 new + 555 unchanged
- 2 fixed = 573 total (was 557) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
21s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
35s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
38s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 4 new + 4575 unchanged - 4 fixed = 4579 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
29s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 12m
59s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 39s{color}
| {color:red} hadoop-yarn-server-tests in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m
24s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m
10s{color} | {color:green} hadoop-yarn-applications-distributedshell in the
patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m
16s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
43s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color}
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828929#comment-15828929
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{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 3 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
57s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 13m
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
12s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
24s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
50s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m
44s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
24s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m
12s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 12m
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 12m
14s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
1m 57s{color} | {color:orange} root: The patch generated 24 new + 473 unchanged
- 1 fixed = 497 total (was 474) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 3m
47s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 2m
43s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m
0s{color} | {color:red} The patch has 11 line(s) that end in whitespace. Use
git apply --whitespace=fix <>. Refer
https://git-scm.com/docs/git-apply {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Skipped patched modules with no Java source:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests
{color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 1m
33s{color} | {color:red} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
44s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common
generated 4 new + 4575 unchanged - 4 fixed = 4579 total (was 4579) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 3m
2s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m
53s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 51s{color}
| {color:red} hadoop-yarn-server-tests in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 17m
17s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 9m 50s{color}
| {color:red} hadoop-yarn-applications-distributedshell in the patch failed.
{color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m
55s{color} | {color:green} hadoop-mapreduce-client-app in the patch passed.
{color} |
| {color:green}+1{color} | {colo
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828783#comment-15828783 ] Varun Saxena commented on YARN-4675: bq. Still some cleaning up of TimelineClientImpl Missed this part of your comment. You can ignore my comment about code duplication across helper and client classes then. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828780#comment-15828780 ] Varun Saxena commented on YARN-4675: Thanks Naga for the patch. Had a brief look at the patch. # In RMContainerAllocator#getResources, we should be getting V2 Client from RunningAppContext to update the timeline address. # setTimelineServiceAddress(String) may not be required as an abstract method in TimelineClient class. # In ApplicationMaster in distributed shell, we have an empty if block with a check for timeline service v2. Can be removed or were you intending to code something there? # There is a still a bit of V2 code in TimelineClientImpl. Polling for timeline service address, one version of putObjects, etc. is used exclusively for ATSv2. # I think we can move all the connection related and retry related stuff to TimelineServiceHelper class. We still have it in TimelineClientImpl. TimelineURLConnectionFactory exists in both the classes for instance. We can probably give a more suitable name to this helper class as well if all we have in here is connection establishment and retry related code. # In ApplicationMaster#finish (Distributed shell code), we are stopping only v1 client. # In ApplicationMaster#finish, we check for not null only for v1 client while publishing some entities. This would mean on finish, v2 entities wont be published. We should make relevant checks. # Same problem as above in NMCallBackHandler#onContainerStarted. We can probably check for similar issues at other places in code too. # In JobHistoryEventHandler#serviceStop, we should call timelineV2Client.stop(); instead of timelineV2Client.start(); Can probably have a more detailed review upon next patch update. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828702#comment-15828702 ] Naganarasimha G R commented on YARN-4675: - bq. Given this JIRA will be committed to only trunk, then it should be fine. [~rohithsharma], i have tried to maintain V1 client as it is and change only V2 client, so that we can have modifications in trunk as well branch-2. Please share your thoughts on the same. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675.v2.002.patch, YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15803481#comment-15803481 ] Rohith Sharma K S commented on YARN-4675: - Given this JIRA will be committed to only trunk, then it should be fine. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15802756#comment-15802756 ] Sangjin Lee commented on YARN-4675: --- One implication of refactoring the interface is that code that uses the timeline client would need to be updated along with the API changes. MR and DS are not a problem, but other off-hadoop clients such as Tez would need to make this change when this change lands on trunk. I assume it is not a major problem, but just so that we are aware. cc [~rohithsharma] > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15762940#comment-15762940 ] Varun Saxena commented on YARN-4675: Yes, I think we can have new interface as TimelineV2Client and impl as TimelineV2ClientImpl. Also TimelineV2Client.createTimelineClient should be fine to get the v2 instance > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, yarn-5355-merge-blocker > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15762866#comment-15762866
]
Naganarasimha G R commented on YARN-4675:
-
Thanks for the discussion [~sjlee0], [~gtCarrera9] and [~varun_saxena] and
sorry for the delay.
few clarifications in the finalized approach:
# So we have finalized that there will be 2 different interfaces for ATSv1(&
1.5) and ATSV2, So shall i name the new interface for V2 as
{{TimelineV2Client}} and impl as {{TimelineV2ClientImpl}} ?
# {{TimelineV2Client.createTimelineClient}} would this interface be fine to get
the v2 instance ?
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15732837#comment-15732837 ] Varun Saxena commented on YARN-4675: I am fine with the separation as well. Probably it did not come out clearly in my previous comment but I was not suggesting that common parts need to be subclassed. I was infact saying that subclassing may not be possible. And common code can probably be put into another helper class which can be called via implementations. Agree that get and renew delegation token parts related to ATSv1 can be added/called in ATSv2, as and when required. If we do use delegation tokens this is likely to be exact same code though. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15730203#comment-15730203 ] Li Lu commented on YARN-4675: - For TimelineClientImpl, I'm totally fine to separate v1 and v2. I'm not worrying too much on code duplication for security related parts since they're yet to be finalized. For the rest part, I'm totally fine with separating them. Let me work on YARN-5974 to unblock this. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15729687#comment-15729687
]
Sangjin Lee commented on YARN-4675:
---
Let's separate the discussion on what to do with the interface
({{TimelineClient}}) from the implementations ({{TimelineClientImpl}}).
I looked at the current interface, and as [~varun_saxena] said the
{{*DelegationToken}} methods are potential common methods shared between v.1
and v.2. However, the rest of the methods are specific to either v.1 or v.2.
Duplicating the same methods are bit unsatisfactory, but having to declare
methods as unsupported in the implementation (the current state) is probably
uglier.
In the implementation, I agree with [~gtCarrera9] that there are ways to
isolate the common code and have the v.1 and v.2 impls reuse that code without
using subclassing.
What do others think?
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, oct16-medium
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15726783#comment-15726783 ] Li Lu commented on YARN-4675: - Created YARN-5974 for removing unnecessary references to TimelineClientImpl. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15726776#comment-15726776 ] Li Lu commented on YARN-4675: - I'm fine with separating the v1 and v2 interfaces. Right now mixing v1 and v2 interfaces in one interface looks pretty confusing to me. Since we've decided timeline v2 is not backward compatible at the very beginning, I think it's fine to let users choose between TimelineClient v1 and v2. bq. things that are referencing TimelineClientImpl directly today Yes, we should not directly refer to TimelineClientImpl in downstream usages. Shall I open a JIRA and remove all of them? bq. the facility for getting delegation token and renewing it would be common to both the clients. We would not want to repeat such large amounts of code in both V1 and V2 client implementations. That's certainly a very valid concern, and addressing this may bring in much discussions on security itself. My bottomline here is that let's *assume* any security facilities do not exist in timeline v2, and let's start the design from the scratch. We may then think about how to merge and reuse the code afterwards. For now, let's not think about maximize code reuse for timeline v1 and v2, especially for security? > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15726699#comment-15726699 ] Varun Saxena commented on YARN-4675: At a cursory glance, the patch posted seems to be at a high level, pretty near to what we had in mind when this was raised. I agree that we do not necessarily need a single interface however if we do that we would probably pull out connection related (URL connection factory, retry, etc.) code into a separate class. Also, the facility for getting delegation token and renewing it would be common to both the clients. We would not want to repeat such large amounts of code in both V1 and V2 client implementations. Maybe we can have a separate class for all this common stuff and method for posting entities in an interface. Implementations then can probably extend from this class. But then TimelineClient extends AbstractService and we would not want to break that. Maybe instead of extending the class containing the common code we can just use it as a helper class with an object of it in each implementation. Thoughts ? > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15726389#comment-15726389
]
Sangjin Lee commented on YARN-4675:
---
For your reference, things that are referencing {{TimelineClientImpl}} directly
today:
- JobHistoryFileReplayMapperV1 (MR)
- SimpleEntityWriterV1 (MR)
- TestDistributedShell (DS)
- TestDSAppMaster (DS)
- TestNMTimelinePublisher (node manager)
- TestTimelineWebServicesWithSSL (AHS)
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, oct16-medium
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15726372#comment-15726372
]
Sangjin Lee commented on YARN-4675:
---
I agree that we need to have a firm conclusion one way or another on this
before the next release.
There are two aspects to this: (1) reorganize {{TimelineClientImpl}} into v.1
specific code and v.2 specific code, and (2) create separate interfaces (APIs)
for v.1 and v.2.
IMO, (1) is not very controversial and that's what's captured in the current
patch. I think we should do it as it's more of an internal "implementation
detail". For the most part, clients should *not* use {{TimelineClientImpl}} or
other impl classes directly but rather stick with {{TimelineClient}}. I do see
some references to {{TimelineClientImpl}} in MR and distributed shell and what
not, and it might be good to address them to avoid using {{TimelineClientImpl}}
as much as possible.
On the other hand, (2) is a little bigger issue as it affects how clients will
implement using timeline service. The initial desire was to stick with a single
interface ({{TimelineClient}}) for both v.1 and v.2 because we wanted to
minimize the impact on client code. However, since we have new entity API, we
still had to have v.1-specific methods and v.2-specific methods. Therefore,
clients still need to do things like:
{code}
if (v.1) {
client.putEntities(...); // call the v.1 method
} else {
client.putEntities(...); // call the v.2 method
}
{code}
So retaining a single interface doesn't really help them much. There is also
the aspect of preparing the entities in a different way, depending on v.1 or
v.2.
So, in that sense, it doesn't seem that bad to separate the interface itself.
This is still a much more significant change because it would impact all the
client code (MR, DS, Tez, etc.). What are your thoughts on both aspects?
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, oct16-medium
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15712881#comment-15712881 ] Li Lu commented on YARN-4675: - Yes I agree we need to decide on this issue soon. +1 for reorganizing TimelineClientImpl. Do we also need to distinguish v2 APIs from timeline clients as well? As of now we will have timeline APIs for v1, v1.5, and v2 so I think it may be helpful to distinguish at least v2 APIs. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15712859#comment-15712859 ] Vrushali C commented on YARN-4675: -- As discussed in the call today, let's discuss if we need to do this *before* the next releases (open hadoop release as well as internal ones) coming up. Will be better to do the client side interfacing changes now rather than later. My thoughts are that this will make it cleaner to have separate client classes like the patch has. So I am +1 on reorganizing the TimeClientImpl into V1 and V2. cc [~gtCarrera9] [~sjlee0] [~varun_saxena] > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15623831#comment-15623831 ] Naganarasimha G R commented on YARN-4675: - Thanks [~vrushalic], Will update the patch at the earliest and also IIUC i was not introducing any new changes for the client but just organizing the code so i think it should be fine but anyways will upload the patch at the earliest. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15613546#comment-15613546
]
Hadoop QA commented on YARN-4675:
-
| (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:red}-1{color} | {color:red} patch {color} | {color:red} 0m 8s{color}
| {color:red} YARN-4675 does not apply to YARN-2928. Rebase required? Wrong
Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | YARN-4675 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12788479/YARN-4675-YARN-2928.v1.001.patch
|
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/13604/console |
| Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355, oct16-medium
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15613444#comment-15613444 ] Vrushali C commented on YARN-4675: -- Looked at this as part of the yarn meetup #4. I also discussed with [~sjlee0]. In the next weekly call, I think we should think over this and decide whether or not we should go ahead with this, sooner rather than later. This should be done before 3.0 alpha is released, if we want this. I believe if we do this refactoring, Tez will have to re-code it's client calls, which is fine now but won't be so once 3.0 is released. My thoughts are that this will make it cleaner to have separate client classes like the patch has. So I am +1 on reorganizing the TimeClientImpl into V1 and V2. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > Labels: YARN-5355, oct16-medium > Attachments: YARN-4675-YARN-2928.v1.001.patch > > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- 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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15371222#comment-15371222
]
Hadoop QA commented on YARN-4675:
-
| (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:red}-1{color} | {color:red} patch {color} | {color:red} 0m 6s {color}
| {color:red} YARN-4675 does not apply to YARN-2928. Rebase required? Wrong
Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12788479/YARN-4675-YARN-2928.v1.001.patch
|
| JIRA Issue | YARN-4675 |
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/12271/console |
| Powered by | Apache Yetus 0.3.0 http://yetus.apache.org |
This message was automatically generated.
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
>
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
>Reporter: Naganarasimha G R
>Assignee: Naganarasimha G R
> Labels: YARN-5355
> Attachments: YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
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
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15152776#comment-15152776
]
Hadoop QA commented on YARN-4675:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 12s
{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 1 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
47s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 23s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_72 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 25s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
21s {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
14s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
16s {color} | {color:green} YARN-2928 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 28s
{color} | {color:green} YARN-2928 passed with JDK v1.8.0_72 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 32s
{color} | {color:green} YARN-2928 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
26s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 20s
{color} | {color:green} the patch passed with JDK v1.8.0_72 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 20s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 24s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 24s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 17s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: patch
generated 11 new + 20 unchanged - 14 fixed = 31 total (was 34) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 29s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
10s {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} 1m
28s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 25s
{color} | {color:green} the patch passed with JDK v1.8.0_72 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 31s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 1m 56s {color}
| {color:red} hadoop-yarn-common in the patch failed with JDK v1.8.0_72.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 2m 9s {color} |
{color:red} hadoop-yarn-common in the patch failed with JDK v1.7.0_95. {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} 23m 2s {color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| JDK v1.8.0_72 Failed junit tests |
hadoop.yarn.client.api.impl.TestTimelineClient |
| JDK v1.7.0_95 Failed junit tests |
hadoop.yarn.client.api.impl.TestTimelineClient |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:0ca8df7 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12788479/YARN-4675-YARN-2928.v1.001.patch
|
| JIRA Issue | YARN-4675 |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux
[jira] [Commented] (YARN-4675) Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
[ https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15134534#comment-15134534 ] Naganarasimha G R commented on YARN-4675: - This issue needs to be merged after the last rebase with trunk, as it might have impact on rebase with the trunk. > Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl > > > Key: YARN-4675 > URL: https://issues.apache.org/jira/browse/YARN-4675 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Naganarasimha G R >Assignee: Naganarasimha G R > > We need to reorganize TimeClientImpl into TimeClientV1Impl , > TimeClientV2Impl and if required a base class, so that its clear which part > of the code belongs to which version and thus better maintainable. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
