Skip to content

Conversation

@dvogelbacher
Copy link
Contributor

What changes were proposed in this pull request?

Kubernetes driver and executor pods should request memory + memoryOverhead as their resources instead of just memory, see https://issues.apache.org/jira/browse/SPARK-23825

How was this patch tested?

Existing unit tests were adapted.

@dvogelbacher dvogelbacher changed the title requesting memory + memory overhead when creating kubernetes pods for… [ SPARK-23825] [K8s] Requesting memory + memory overhead for pods Mar 30, 2018
@dvogelbacher dvogelbacher changed the title [ SPARK-23825] [K8s] Requesting memory + memory overhead for pods [ SPARK-23825] [K8s] Requesting memory + memory overhead for pod memory Mar 30, 2018
@mccheah
Copy link
Contributor

mccheah commented Mar 30, 2018

ok to test

@SparkQA
Copy link

SparkQA commented Mar 30, 2018

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1821/

@SparkQA
Copy link

SparkQA commented Mar 30, 2018

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1821/

@felixcheung
Copy link
Member

Jenkins, ok to test

@SparkQA
Copy link

SparkQA commented Mar 30, 2018

Test build #88738 has finished for PR 20943 at commit 69be1b3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dvogelbacher
Copy link
Contributor Author

@felixcheung can you review this?

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a reasonable point.
@foxish @mccheah @erikerlandson what do you think?

val requests = resourceRequirements.getRequests.asScala
assert(requests("cpu").getAmount === "2")
assert(requests("memory").getAmount === "256Mi")
assert(requests("memory").getAmount === "456Mi")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering - shouldn't this be 256M + 384M?
The documentation says - executorMemory * 0.10, with minimum of 384.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm - @liyinan926 just pointed out that it's being set explicitly above which I totally missed.

Copy link
Contributor

@foxish foxish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

val requests = resourceRequirements.getRequests.asScala
assert(requests("cpu").getAmount === "2")
assert(requests("memory").getAmount === "256Mi")
assert(requests("memory").getAmount === "456Mi")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm - @liyinan926 just pointed out that it's being set explicitly above which I totally missed.

@dvogelbacher dvogelbacher changed the title [ SPARK-23825] [K8s] Requesting memory + memory overhead for pod memory [SPARK-23825][K8s] Requesting memory + memory overhead for pod memory Apr 2, 2018
@asfgit asfgit closed this in 6151f29 Apr 2, 2018
robert3005 pushed a commit to palantir/spark that referenced this pull request Apr 4, 2018
## What changes were proposed in this pull request?

Kubernetes driver and executor pods should request `memory + memoryOverhead` as their resources instead of just `memory`, see https://issues.apache.org/jira/browse/SPARK-23825

## How was this patch tested?
Existing unit tests were adapted.

Author: David Vogelbacher <[email protected]>

Closes apache#20943 from dvogelbacher/spark-23825.
mshtelma pushed a commit to mshtelma/spark that referenced this pull request Apr 5, 2018
## What changes were proposed in this pull request?

Kubernetes driver and executor pods should request `memory + memoryOverhead` as their resources instead of just `memory`, see https://issues.apache.org/jira/browse/SPARK-23825

## How was this patch tested?
Existing unit tests were adapted.

Author: David Vogelbacher <[email protected]>

Closes apache#20943 from dvogelbacher/spark-23825.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants