You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/09/23 06:47:27 UTC

[GitHub] [spark] dongjoon-hyun opened a new pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

dongjoon-hyun opened a new pull request #29846:
URL: https://github.com/apache/spark/pull/29846


   ### What changes were proposed in this pull request?
   
   This PR aims to support dynamic PVC creation and deletion for K8s executors.
   
   ### Why are the changes needed?
   
   While SPARK-32655 supports to mount a pre-created PVC, this PR can create PVC itself dynamically and reduce lots of manual efforts.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. This is a new feature.
   
   ### How was this patch tested?
   
   Pass the newly added test cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698021654


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33671/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit to users because this is a part of transient ids and the prefix already guarantees no conflicts.
   - `PVC_ACCESS_MODE`: Ya. I thought like you at the beginning, but I changed to this form to reduce the problem surface. Although `PVC_ACCESS_MODE  config` makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, additionally.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697169424


   **[Test build #129011 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129011/testReport)** for PR 29846 at commit [`2560335`](https://github.com/apache/spark/commit/2560335a41ef2fdd8f7471582f33a6f82d6b592c).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697196885






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698015143


   **[Test build #129050 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129050/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493947730



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeSpec.scala
##########
@@ -21,7 +21,10 @@ private[spark] sealed trait KubernetesVolumeSpecificConf
 private[spark] case class KubernetesHostPathVolumeConf(hostPath: String)
   extends KubernetesVolumeSpecificConf
 
-private[spark] case class KubernetesPVCVolumeConf(claimName: String)
+private[spark] case class KubernetesPVCVolumeConf(
+    claimName: String,
+    storageClass: Option[String] = None,
+    size: Option[String] = None)

Review comment:
       Do `storageClass` and `size` must be both defined or empty? Can we only define `storageClass` but without `size`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697169424


   **[Test build #129011 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129011/testReport)** for PR 29846 at commit [`2560335`](https://github.com/apache/spark/commit/2560335a41ef2fdd8f7471582f33a6f82d6b592c).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698028783






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697206379






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] HyukjinKwon commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697308228


   cc @holdenk, could you take a look for the test failure please? K8S PRs are blocked by it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit to users because this is a part of transient ids and the prefix already guarantees no conflicts.
   - `PVC_ACCESS_MODE`: Ya. I thought like you at the beginning, but I changed to this form. Although this makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698010893


   **[Test build #129050 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129050/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit to users because this is a part of transient ids and the prefix already guarantees no conflicts.
   - `PVC_ACCESS_MODE`: Ya. I thought like you at the beginning, but I changed to this form to reduce the problem surface. Although this makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697196870


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33634/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697170208


   **[Test build #129011 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129011/testReport)** for PR 29846 at commit [`2560335`](https://github.com/apache/spark/commit/2560335a41ef2fdd8f7471582f33a6f82d6b592c).
    * This patch **fails Scala style tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697180226


   **[Test build #129013 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129013/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698028783






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697206379


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun closed pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #29846:
URL: https://github.com/apache/spark/pull/29846


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697170232






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697170245


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/129011/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] holdenk commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697539116


   I agree this PR isn't touching anything in the decommissioning logic. That being said, I'll spend some time today on the decommissioning integration tests.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] HyukjinKwon removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
HyukjinKwon removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697308125


   cc @holdenk, could you take a look for the test failure please? Two PRs are blocked by it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697196885


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697206363


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33636/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698033498


   Thank you, @holdenk and @viirya .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698075783


   The follow-up PR is ready, https://github.com/apache/spark/pull/29859 .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expect some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit because this is a part of transient ids.
   - `PVC_ACCESS_MODE`: Although this makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit because this is a part of transient ids.
   - `PVC_ACCESS_MODE`: Although this makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697180352






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697187946


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33634/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] holdenk commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698009613


   Thanks for the clarity @dongjoon-hyun LGTM pending jenkins


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697540293


   Thanks, @holdenk .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] HyukjinKwon commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697308125


   cc @holdenk, could you take a look for the test failure please? Two PRs are blocked by it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698028969


   All tests passed. Merged to master for Apache Spark 3.1.0 on December 2020.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697170232


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697180352






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698009764


   Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698008622


   Retest this please.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] holdenk commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493778976



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -323,4 +323,22 @@ private[spark] object KubernetesUtils extends Logging {
         .build()
     }
   }
+
+  // Add a OwnerReference to the given resources making the pod an owner of them so when
+  // the pod is deleted, the resources are garbage collected.
+  def addOwnerReference(pod: Pod, resources: Seq[HasMetadata]): Unit = {

Review comment:
       I like this :)
   Initially I was thinking about asking about re-using the PVCs because I know that in some systems PVC creating is slow (for dynamic scaling), but it's probably better to return the resources and not do some weird storage pool like I was thinking of.

##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Would it make sense to have the OnDemand, postfix, and access mode configurable?

##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
##########
@@ -163,22 +165,6 @@ private[spark] class Client(
     }
   }
 
-  // Add a OwnerReference to the given resources making the driver pod an owner of them so when
-  // the driver pod is deleted, the resources are garbage collected.
-  private def addDriverOwnerReference(driverPod: Pod, resources: Seq[HasMetadata]): Unit = {
-    val driverPodOwnerReference = new OwnerReferenceBuilder()
-      .withName(driverPod.getMetadata.getName)
-      .withApiVersion(driverPod.getApiVersion)
-      .withUid(driverPod.getMetadata.getUid)
-      .withKind(driverPod.getKind)
-      .withController(true)
-      .build()
-    resources.foreach { resource =>
-      val originalMetadata = resource.getMetadata
-      originalMetadata.setOwnerReferences(Collections.singletonList(driverPodOwnerReference))
-    }
-  }
-

Review comment:
       Thank you for unifying this code :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] HyukjinKwon commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697592931


   Thanks @holdenk and @dongjoon-hyun.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698010893


   **[Test build #129050 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129050/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697539175


   cc @holdenk , @dbtsai , @viirya , @sunchao 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697175417


   **[Test build #129013 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129013/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698068616


   @dongjoon-hyun Looks like this broke Scala 2.13 [build](https://github.com/apache/spark/runs/1157816762). Would you like to fix it?
   
   ```scala
   Error: ] /home/runner/work/spark/spark/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala:117: type mismatch;
    found   : scala.collection.mutable.ArrayBuffer[io.fabric8.kubernetes.api.model.HasMetadata]
    required: Seq[io.fabric8.kubernetes.api.model.HasMetadata]
   ```
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698069211


   Oh, thank you for reporting! Sure!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493952674



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeSpec.scala
##########
@@ -21,7 +21,10 @@ private[spark] sealed trait KubernetesVolumeSpecificConf
 private[spark] case class KubernetesHostPathVolumeConf(hostPath: String)
   extends KubernetesVolumeSpecificConf
 
-private[spark] case class KubernetesPVCVolumeConf(claimName: String)
+private[spark] case class KubernetesPVCVolumeConf(
+    claimName: String,
+    storageClass: Option[String] = None,
+    size: Option[String] = None)

Review comment:
       Yes. We need both. In this PR, if one of them is missing, the fallback operation is the existing PVC mounting behavior. So, it doesn't try to create PVC and assume the PVC exists with the given PVC name.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697956767


   Thank you for review, @holdenk .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit to users because this is a part of transient ids and the prefix already guarantees no conflicts.
   - `PVC_ACCESS_MODE`: Although this makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697175417


   **[Test build #129013 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129013/testReport)** for PR 29846 at commit [`97d80fc`](https://github.com/apache/spark/commit/97d80fc753cafdd7e02d2c2cefb20cb9e1ed86e6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697206392


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33636/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698028773


   Kubernetes integration test status success
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33671/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697533646


   The decomission failure is irrelevant to this one.
   ```suggestion
   KubernetesSuite:
   - Run SparkPi with no resources
   - Run SparkPi with a very long application name.
   - Use SparkLauncher.NO_RESOURCE
   - Run SparkPi with a master URL without a scheme.
   - Run SparkPi with an argument.
   - Run SparkPi with custom labels, annotations, and environment variables.
   - All pods have the same service account by default
   - Run extraJVMOptions check on driver
   - Run SparkRemoteFileTest using a remote data file
   - Run SparkPi with env and mount secrets.
   - Run PySpark on simple pi.py example
   - Run PySpark with Python3 to test a pyfiles example
   - Run PySpark with memory customization
   - Run in client mode.
   - Start pod creation from template
   - PVs with local storage
   - Launcher client dependencies
   - Test basic decommissioning *** FAILED ***
   ```
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697196236


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33636/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-697196892


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33634/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698015255






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29846:
URL: https://github.com/apache/spark/pull/29846#discussion_r493881157



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
##########
@@ -84,4 +112,15 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
       (volumeMount, volume)
     }
   }
+
+  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
+    additionalResources
+  }
+}
+
+private[spark] object MountVolumesFeatureStep {
+  val PVC_ON_DEMAND = "OnDemand"
+  val PVC = "PersistentVolumeClaim"
+  val PVC_POSTFIX = "-pvc"
+  val PVC_ACCESS_MODE = "ReadWriteOnce"

Review comment:
       Ya, it's possible, but I didn't do that in this PR because of the followings.
   - `PVC_ON_DEMAND`: It's a dummy placeholder because the existing code expects some pre-defined names. We had better recommend a fixed one instead of making a configurable one in this case.
   - `PVC_POSTFIX`: It can be configurable but doesn't give much benefit to users because this is a part of transient ids and the prefix already guarantees no conflicts.
   - `PVC_ACCESS_MODE`: Ya. I thought like you at the beginning, but I changed to this form to reduce the problem surface. Although `PVC_ACCESS_MODE  config` makes sense a lot, I leave this PR to focus on a fixed one because this PR aims to generate a new PVC for each executor. In other words, this PR is not suggesting creating a `ReadWriteMany` PVC and sharing across in multiple executors. 
   
   For `ReadWriteMany` PVC, we don't need to use this PR. The existing Spark PVC feature can mount a single `ReadWriteMany` PVC into all executors without any problem and there is no burden to maintain `ReadWriteMany` PVC, because it's always a single one. In addition, we also support NFS (AWS EFS) mounting, too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29846: [SPARK-32971][K8S] Support dynamic PVC creation/deletion for K8s executors

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29846:
URL: https://github.com/apache/spark/pull/29846#issuecomment-698015255






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org