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

[GLUTEN-4424] Explore adding Spark 35 w/ Scala 2.12 only (WIP) #4425

Closed
wants to merge 23 commits into from

Conversation

holdenk
Copy link
Contributor

@holdenk holdenk commented Jan 17, 2024

What changes were proposed in this pull request?

This adds Spark 3.5 support w/ Scala 2.12 avoiding the 2.13 related changes in #3476

(Fixes: #4424)

How was this patch tested?

Existing unit tests (although they're not in great shape so I'm not confident yet).

Copy link

#4424

Copy link

Run Gluten Clickhouse CI

2 similar comments
Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

@FelixYBW
Copy link
Contributor

We usually take 3 steps to add a Spark version.

  1. pass compile, add related code to shims, adding support to the new features
  2. pass TPCH/DS
  3. pass UTs

@holdenk
Copy link
Contributor Author

holdenk commented Jan 17, 2024

@FelixYBW: Do you mean those three steps would be in separate PRs?

@FelixYBW
Copy link
Contributor

@FelixYBW: Do you mean those three steps would be in separate PRs?

Yes, may be many PRs, let's solve this one by one.

@weiting-chen weiting-chen mentioned this pull request Feb 18, 2024
21 tasks
… make it into 3.5.0, so code won't compile just yet I'll update target to 3.5.1 once released and we can continue from there.
Copy link

Run Gluten Clickhouse CI

@zhouyuan
Copy link
Contributor

The style issue can be fixed with spotless
mvn spotless:apply -Pspark-3.2 clean package

@JkSelf
Copy link
Contributor

JkSelf commented Feb 26, 2024

@holdenk
Thank you very much for your work on the 3.5 version. Based on the experience of supporting 3.4, we can further split this PR, which will accelerate the progress of our review. This first PR only needs to ensure that the compilation can pass and make the necessary shim code changes. We can enable TPCH/DS in the second PR. Of course, if the first PR has already enabled TPCH/DS , we won't need the second PR. Next, we can enable unit tests. If there are many failed ut, we can list these failed ut first and then open separate PRs to address them later.

Can you help to resolve the conflicts and format issue firstly? And also remove the ut changes from this PR? If you don't have much time, I can take over this task. Looking forward to your reply. Thanks.

@holdenk
Copy link
Contributor Author

holdenk commented Feb 26, 2024

I was going to wait for the 3.5.1 release to be ready on the Spark side (it's very close) and then continue on the PR. I can split the test changes out, personally I don't like merging code without tests or with failing tests but if that's the way y'all do it I can split it.

@zhouyuan
Copy link
Contributor

I was going to wait for the 3.5.1 release to be ready on the Spark side (it's very close) and then continue on the PR. I can split the test changes out, personally I don't like merging code without tests or with failing tests but if that's the way y'all do it I can split it.

The reason is based on our experience, fixing unit tests may require a long time, so we usually split the support patch into several parts, otherwise it will be a very big PR

thanks,
-yuan

@JkSelf
Copy link
Contributor

JkSelf commented Feb 29, 2024

Spark 3.5.1 has been officially released. https://spark.apache.org/releases/spark-release-3-5-1.html

@holdenk
Copy link
Contributor Author

holdenk commented Feb 29, 2024

Yup, I'm going on vacation though until next Thursday so it you want to take it over go for it otherwise I can take a look when I'm back.

@JkSelf
Copy link
Contributor

JkSelf commented Feb 29, 2024

Yup, I'm going on vacation though until next Thursday so it you want to take it over go for it otherwise I can take a look when I'm back.

Sure, I will handle the task for 351 and ensure you receive the credit. Thank you for your efforts.

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale stale label Apr 15, 2024
Copy link

This PR was auto-closed because it has been stalled for 10 days with no activity. Please feel free to reopen if it is still valid. Thanks.

@github-actions github-actions bot closed this Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Spark 3.5 w/ Scala 2.12
4 participants