[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure #36374

dcoliversun · 2022-04-27T09:23:51Z

What changes were proposed in this pull request?

This PR aims to show a directional error message for executor PVC dynamic allocation failure.

Why are the changes needed?

#29846 supports dynamic PVC creation/deletion for K8s executors.
#29557 support execId placeholder in executor PVC conf.
If not set spark.kubernetes.executor.volumes.persistentVolumeClaim.spark-local-dir-1.options.claimName with onDemand or SPARK_EXECUTOR_ID, spark will continue to try to create the executor pod.
After this PR, spark can show a directional error message for this situation.

ERROR ExecutorPodsSnapshotsStoreImpl: Going to stop due to IllegalArgumentException
java.lang.IllegalArgumentException: PVC ClaimName should contain OnDemand or SPARK_EXECUTOR_ID when multiple executors are required

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add unit test.

…llocation Failure

AmplabJenkins · 2022-04-28T12:58:55Z

Can one of the admins verify this patch?

dongjoon-hyun

Thank you for changing your direction. This is much better.

dongjoon-hyun · 2022-04-28T18:53:36Z

...netes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala

@@ -120,6 +122,20 @@ private[spark] class MountVolumesFeatureStep(conf: KubernetesConf)
  override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
    additionalResources.toSeq
  }
+
+  private def checkPVCClaimNameWhenMultiExecutors(claimName: String): Unit = {


checkPVCClaimNameWhenMultiExecutors -> checkPVCClaimName because this PR checks all cases instead of WhenMultiExecutors and doesn't raise exception for single instance case.

Change it as checkPVCClaimName :)

dongjoon-hyun · 2022-04-28T18:54:46Z

...netes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala

+  private def checkPVCClaimNameWhenMultiExecutors(claimName: String): Unit = {
+    val invalidClaimName =
+      if (!claimName.contains(PVC_ON_DEMAND) && !claimName.contains(ENV_EXECUTOR_ID)) true
+      else false


This is misleading because this is totally valid when there is only one executor.

Done, check it when multiple executors

dongjoon-hyun · 2022-04-28T18:56:02Z

...netes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala

+      else false
+
+    val executorInstances = conf.get(EXECUTOR_INSTANCES)
+    if (executorInstances.isEmpty) return


Please try to avoid return in Scala method.

Yeah, remove return

dongjoon-hyun · 2022-04-28T18:57:02Z

...netes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala

+    if (invalidClaimName && executorInstances.get > 1) {
+      throw new IllegalArgumentException("PVC ClaimName should contain " +
+        PVC_ON_DEMAND + " or " + ENV_EXECUTOR_ID +
+        " when multiple executors are required")


The error message looks informative but please include the current claim name in the error message too.

Have added the current claim name

dongjoon-hyun · 2022-04-28T18:58:04Z

.../core/src/test/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStepSuite.scala

@@ -148,6 +149,26 @@ class MountVolumesFeatureStepSuite extends SparkFunSuite {
    assert(executorPVC.getClaimName.endsWith("-exec-1-pvc-0"))
  }

+  test("SPARK-39006 Show a directional error message for PVC Dynamic Allocation Failure") {


We need :, e.g., SPARK-39006 -> SPARK-39006:.

After this PR, we don't have PVC Dynamic Allocation Failure. Please revise the test case.

Have two changes here:

Add : after SPARK-39006

Change test case as Check PVC ClaimName

dongjoon-hyun · 2022-04-28T18:58:47Z

.../core/src/test/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStepSuite.scala

+      mountReadOnly = true,
+      KubernetesPVCVolumeConf("testClaimName")
+    )
+    val conf = new SparkConf().set(EXECUTOR_INSTANCES, 2)


Please add a positive test case where executor instance is 1.

Fine, added a positive test here

dongjoon-hyun

I left a few comments. Thanks, @dcoliversun .

dcoliversun · 2022-04-29T09:41:08Z

@dongjoon-hyun Thanks for your comments, I have updated code. Please review again

dongjoon-hyun

+1, LGTM. Thank you, @dcoliversun .
Merged to master for Apache Spark 3.4.

dcoliversun · 2022-05-08T08:56:21Z

Thank you @dongjoon-hyun

0xdarkman · 2023-06-20T11:20:54Z

can we expect the fix to be in next spark version release 3.4.1?

dongjoon-hyun · 2023-06-20T15:59:29Z

can we expect the fix to be in next spark version release 3.4.1?

Could you be more specific, @0xdarkman ? Which JIRA do you want for Apache Spark 3.4.1? You can check Apache Spark 3.4.1 RC1 vote for the detail.

https://lists.apache.org/thread/fo5qlkqstjmszolzl9j6yk95jb61jr5t

dongjoon-hyun · 2023-06-20T16:01:18Z

For this PR (SPARK-39006), it's reverted cleanly via SPARK-43342 at Apache Spark 3.4.1.

[SPARK-43342][K8S] Revert SPARK-39006 Show a directional error message for executor PVC dynamic allocation failure #41069

0xdarkman · 2023-06-21T13:39:36Z

can we expect the fix to be in next spark version release 3.4.1?

Could you be more specific, @0xdarkman ? Which JIRA do you want for Apache Spark 3.4.1? You can check Apache Spark 3.4.1 RC1 vote for the detail.

https://lists.apache.org/thread/fo5qlkqstjmszolzl9j6yk95jb61jr5t

I have been migrating from spark 3.3.2 and deployed spark 3.4.0 and had error with PVC ReadWriteMany access so this is how I come across this issue.
I understand I can not use spark 3.4.0 because the change is reverted so the version I can use either spark 3.3.2 or wait for spark 3.4.1 in order to use PVC with ReadWriteMany access when multiple executors used.

if it is fix, change reverted or whatever it does not matter to me as far as I can use new spark version with PVC access similarly to what spark version did prior v 3.4.0.

dongjoon-hyun · 2023-06-22T07:21:27Z

Could you try Apache Spark 3.4.1 in advance? RC1 binary is already there and the vote will finished in 24 hours.

If you can participate the community vote, it would be great, @0xdarkman .

0xdarkman · 2023-06-23T12:40:40Z

I am running this version 3.4.1 and it looks good.

dongjoon-hyun · 2023-06-24T02:00:51Z

Thank you for confirming. Apache Spark 3.4.1 is released officially.

[SPARK-39006][K8S] Show a directional error message for PVC Dynamic A…

856cbbe

…llocation Failure

dcoliversun mentioned this pull request Apr 27, 2022

[SPARK-39006][K8S] Check PVC claimName must be OnDemand when multiple executors required #36333

Closed

github-actions bot added the KUBERNETES label Apr 27, 2022

dongjoon-hyun reviewed Apr 28, 2022

View reviewed changes

dongjoon-hyun requested changes Apr 28, 2022

View reviewed changes

address comments

5ed2a17

dcoliversun requested a review from dongjoon-hyun May 4, 2022 02:12

dongjoon-hyun approved these changes May 8, 2022

View reviewed changes

dongjoon-hyun closed this in b065c94 May 8, 2022

dcoliversun deleted the SPARK-39006 branch May 8, 2022 08:56

dcoliversun mentioned this pull request Jan 21, 2023

[SPARK-41781][K8S] Add the ability to create pvc before creating driver/executor pod #39306

Closed

dcoliversun mentioned this pull request May 5, 2023

[SPARK-43342][K8S] Support driver and executors to shared same Kubernetes PVC #41057

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure #36374

[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure #36374

dcoliversun commented Apr 27, 2022

AmplabJenkins commented Apr 28, 2022

dongjoon-hyun left a comment

dongjoon-hyun Apr 28, 2022

dcoliversun Apr 29, 2022

dongjoon-hyun Apr 28, 2022 •

edited

Loading

dcoliversun Apr 29, 2022

dongjoon-hyun Apr 28, 2022

dcoliversun Apr 29, 2022

dongjoon-hyun Apr 28, 2022

dcoliversun Apr 29, 2022

dongjoon-hyun Apr 28, 2022

dcoliversun Apr 29, 2022

dongjoon-hyun Apr 28, 2022

dcoliversun Apr 29, 2022

dongjoon-hyun left a comment

dcoliversun commented Apr 29, 2022

dongjoon-hyun left a comment

dcoliversun commented May 8, 2022

0xdarkman commented Jun 20, 2023

dongjoon-hyun commented Jun 20, 2023

dongjoon-hyun commented Jun 20, 2023

0xdarkman commented Jun 21, 2023 •

edited

Loading

dongjoon-hyun commented Jun 22, 2023 •

edited

Loading

0xdarkman commented Jun 23, 2023

dongjoon-hyun commented Jun 24, 2023

[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure #36374

[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure #36374

Conversation

dcoliversun commented Apr 27, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

AmplabJenkins commented Apr 28, 2022

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dcoliversun commented Apr 29, 2022

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dcoliversun commented May 8, 2022

0xdarkman commented Jun 20, 2023

dongjoon-hyun commented Jun 20, 2023

dongjoon-hyun commented Jun 20, 2023

0xdarkman commented Jun 21, 2023 • edited Loading

dongjoon-hyun commented Jun 22, 2023 • edited Loading

0xdarkman commented Jun 23, 2023

dongjoon-hyun commented Jun 24, 2023

dongjoon-hyun Apr 28, 2022 •

edited

Loading

0xdarkman commented Jun 21, 2023 •

edited

Loading

dongjoon-hyun commented Jun 22, 2023 •

edited

Loading