[pos] Support building presto-on-spark for spark2.4 and spark3.4#25552
[pos] Support building presto-on-spark for spark2.4 and spark3.4#25552singcha merged 1 commit intoprestodb:masterfrom
Conversation
|
|
|
@wangkewen Thansk for your contribution, can you sign the EasyCLA and resolve the conflicts first? Also since this is a new feature, it is better to provide release notes and documentation about setup and configurations. |
0f08664 to
7a73f47
Compare
79dbe4b to
92d0b76
Compare
77c9586 to
06641bd
Compare
shrinidhijoshi
left a comment
There was a problem hiding this comment.
@wangkewen Thanks for making the changes. I have left some initial comments on amount of classes that are duplicated.
I see quite a few pom issues that CI is flagging, let's work through fixing all those.
.../src/main/java/com/facebook/presto/spark/classloader_interface/PrestoSparkNativeTaskRdd.java
Show resolved
Hide resolved
776df45 to
3959e03
Compare
...r-spark2/src/main/java/com/facebook/presto/spark/classloader_interface/PrestoSparkUtils.java
Outdated
Show resolved
Hide resolved
shrinidhijoshi
left a comment
There was a problem hiding this comment.
@wangkewen Thanks for addressing the comments. I did another pass and left some more comments.
Also, please update the PR description in line with presto contributor guidlines.
|
rebase on @shrinidhijoshi's PR-25794 |
|
rebase on master to include @shrinidhijoshi 's PR-25797 |
shrinidhijoshi
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments @wangkewen . Based on the latest changes I left a few more comments.
|
@wangkewen Created #25833 to track adding spark3.4 based CI tests as a follow up |
shrinidhijoshi
left a comment
There was a problem hiding this comment.
LGTM! Thanks for working through all the comments @wangkewen
|
Change looks good. Can you clean up the commits? (squash them and make sure the commit title and message follows our guidelines)? https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines#commit-formatting-and-pull-requests |
|
@rschlussel I have updated it. |
All code changes are made within the presto-spark-classloader-interface module. To support different implementations for spark2.4 and spark3.4, two separate modules have been created: presto-spark-classloader-spark2 and presto-spark-classloader-spark3. These modules contain the version-specific codes, including `PrestoSparkUtils`. It adds support for shuffle deserialize in `PrestoSparkShuffleSerializer`.
|
@wangkewen has imported this pull request. If you are a Meta employee, you can view this in D80638570. |
Description
This initial PR prepares the release of Presto on Spark 3.4.1 by incorporating updates from this pull request by @shrinidhijoshi.
Prior to the stable release of Presto on Spark 3 (e.g., Spark 3.4.1), code compatibility is maintained for both Spark 2.4 (the current version) and Spark 3.4. To support this, a Maven profile named
spark3has been created to facilitate building Presto on Spark 3.All code changes are made within the presto-spark-classloader-interface module. To support different implementations for Spark versions 2.4 and 3.4, two separate modules have been created: presto-spark-classloader-spark2 and presto-spark-classloader-spark3. These modules contain the version-specific code, including a new
PrestoSparkUtils.In this PR, it adds support for shuffle deserialize in PrestoSparkShuffleSerializer.java
Motivation and Context
It is needed for upgrading Presto on Spark to support Spark3 (e.g., Spark 3.4.1) .
Impact
Test Plan
Internal pt verifier test: x1125914587947600
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.