Fixed a typo in DAGScheduler. #8308
Conversation
|
ok to test. |
|
Yeah, pretty trivial but true. Same for taskSetFailed I think. |
|
Would you like if I go through all of them? |
|
I think the additional changes I mentioned are worth fixing here since they're very much of the same form. If you have time to scan for similar problems in nearby source, that's fine, but fixing all the related issues in this file seems like a good logical change. |
|
Test build #41234 has finished for PR 8308 at commit
|
|
I went through it, also reorganized a method to the front and made method comments consistent. |
| /** | ||
| * Finds the earliest-created active job that needs the stage. | ||
| * | ||
| * TODO: Probably should actually find among the active jobs that need this |
There was a problem hiding this comment.
I'm not sure all of this should be scaladoc. This looks like a private comment in the source code. Same for "Broken out for easier testing..."
|
Test build #41325 has finished for PR 8308 at commit
|
|
|
||
| case ExceptionFailure(className, description, stackTrace, fullStackTrace, metrics) => | ||
| // Do nothing here, left up to the TaskScheduler to decide how to handle user failures | ||
| // Do nothing here, left up to the TaskScheduler implementation | ||
| // to decide how to handle user failures |
There was a problem hiding this comment.
Why was this changed? it's not a big deal, but it was OK on one line. Changes always have this small but non-zero chance of tangling up a merge later, so trivial changes are usually avoided
There was a problem hiding this comment.
I guess it must have been going over 100 in length.
There was a problem hiding this comment.
Oh right, because of the extra word. That's fine.
PS You'll need to rebase this branch on master
|
Test build #41326 has finished for PR 8308 at commit
|
|
Test build #41450 timed out for PR 8308 at commit |
| @@ -769,7 +790,9 @@ class DAGScheduler( | |||
| } | |||
| } | |||
|
|
|||
| /** Called when stage's parents are available and we can now do its task. */ | |||
| /** | |||
| * Called when stage's parents are available and we can now do its task. | |||
There was a problem hiding this comment.
The changes here that just make scaladoc into a multiline comment don't do anything .. it's still valid on one line. I think the other changes look fine, and this is a no-op at best, but personally would not change these lines if there is no functional change.
|
LGTM |
|
Test build #41529 has finished for PR 8308 at commit
|
|
The test failure must be spurious; this is a scaladoc-only change, and compilation / style checks succeed. |
Author: ehnalis <[email protected]> Closes #8308 from ehnalis/master. (cherry picked from commit 7f1e507) Signed-off-by: Sean Owen <[email protected]>
No description provided.