-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-26446][CORE] Add cachedExecutorIdleTimeout docs at ExecutorAllocationManager #23386
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 #4486 has finished for PR 23386 at commit
|
|
retest this please. |
|
Test build #4487 has finished for PR 23386 at commit
|
|
Retest this please. |
|
Hi, @TopGunViper . Thank you for your first contribution! |
|
Hi, @dongjoon-hyun Thank you for your review. |
|
Test build #100482 has finished for PR 23386 at commit
|
dongjoon-hyun
left a comment
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.
+1, LGTM. Thanks, @TopGunViper .
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.
Ur, I know the meaning, but this sentence itself looks unclear to me; if, until? Could you make this simpler?
Note that if an executor caching any data blocks will be removed until it has been idle for more then L seconds.
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 remove if and change until to if or once ?
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.
And, more then -> more than
core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
Outdated
Show resolved
Hide resolved
dec0449 to
e887d2b
Compare
|
Merged to master. Thank you, @TopGunViper , @srowen , @Ngone51 . |
|
Thanks a lot for review, @dongjoon-hyun @Ngone51 @srowen |
…ocationManager ## What changes were proposed in this pull request? Add docs to describe how remove policy act while considering the property `spark.dynamicAllocation.cachedExecutorIdleTimeout` in ExecutorAllocationManager ## How was this patch tested? comment-only PR. Closes apache#23386 from TopGunViper/SPARK-26446. Authored-by: wuqingxin <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…ocationManager ## What changes were proposed in this pull request? Add docs to describe how remove policy act while considering the property `spark.dynamicAllocation.cachedExecutorIdleTimeout` in ExecutorAllocationManager ## How was this patch tested? comment-only PR. Closes apache#23386 from TopGunViper/SPARK-26446. Authored-by: wuqingxin <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
Add docs to describe how remove policy act while considering the property
spark.dynamicAllocation.cachedExecutorIdleTimeoutin ExecutorAllocationManagerHow was this patch tested?
comment-only PR.