[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14522271#comment-14522271 ] Jian He commented on YARN-3546: --- bq. Let's consider below situation, Hi [~sandflee], it's a valid situation. But waitForSchedulerAppAttemptAdded is really just a test utility method, it's not used in any production code. The whole MockRM is used for testing only. AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14522456#comment-14522456 ] sandflee commented on YARN-3546: ok, close it now, thanks [~jianhe] AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14520292#comment-14520292 ] Jian He commented on YARN-3546: --- [~sandflee], inside the scheduler, every application only has one attempt. so the current attempt is the attempt corresponding to the appAttemptId. So the name 'getAppAttempt(attemptId)' is matching with the internal implementation. If you agree, we can close this jira. AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[
https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14520706#comment-14520706
]
sandflee commented on YARN-3546:
[~jianhe], thanks for your explanation, I stil have one doubt,
waitForSchedulerAppAttemptAdded(ApplicationAttemptId attemptId) in MockRM maybe
expect to waiting RMAppAttempt with attemptId to be SCHEDULED, actually It'll
be true after first appAttempt is SCHEDULED. It'll cause launch AM failed
before YARN-3533. maybe waitForSchedulerAppAttemptAdded should be corrected as
below:
int tick = 0;
// Wait for at most 5 sec
SchedulerApplicationAttempt attempt =
((AbstractYarnScheduler)rm.getResourceScheduler()).getApplicationAttempt(attemptId);
while (null == attempt
attempt.getApplicationAttemptId().equals(attemptId) == false
tick 50) {
Thread.sleep(100);
if (tick % 10 == 0) {
System.out.println(waiting for SchedulerApplicationAttempt=
+ attemptId + added.);
}
tick++;
attempt =
((AbstractYarnScheduler)rm.getResourceScheduler()).getApplicationAttempt(attemptId);
}
so why waitForSchedulerAppAttemptAdded is not done as expected, I thought the
deep reason is getApplicationAttempt(attemptId) is misleading, Especially for
the scheduler newbies , we may expect it to return corresponding sheduler app
attempt with attemptId.
That's the reason why I suggest to rename it, please correct me if anything
wrong, thanks.
AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're
some misuse of it
-
Key: YARN-3546
URL: https://issues.apache.org/jira/browse/YARN-3546
Project: Hadoop YARN
Issue Type: Bug
Components: resourcemanager
Reporter: sandflee
I'm not familiar with scheduler, with first eyes, I thought this func
returns the schdulerAppAttempt info corresponding to appAttemptId, but
actually it returns the current schdulerAppAttempt.
It seems misled others too, such as
TestWorkPreservingRMRestart.waitForNumContainersToRecover
MockRM.waitForSchedulerAppAttemptAdded
should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId
applicationid)
or returns null if current attempt id not equals to the request attempt id ?
comment preferred!
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14520742#comment-14520742 ] Jian He commented on YARN-3546: --- Didn't get you. the waitForSchedulerAppAttemptAdded implementations should be correct, it waits for the given attempt to be added into scheduler. In fact, after YARN-3533, this call is not needed any more. so, it becomes a no-op. Regarding the name, it does't make too much difference.. The implement is to return the corresponding attempt, given the attempt Id. And the only way today to get that is through the corresponding application and from there get the current attempt. AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14520819#comment-14520819 ] sandflee commented on YARN-3546: sorry for my explanation. Let's consider below situation, 1, am1 crashed , rmAppAttempt1 send ATTEMPT_REMOVE event to scheduler 2, scheduler stops schedulerAppAttempt1(not clear), waits for rmAppAttempt2 added to scheduler 3, at this time If we use waitForSchedulerAppAttemptAdded(attempt2) it will be true, and maybe it's not the expected action AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-3546) AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it
[ https://issues.apache.org/jira/browse/YARN-3546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14520838#comment-14520838 ] sandflee commented on YARN-3546: The implement of AbstractYarnScheduler.getApplicationAttempt has no problem, just the name and parrameters confusing me when I view launchAM failures in YARN-3387 and maybe confusing other scheduler newbies. I wondered If I had explained more clearly, sorry for that. AbstractYarnScheduler.getApplicationAttempt seems misleading, and there're some misuse of it - Key: YARN-3546 URL: https://issues.apache.org/jira/browse/YARN-3546 Project: Hadoop YARN Issue Type: Bug Components: resourcemanager Reporter: sandflee I'm not familiar with scheduler, with first eyes, I thought this func returns the schdulerAppAttempt info corresponding to appAttemptId, but actually it returns the current schdulerAppAttempt. It seems misled others too, such as TestWorkPreservingRMRestart.waitForNumContainersToRecover MockRM.waitForSchedulerAppAttemptAdded should I rename it to T getCurrentSchedulerApplicationAttempt(ApplicationId applicationid) or returns null if current attempt id not equals to the request attempt id ? comment preferred! -- This message was sent by Atlassian JIRA (v6.3.4#6332)
