-
Notifications
You must be signed in to change notification settings - Fork 29k
SPARK-4337. [YARN] Add ability to cancel pending requests #4141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #25901 has started for PR 4141 at commit
|
|
Test build #25901 has finished for PR 4141 at commit
|
|
Test FAILed. |
|
LGTM. Something seems funny with jenkins, lots of PRs are failing tests with seemingly no reason... |
|
LGTM, i think @tdas you can take a look at this PR. |
|
retest this please |
|
Test build #26068 has started for PR 4141 at commit
|
|
Test build #26068 has finished for PR 4141 at commit
|
|
Test FAILed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about making it private[yarn]? will still be visible in tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YarnAllocator is already private[yarn], so this will end up private[yarn] already.
|
just a minor comment, otherwise lgtm |
|
Jenkins, test this please. |
|
Test build #26110 has started for PR 4141 at commit
|
|
Test build #26110 has finished for PR 4141 at commit
|
|
Test PASSed. |
|
@lianhuiwang I am not sure I can comment on this PR. I am unfamiliar with this piece of code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can just make this $numToCancel
|
@sryza this looks correct to me. My comments are mostly minor. Quick question, do we have to do any additional thing for this to work with dynamic allocation too or does it come for free with this patch? |
|
Never mind I just found #4168 |
472f944 to
a98bd20
Compare
|
Thanks for the review @andrewor14. Updated patch addresses your comments. |
|
Test build #26921 has started for PR 4141 at commit
|
|
Test build #26921 has finished for PR 4141 at commit
|
|
Test PASSed. |
|
Ok I'm merging this into master and 1.3 thanks |
Author: Sandy Ryza <sandy@cloudera.com> Closes #4141 from sryza/sandy-spark-4337 and squashes the following commits: a98bd20 [Sandy Ryza] Andrew's comments cdaab7f [Sandy Ryza] SPARK-4337. Add ability to cancel pending requests to YARN (cherry picked from commit 1a88f20) Signed-off-by: Andrew Or <andrew@databricks.com>
No description provided.