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

Remove shims module [databricks] #4629

Merged
merged 17 commits into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions aggregator/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@
<version>${project.version}</version>
<classifier>${spark.version.classifier}</classifier>
</dependency>
<dependency>
<groupId>com.nvidia</groupId>
<artifactId>rapids-4-spark-shims-${spark.version.classifier}_${scala.binary.version}</artifactId>
<version>${project.version}</version>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
6 changes: 0 additions & 6 deletions api_validation/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,6 @@
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.nvidia</groupId>
<artifactId>rapids-4-spark-shims-${spark.version.classifier}_${scala.binary.version}</artifactId>
<version>22.04.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
</dependencies>

<build>
Expand Down
2 changes: 1 addition & 1 deletion docs/dev/compute_sanitizer.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
layout: page
title: Compute Sanitizer
nav_order: 6
nav_order: 7
parent: Developer Overview
---

Expand Down
2 changes: 1 addition & 1 deletion docs/dev/idea-code-style-settings.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
layout: page
title: IDEA Code Style Settings
nav_order: 4
nav_order: 5
parent: Developer Overview
---
```xml
Expand Down
2 changes: 1 addition & 1 deletion docs/dev/microk8s.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
layout: page
title: Setting up a Microk8s Environment
nav_order: 5
nav_order: 6
parent: Developer Overview
---

Expand Down
10 changes: 5 additions & 5 deletions shims/README.md → docs/dev/shims.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
layout: page
title: Shim Development
nav_order: 1
nav_order: 4
parent: Developer Overview
---
# Shim Development
Expand All @@ -16,7 +16,7 @@ common code, maximize reuse, and minimize logic duplication.
This is achieved by using a ServiceProvider pattern. All Shims implement the same API,
the suitable Shim implementation is loaded after detecting the current Spark build version
attempting to instantiate our plugin. We use the
[ShimLoader](../sql-plugin/src/main/scala/com/nvidia/spark/rapids/ShimLoader.scala)
[ShimLoader](https://github.com/NVIDIA/spark-rapids/blob/main/sql-plugin/src/main/scala/com/nvidia/spark/rapids/ShimLoader.scala)
class as a tight entry point for interacting with the host Spark runtime.

In the following we provide recipes for typical scenarios addressed by the Shim layer.
Expand All @@ -43,8 +43,8 @@ between Spark 3.1.x and Spark 3.2.x. So instead of deriving from such classes di
inject an intermediate trait e.g. `com.nvidia.spark.rapids.shims.v2.ShimExpression` that
has a varying source code depending on the Spark version we compile against to overcome this
issue as you can see e.g., comparing TreeNode:
1. [ShimExpression For 3.0.x and 3.1.x](../sql-plugin/src/main/301until320-all/scala/com/nvidia/spark/rapids/shims/v2/TreeNode.scala#L23)
2. [ShimExpression For 3.2.x](../sql-plugin/src/main/301until320-all/scala/com/nvidia/spark/rapids/shims/v2/TreeNode.scala#L23)
1. [ShimExpression For 3.0.x and 3.1.x](https://github.com/NVIDIA/spark-rapids/blob/main/sql-plugin/src/main/post320-treenode/scala/com/nvidia/spark/rapids/shims/v2/TreeNode.scala#L23)
2. [ShimExpression For 3.2.x](https://github.com/NVIDIA/spark-rapids/blob/main/sql-plugin/src/main/pre320-treenode/scala/com/nvidia/spark/rapids/shims/v2/TreeNode.scala#L23)

This resolves compile-time problems, however, now we face the problem at run time.

Expand Down Expand Up @@ -105,7 +105,7 @@ has not been loaded yet. More accurately, it may not be strictly needed until la
query can be run when the Spark SQL session and its extensions are initialized. It improves the
user experience if the first query is not penalized beyond necessary though. By design, Plugin guarantees
that the classloader is
[set up at load time](../sql-plugin/src/main/scala/com/nvidia/spark/SQLPlugin.scala#L29)
[set up at load time](https://github.com/NVIDIA/spark-rapids/blob/main/sql-plugin/src/main/scala/com/nvidia/spark/SQLPlugin.scala#L29)
before the DriverPlugin and ExecutorPlugin instances are called the `init` method on.

By making a visible class merely a wrapper of the real implementation, extending `scala.Proxy` where `self` is a lazy
Expand Down
66 changes: 52 additions & 14 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/301/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand All @@ -130,7 +131,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -162,6 +162,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/302/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand All @@ -183,7 +184,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -219,6 +219,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/303/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand All @@ -235,7 +236,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -271,6 +271,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/304/scala</source>
<source>${project.basedir}/src/main/301until310-all/scala</source>
<source>${project.basedir}/src/main/301until310-nondb/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
Expand All @@ -287,7 +288,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -323,6 +323,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/311/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
Expand All @@ -343,7 +344,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -408,7 +408,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -456,6 +455,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/312db/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311until320-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
Expand All @@ -473,7 +473,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -507,6 +506,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/312/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
Expand All @@ -527,7 +527,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -564,6 +563,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/313/scala</source>
<source>${project.basedir}/src/main/301until320-all/scala</source>
<source>${project.basedir}/src/main/301until320-nondb/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
Expand All @@ -584,7 +584,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -621,6 +620,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/320/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
<source>${project.basedir}/src/main/311+-nondb/scala</source>
Expand All @@ -631,14 +631,23 @@
</sources>
</configuration>
</execution>
<execution>
<id>add-test-profile-src-320</id>
<goals><goal>add-test-source</goal></goals>
<phase>none</phase> <!--Disable it for all submodules, only sql-plugin needs to enable it -->
<configuration>
<sources>
<source>${project.basedir}/src/test/320/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -674,6 +683,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/321/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
<source>${project.basedir}/src/main/311+-nondb/scala</source>
Expand All @@ -684,14 +694,23 @@
</sources>
</configuration>
</execution>
<execution>
<id>add-test-profile-src-321</id>
<goals><goal>add-test-source</goal></goals>
<phase>none</phase> <!--Disable it for all submodules, only sql-plugin needs to enable it -->
<configuration>
<sources>
<source>${project.basedir}/src/test/321/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -727,6 +746,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/322/scala</source>
<source>${project.basedir}/src/main/301until330-all/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
<source>${project.basedir}/src/main/311+-nondb/scala</source>
Expand All @@ -737,14 +757,23 @@
</sources>
</configuration>
</execution>
<execution>
<id>add-test-profile-src-322</id>
<goals><goal>add-test-source</goal></goals>
<phase>none</phase> <!--Disable it for all submodules, only sql-plugin needs to enable it -->
<configuration>
<sources>
<source>${project.basedir}/src/test/322/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -780,6 +809,7 @@
<configuration>
<sources>
<source>${project.basedir}/src/main/301+-nondb/scala</source>
<source>${project.basedir}/src/main/330/scala</source>
<source>${project.basedir}/src/main/311+-all/scala</source>
<source>${project.basedir}/src/main/311+-nondb/scala</source>
<source>${project.basedir}/src/main/320+/scala</source>
Expand All @@ -789,14 +819,23 @@
</sources>
</configuration>
</execution>
<execution>
<id>add-test-profile-src-330</id>
<goals><goal>add-test-source</goal></goals>
<phase>none</phase> <!--Disable it for all submodules, only sql-plugin needs to enable it -->
<configuration>
<sources>
<source>${project.basedir}/src/test/330/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down Expand Up @@ -858,7 +897,6 @@
<modules>
<module>dist</module>
<module>integration_tests</module>
<module>shims</module>
<module>shuffle-plugin</module>
<module>sql-plugin</module>
<module>tests</module>
Expand Down
Loading