[SPARK-38194] Followup: Fix k8s memory overhead passing to executor pods#35901
[SPARK-38194] Followup: Fix k8s memory overhead passing to executor pods#35901Kimahriman wants to merge 3 commits intoapache:masterfrom
Conversation
|
@tgravescs is this what you were thinking? |
...rnetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala
Outdated
Show resolved
Hide resolved
Kimahriman
force-pushed
the
k8s-memory-overhead-executors
branch
from
ddd276e to
16460c4
Compare
|
Think I need to fix the test for this too |
|
merged to master, thanks @Kimahriman |
|
Thanks for your fix, late LGTM |
| // The memory overhead factor to use. If the user has not set it, then use a different | ||
| // value for non-JVM apps. This value is propagated to executors. | ||
| private val overheadFactor = | ||
| // The default memory overhead factor to use, derived from the deprecated |
|
FYI, master CI passed in K8S IT, We might want to backport this with squash previous revert patch to 3.3 if needed. |
|
@dongjoon-hyun do you have any concerns with pulling into 3.3 at this point? |
|
No, it is a legitimate backporting request, @tgravescs . To @Kimahriman , as @Yikun mentioned, could you make a single healthy backporting PR to |
|
Yeah I can do that. Do I need to combine the commits myself or do you just squash everything when you merge anyway? |
|
Apache Spark merge-script will squash for you. You can make a PR with all commits. |
|
That's what I thought, just wanted to make sure! |
Follow up to apache#35504 to fix k8s memory overhead handling. apache#35504 introduced a bug only caught by the K8S integration tests. Fix back to old behavior. See if IT passes Closes apache#35901 from Kimahriman/k8s-memory-overhead-executors. Authored-by: Adam Binford <adamq43@gmail.com> Signed-off-by: Thomas Graves <tgraves@apache.org>
4 participants
What changes were proposed in this pull request?
Follow up to #35504 to fix k8s memory overhead handling.
Why are the changes needed?
#35504 introduced a bug only caught by the K8S integration tests.
Does this PR introduce any user-facing change?
Fix back to old behavior.
How was this patch tested?
See if IT passes