Skip to content
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

[SPARK-25262][K8S] Allow SPARK_LOCAL_DIRS to be tmpfs backed on K8S #22323

Closed
wants to merge 2 commits into from

Conversation

rvesse
Copy link
Member

@rvesse rvesse commented Sep 3, 2018

What changes were proposed in this pull request?

The default behaviour of Spark on K8S currently is to create emptyDir volumes to back SPARK_LOCAL_DIRS. In some environments e.g. diskless compute nodes this may actually hurt performance because these are backed by the Kubelet's node storage which on a diskless node will typically be some remote network storage.

Even if this is enterprise grade storage connected via a high speed interconnect the way Spark uses these directories as scratch space (lots of relatively small short lived files) has been observed to cause serious performance degradation. Therefore we would like to provide the option to use K8S's ability to instead back these emptyDir volumes with tmpfs. Therefore this PR adds a configuration option that enables SPARK_LOCAL_DIRS to be backed by Memory backed emptyDir volumes rather than the default.

Documentation is added to describe both the default behaviour plus this new option and its implications. One of which is that scratch space then counts towards your pods memory limits and therefore users will need to adjust their memory requests accordingly.

NB - This is an alternative version of PR #22256 reduced to just the tmpfs piece

How was this patch tested?

Ran with this option in our diskless compute environments to verify functionality

Adds a configuration option that enables SPARK_LOCAL_DIRS to be backed
by Memory backed emptyDir volumes rather than the default which is
whatever the kubelet's node storage happens to be
- Simplify syntax
- Indentation fixes
- Doc improvements and clarifications
@rvesse
Copy link
Member Author

rvesse commented Sep 6, 2018

All comments so far addressed, can we kick off the PR builder on this now?

Copy link
Contributor

@liyinan926 liyinan926 left a comment

Choose a reason for hiding this comment

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

LGTM

@liyinan926
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented Sep 6, 2018

@SparkQA
Copy link

SparkQA commented Sep 6, 2018

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

@@ -215,6 +215,19 @@ spark.kubernetes.driver.volumes.persistentVolumeClaim.checkpointpvc.options.clai

The configuration properties for mounting volumes into the executor pods use prefix `spark.kubernetes.executor.` instead of `spark.kubernetes.driver.`. For a complete list of available options for each supported type of volumes, please refer to the [Spark Properties](#spark-properties) section below.

## Local Storage

Spark uses temporary scratch space to spill data to disk during shuffles and other operations. When using Kubernetes as the resource manager the pods will be created with an [emptyDir](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir) volume mounted for each directory listed in `SPARK_LOCAL_DIRS`. If no directories are explicitly specified then a default directory is created and configured appropriately.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this again, I'm wondering if we're getting too detailed in our documentation about all the features we support. Particularly with the pod template files pull request coming in adding more documentation, and it's possible that we have an information overload on this particular page.

Would it be reasonable to only include this information in the configuration options below, under the spark.kubernetes.local.dirs.tmpfs setting?

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't really want to block on this though so we can merge regardless I think. This might be something to consider for the next release, to go back over this document as a whole and clean it up if we think it's too verbose.

@mccheah
Copy link
Contributor

mccheah commented Sep 6, 2018

I'm going to merge this, for Spark 3.0 we should look at #22323 (comment) though.

@asfgit asfgit closed this in da6fa38 Sep 6, 2018
@rvesse rvesse deleted the SPARK-25262-tmpfs branch October 31, 2018 11:17
Jeffwan pushed a commit to Jeffwan/spark that referenced this pull request Oct 1, 2019
The default behaviour of Spark on K8S currently is to create `emptyDir` volumes to back `SPARK_LOCAL_DIRS`.  In some environments e.g. diskless compute nodes this may actually hurt performance because these are backed by the Kubelet's node storage which on a diskless node will typically be some remote network storage.

Even if this is enterprise grade storage connected via a high speed interconnect the way Spark uses these directories as scratch space (lots of relatively small short lived files) has been observed to cause serious performance degradation.  Therefore we would like to provide the option to use K8S's ability to instead back these `emptyDir` volumes with `tmpfs`. Therefore this PR adds a configuration option that enables `SPARK_LOCAL_DIRS` to be backed by Memory backed `emptyDir` volumes rather than the default.

Documentation is added to describe both the default behaviour plus this new option and its implications.  One of which is that scratch space then counts towards your pods memory limits and therefore users will need to adjust their memory requests accordingly.

*NB* - This is an alternative version of PR apache#22256 reduced to just the `tmpfs` piece

Ran with this option in our diskless compute environments to verify functionality

Author: Rob Vesse <rvesse@dotnetrdf.org>

Closes apache#22323 from rvesse/SPARK-25262-tmpfs.
Jeffwan pushed a commit to Jeffwan/spark that referenced this pull request Feb 28, 2020
The default behaviour of Spark on K8S currently is to create `emptyDir` volumes to back `SPARK_LOCAL_DIRS`.  In some environments e.g. diskless compute nodes this may actually hurt performance because these are backed by the Kubelet's node storage which on a diskless node will typically be some remote network storage.

Even if this is enterprise grade storage connected via a high speed interconnect the way Spark uses these directories as scratch space (lots of relatively small short lived files) has been observed to cause serious performance degradation.  Therefore we would like to provide the option to use K8S's ability to instead back these `emptyDir` volumes with `tmpfs`. Therefore this PR adds a configuration option that enables `SPARK_LOCAL_DIRS` to be backed by Memory backed `emptyDir` volumes rather than the default.

Documentation is added to describe both the default behaviour plus this new option and its implications.  One of which is that scratch space then counts towards your pods memory limits and therefore users will need to adjust their memory requests accordingly.

*NB* - This is an alternative version of PR apache#22256 reduced to just the `tmpfs` piece

Ran with this option in our diskless compute environments to verify functionality

Author: Rob Vesse <rvesse@dotnetrdf.org>

Closes apache#22323 from rvesse/SPARK-25262-tmpfs.
hopper-signifyd added a commit to hopper-signifyd/spark that referenced this pull request Jul 2, 2020
hopper-signifyd added a commit to hopper-signifyd/spark that referenced this pull request Jul 2, 2020
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.

4 participants