Skip to content

Conversation

@dcoliversun
Copy link
Contributor

What changes were proposed in this pull request?

This PR reverts SPARK-39006 and add a case of sharing the same PVC between the driver and multiple executors in PV testing of integration testing.

Some PV types (such as NFS) support data sharing via the same PVC for multiple Pods. However, SPARK-39006 broke this scenario, causing multiple Pods unable to share the same PVC.

Why are the changes needed?

This is regression caused by SPARK-39006.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests, and added test scenarios

…tor PVC dynamic allocation failure"

This reverts commit b065c94.
@dcoliversun
Copy link
Contributor Author

@dongjoon-hyun @cometta would be good if you have time to review this PR

CONTAINER_MOUNT_PATH)
.set(s"spark.kubernetes.executor.volumes.persistentVolumeClaim.data.options.claimName",
PVC_NAME)
.set(s"spark.executor.instances", "2")
Copy link
Member

Choose a reason for hiding this comment

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

Sorry but I would disagree with this test case because the underlying is not NFS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to see if it's possible to set up NFS in k8s integration testing.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

-1 for the indention of this PR. We need more specialization to be valid.

Support driver and executors to shared same Kubernetes PVC

@dongjoon-hyun
Copy link
Member

I wrote a clean-revert PR with your authorship. Reverting PR should follow this style instead of claiming a new contribution like Support ....

@dcoliversun
Copy link
Contributor Author

Replaced this with #41069

@dcoliversun dcoliversun closed this May 6, 2023
dongjoon-hyun pushed a commit that referenced this pull request May 7, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes #41057

Closes #41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 3ba1fa3)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request May 7, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes #41057

Closes #41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
LuciferYang pushed a commit to LuciferYang/spark that referenced this pull request May 10, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes apache#41057

Closes apache#41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes apache#41057

Closes apache#41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 3ba1fa3)
Signed-off-by: Dongjoon Hyun <[email protected]>
GladwinLee pushed a commit to lyft/spark that referenced this pull request Oct 10, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes apache#41057

Closes apache#41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 3ba1fa3)
Signed-off-by: Dongjoon Hyun <[email protected]>
catalinii pushed a commit to lyft/spark that referenced this pull request Oct 10, 2023
…e for executor PVC dynamic allocation failure

### What changes were proposed in this pull request?

This reverts commit b065c94.

### Why are the changes needed?

To remove the regression from SPARK-39006.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

Closes apache#41057

Closes apache#41069 from dongjoon-hyun/SPARK-43342.

Authored-by: Qian.Sun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 3ba1fa3)
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants