[SPARK-13901][CORE]correct the logDebug information when jump to the next locality level #11719
Conversation
| @@ -557,7 +557,7 @@ private[spark] class TaskSetManager( | |||
| lastLaunchTime += localityWaits(currentLocalityIndex) | |||
| currentLocalityIndex += 1 | |||
| logDebug(s"Moving to ${myLocalityLevels(currentLocalityIndex)} after waiting for " + | |||
| s"${localityWaits(currentLocalityIndex)}ms") | |||
| s"${localityWaits(currentLocalityIndex - 1)}ms") | |||
There was a problem hiding this comment.
Pull this out into a val above, use it above, and then just reuse it here. It's clearer
|
I'm really sorry to make extra commits...Are these codes clearer ? |
| } else if (curTime - lastLaunchTime >= localityWaits(currentLocalityIndex)) { | ||
| // Jump to the next locality level, and reset lastLaunchTime so that the next locality | ||
| // wait timer doesn't immediately expire | ||
| lastLaunchTime += localityWaits(currentLocalityIndex) | ||
| currentLocalityIndex += 1 | ||
| logDebug(s"Moving to ${myLocalityLevels(currentLocalityIndex)} after waiting for " + | ||
| s"${localityWaits(currentLocalityIndex)}ms") | ||
| s"${localityWaits(previousLocalityIndex)}ms") |
There was a problem hiding this comment.
Ah OK, not quite what I meant. But, how about a slightly different idea. To match how the debug statement above works, leave it as-is, but then make this one:
logDebug(s"Moving to ${myLocalityLevels(currentLocalityIndex+1)} after waiting for " +
s"${localityWaits(currentLocalityIndex)}ms")
currentLocalityIndex += 1
There was a problem hiding this comment.
Oh,I got it ,thank you for help.
|
@srowen Ready for another look. |
|
Jenkins test this please |
|
Test build #53295 has finished for PR 11719 at commit
|
|
Jenkins retest this please |
|
Test build #53307 has finished for PR 11719 at commit
|
|
The error of "java.lang.IllegalArgumentException: requirement failed" is confusing,I just exchange two lines of codes..Actually am I wrong ? |
|
I'm certain this is an unrelated failure, but not sure why the failure occurs. It has happened on a few builds. |
| @@ -555,9 +555,9 @@ private[spark] class TaskSetManager( | |||
| // Jump to the next locality level, and reset lastLaunchTime so that the next locality | |||
| // wait timer doesn't immediately expire | |||
| lastLaunchTime += localityWaits(currentLocalityIndex) | |||
| currentLocalityIndex += 1 | |||
| logDebug(s"Moving to ${myLocalityLevels(currentLocalityIndex)} after waiting for " + | |||
| logDebug(s"Moving to ${myLocalityLevels(currentLocalityIndex + 1)} after waiting for " + | |||
There was a problem hiding this comment.
haven't looked closely at this, but could currentLocalityIndex + 1 create an index out of bound for myLocalityLevels?
There was a problem hiding this comment.
Before, it was incremented first and then used as an index, and now it's increment afterwards, so it shouldn't be different. Really the net change is for the second part of the message to refer to the previous index, so should be OK.
… next locality level JIRA Issue:https://issues.apache.org/jira/browse/SPARK-13901 In getAllowedLocalityLevel method of TaskSetManager,we get wrong logDebug information when jump to the next locality level.So we should fix it. Author: trueyao <501663994@qq.com> Closes #11719 from trueyao/logDebug-localityWait. (cherry picked from commit ea9ca6f) Signed-off-by: Sean Owen <sowen@cloudera.com>
… next locality level JIRA Issue:https://issues.apache.org/jira/browse/SPARK-13901 In getAllowedLocalityLevel method of TaskSetManager,we get wrong logDebug information when jump to the next locality level.So we should fix it. Author: trueyao <501663994@qq.com> Closes apache#11719 from trueyao/logDebug-localityWait.
JIRA Issue:https://issues.apache.org/jira/browse/SPARK-13901
In getAllowedLocalityLevel method of TaskSetManager,we get wrong logDebug information when jump to the next locality level.So we should fix it.