Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-48887][K8S] Enable spark.kubernetes.executor.checkAllContainers by default #47337

Closed
wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jul 12, 2024

What changes were proposed in this pull request?

This PR aims to enable spark.kubernetes.executor.checkAllContainers by default from Apache Spark 4.0.0.

Why are the changes needed?

Since Apache Spark 3.1.0, spark.kubernetes.executor.checkAllContainers is supported and useful because sidecar pattern is used in many cases. Also, it prevents user mistakes which forget and ignore the sidecars' failures by always reporting sidecar failures via executor status.

Does this PR introduce any user-facing change?

  • This configuration is no-op when there is no other container.
  • This will report user containers' error correctly when there exist other containers which are provided by the users.

How was this patch tested?

Both true and false are covered by our CI test coverage since Apache Spark 3.1.0.

Was this patch authored or co-authored using generative AI tooling?

No.

@dongjoon-hyun
Copy link
Member Author

Could you review this PR, @viirya ?

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

Makes sense. Looks good to me.

@dongjoon-hyun
Copy link
Member Author

Thank you! Merged to master for Apache Spark 4.0.0-preview2.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-48887 branch July 12, 2024 21:54
jingz-db pushed a commit to jingz-db/spark that referenced this pull request Jul 22, 2024
…rs` by default

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

This PR aims to enable `spark.kubernetes.executor.checkAllContainers` by default from Apache Spark 4.0.0.

### Why are the changes needed?

Since Apache Spark 3.1.0, `spark.kubernetes.executor.checkAllContainers` is supported and useful because [sidecar pattern](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/) is used in many cases. Also, it prevents user mistakes which forget and ignore the sidecars' failures by always reporting sidecar failures via executor status.
- apache#29924

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

- This configuration is no-op when there is no other container.
- This will report user containers' error correctly when there exist other containers which are provided by the users.

### How was this patch tested?

Both `true` and `false` are covered by our CI test coverage since Apache Spark 3.1.0.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47337 from dongjoon-hyun/SPARK-48887.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
szehon-ho pushed a commit to szehon-ho/spark that referenced this pull request Aug 7, 2024
…rs` by default

This PR aims to enable `spark.kubernetes.executor.checkAllContainers` by default from Apache Spark 4.0.0.

Since Apache Spark 3.1.0, `spark.kubernetes.executor.checkAllContainers` is supported and useful because [sidecar pattern](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/) is used in many cases. Also, it prevents user mistakes which forget and ignore the sidecars' failures by always reporting sidecar failures via executor status.
- apache#29924

- This configuration is no-op when there is no other container.
- This will report user containers' error correctly when there exist other containers which are provided by the users.

Both `true` and `false` are covered by our CI test coverage since Apache Spark 3.1.0.

No.

Closes apache#47337 from dongjoon-hyun/SPARK-48887.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
…rs` by default

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

This PR aims to enable `spark.kubernetes.executor.checkAllContainers` by default from Apache Spark 4.0.0.

### Why are the changes needed?

Since Apache Spark 3.1.0, `spark.kubernetes.executor.checkAllContainers` is supported and useful because [sidecar pattern](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/) is used in many cases. Also, it prevents user mistakes which forget and ignore the sidecars' failures by always reporting sidecar failures via executor status.
- apache#29924

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

- This configuration is no-op when there is no other container.
- This will report user containers' error correctly when there exist other containers which are provided by the users.

### How was this patch tested?

Both `true` and `false` are covered by our CI test coverage since Apache Spark 3.1.0.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47337 from dongjoon-hyun/SPARK-48887.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
himadripal pushed a commit to himadripal/spark that referenced this pull request Oct 19, 2024
…rs` by default

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

This PR aims to enable `spark.kubernetes.executor.checkAllContainers` by default from Apache Spark 4.0.0.

### Why are the changes needed?

Since Apache Spark 3.1.0, `spark.kubernetes.executor.checkAllContainers` is supported and useful because [sidecar pattern](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/) is used in many cases. Also, it prevents user mistakes which forget and ignore the sidecars' failures by always reporting sidecar failures via executor status.
- apache#29924

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

- This configuration is no-op when there is no other container.
- This will report user containers' error correctly when there exist other containers which are provided by the users.

### How was this patch tested?

Both `true` and `false` are covered by our CI test coverage since Apache Spark 3.1.0.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47337 from dongjoon-hyun/SPARK-48887.

Authored-by: Dongjoon Hyun <[email protected]>
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