[SPARK-39006][K8S] Show a directional error message for executor PVC dynamic allocation failure by dcoliversun · Pull Request #36374 · apache/spark

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

 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

+ 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

+ 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

+ 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

 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

+ 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-actionsBot 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 b065c94May 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

Uh oh!

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?

Uh oh!

AmplabJenkins commented Apr 28, 2022

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyunApr 28, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dcoliversun commented Apr 29, 2022

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dcoliversun commented May 8, 2022

Uh oh!

0xdarkman commented Jun 20, 2023

Uh oh!

dongjoon-hyun commented Jun 20, 2023

Uh oh!

dongjoon-hyun commented Jun 20, 2023

Uh oh!

0xdarkman commented Jun 21, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Jun 22, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xdarkman commented Jun 23, 2023

Uh oh!

dongjoon-hyun commented Jun 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dongjoon-hyunApr 28, 2022•
edited
Loading

0xdarkman commented Jun 21, 2023•
edited
Loading

dongjoon-hyun commented Jun 22, 2023•
edited
Loading