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.

Sign up for GitHub

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

Jump to bottom

Prepare for 3.2.1 shim, various shim build fixes and improvements [databricks] #3923

Merged
merged 31 commits into from
Oct 29, 2021
Merged

Prepare for 3.2.1 shim, various shim build fixes and improvements [databricks] #3923

merged 31 commits into from
Oct 29, 2021

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Oct 26, 2021 •
edited
Loading

Signed-off-by: Gera Shegalov gera@apache.org

Contributes to #3470
Closes #3915

  • Remove unused dependency-reduced-pom-generation from agg pom
  • Eliminate double invocation of the shade plugin taking extra 6 seconds by delegating artifact attachment to the build helper plugin
  • Improve clean by customizing maven-clean-plugin
  • Enable *WithDatabricks profiles in buildall
  • Generate SparkShimServiceProvider discovery files on the fly
  • Refactor dist/pom.xml to reduce copy/paste code by 50%, delegate to default plugins when possible by customization
  • Disable doc generation automatically if the profile does not contain 301 to avoid inadvertent diffs on CI
  • Fix typo in verify_same_sha_for_unshimmed
  • Remove redundant SparkShims.getRapidsShuffleManagerClass
  • Consolidate most of Spark320 shim code into Spark32xShims.

@gerashegalov
Implemenent a Spark 3.2.1-SNAPSHOT Shim
0620e3d 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
wip
dbeb2f3
@gerashegalov
Merge remote-tracking branch 'origin/branch-21.12' into spark321-issu…
9c8c9e4 
…e3470
@gerashegalov
wip
cde35f0
@gerashegalov
wip
8a5638a
@gerashegalov
shaded reduced workaround
3a8a0f0
@gerashegalov
wip
57c7fb8
@gerashegalov
loops
36da325
@gerashegalov
Fix dist build
b6a182d 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
individual refactored
1216961
@gerashegalov
Merge remote-tracking branch 'origin/branch-21.12' into spark321-issu…
b9b7d52 
…e3470
@gerashegalov
ShimService file generation fixed
c554d71 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
done
5313e07
@gerashegalov
Merge remote-tracking branch 'origin/branch-21.12' into spark321-issu…
ec85993 
…e3470
@gerashegalov
Add unit test
ed786c8 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
compacter service file generation
984e9fb 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov added this to the Oct 18 - Oct 29 milestone Oct 26, 2021
@gerashegalov gerashegalov self-assigned this Oct 26, 2021
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov gerashegalov linked an issue Oct 26, 2021 that may be closed by this pull request
[BUG] typo in verify_same_sha_for_unshimmed prevents the offending class file name from being logged. #3915
Closed
@gerashegalov gerashegalov added build Related to CI / CD or cleanly building Spark 3.2+ tech debt labels Oct 26, 2021
@gerashegalov
add to ci
6554442 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
Copy link
Collaborator Author

Personally I would have preferred this pr be broken up into build changes and 3.2.1 separately for easier review and figure out what changes went with what.

I took 3.2.1 out of this PR

tgravescs
tgravescs reviewed Oct 27, 2021
View reviewed changes
Copy link
Collaborator

@tgravescs tgravescs left a comment

Choose a reason for hiding this comment

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

mostly nits, love that this cleans up all this hardcoded pom file stuff!

aggregator/pom.xml Outdated Show resolved Hide resolved
aggregator/pom.xml Outdated Show resolved Hide resolved
build/buildall Outdated Show resolved Hide resolved
build/buildall Outdated Show resolved Hide resolved
build/buildall Show resolved Hide resolved
dist/maven-antrun/build-parallel-worlds.xml Outdated Show resolved Hide resolved
dist/maven-antrun/build-parallel-worlds.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
shims/spark320/src/main/scala/com/nvidia/spark/rapids/shims/spark320/Spark320Shims.scala Show resolved Hide resolved
@gerashegalov gerashegalov mentioned this pull request Oct 27, 2021
Merged
@gerashegalov gerashegalov mentioned this pull request Oct 28, 2021
Closed
@gerashegalov
wi[
316a2d1
@gerashegalov
Restore attach-artifact workaround, and dist phases
29576f3 
process-classes is too late

Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov changed the title Prepare for 3.2.1 shim, various shim build fixes and improvements Prepare for 3.2.1 shim, various shim build fixes and improvements [databricks] Oct 28, 2021
@gerashegalov
Copy link
Collaborator Author

build

tgravescs
tgravescs previously approved these changes Oct 29, 2021
View reviewed changes
dist/pom.xml
@@ -42,35 +42,21 @@
<artifactId>rapids-4-spark-aggregator_${scala.binary.version}</artifactId>
<version>${project.version}</version>
</dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this needs upmerged to latest changes where we marked provided? or maybe it doesn't matter because we remove the dependencies in reduced pom

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

upmerge fixed it , and thanks for pointing out inconsistent use of spark-hive deps

@tgravescs
Copy link
Collaborator

guess this needs to be upmerged, I can look again once that is done

@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
Removing explicit spark-sql/hive deps in dist other
d56bf5f 
than antrun

Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov gerashegalov merged commit c6b2479 into NVIDIA:branch-21.12 Oct 29, 2021
@gerashegalov gerashegalov deleted the spark321-issue3470 branch October 29, 2021 22:57
@gerashegalov gerashegalov mentioned this pull request Oct 31, 2021
Closed
This was referenced Nov 1, 2021
Closed
Closed
@gerashegalov gerashegalov mentioned this pull request Nov 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgravescs tgravescs tgravescs approved these changes

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@jlowe jlowe Awaiting requested review from jlowe jlowe is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

Assignees

@gerashegalov gerashegalov

Labels
build Related to CI / CD or cleanly building Spark 3.2+ tech debt
Projects
None yet
Milestone
Oct 18 - Oct 29
Development

Successfully merging this pull request may close these issues.

[BUG] typo in verify_same_sha_for_unshimmed prevents the offending class file name from being logged.
2 participants
@gerashegalov @tgravescs