Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Fix shim-related bugs [databricks] #5708

Merged
merged 1 commit into from
Jun 1, 2022
Merged

Fix shim-related bugs [databricks] #5708

merged 1 commit into from
Jun 1, 2022

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Jun 1, 2022 •
edited
Loading

Bugs related to running with spark.rapids.force.caller.classloader=false #5703

  • Shim ExplainPlanImpl
  • Remove unnecessary reference to SparkShimsImpl

Closes #5707

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

@gerashegalov
Fix shim-related bugs
6ec3c02 
- Shim ExplainPlanImpl
- Remove unnecessary reference to SparkShimsImpl

Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov added bug Something isn't working build Related to CI / CD or cleanly building task Work required that improves the product but is not user facing labels Jun 1, 2022
@gerashegalov gerashegalov added this to the May 23 - Jun 3 milestone Jun 1, 2022
@gerashegalov gerashegalov self-assigned this Jun 1, 2022
@gerashegalov
Copy link
Collaborator Author 

jar tvf $PWD/dist/target/rapids-4-spark_2.12-22.08.0-SNAPSHOT-cuda11.jar | grep ExplainPlanImpl
  1700 Tue May 31 22:22:44 PDT 2022 spark3xx-common/com/nvidia/spark/rapids/ExplainPlanImpl.class

@gerashegalov gerashegalov changed the title Fix shim-related bugs Fix shim-related bugs [databricks] Jun 1, 2022
@gerashegalov
Copy link
Collaborator Author

build

@tgravescs
Copy link
Collaborator

it would be nice to have a better description here stating ExplainPlanImpl is currently packaging only one version (com/nvidia/spark/rapids/ExplainPlanImpl.class) and potentially missing the shim specific versions:

./sql-plugin/target/spark311/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class
./sql-plugin/target/spark321cdh/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class
./sql-plugin/target/spark320/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class
./sql-plugin/target/spark321/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class
./sql-plugin/target/spark313/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class
./sql-plugin/target/spark312/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class

@gerashegalov
Copy link
Collaborator Author

it would be nice to have a better description here stating ExplainPlanImpl is currently packaging only one version (com/nvidia/spark/rapids/ExplainPlanImpl.class) and potentially missing the shim specific versions:

./sql-plugin/target/spark311/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class ./sql-plugin/target/spark321cdh/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class ./sql-plugin/target/spark320/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class ./sql-plugin/target/spark321/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class ./sql-plugin/target/spark313/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class ./sql-plugin/target/spark312/classes/com/nvidia/spark/rapids/ExplainPlanImpl.class

The problem is that the classloading isolation of spakr-version-dependent incompatible classes referenced via ExplainPlanImpl->GpuOverrides does not work if the entry-point class loaded via ShimLoader is not hidden to a parallel world dir like spark-* in the jar.

@tgravescs
Copy link
Collaborator

reference #5703

@gerashegalov gerashegalov merged commit e54eb4c into NVIDIA:branch-22.08 Jun 1, 2022
@gerashegalov gerashegalov deleted the gerashegalov/shimBugFixesFrom5646 branch June 1, 2022 17:24
@gerashegalov gerashegalov mentioned this pull request Jun 1, 2022
Closed
@gerashegalov gerashegalov linked an issue Jun 2, 2022 that may be closed by this pull request
[BUG] Fix shim-related bugs #5707
Closed
2 tasks
@gerashegalov gerashegalov mentioned this pull request Jun 2, 2022
Closed
2 tasks
HaoYang670 pushed a commit to HaoYang670/spark-rapids that referenced this pull request Jun 6, 2022
@gerashegalov @HaoYang670
Fix shim-related bugs (NVIDIA#5708)
aabe8bb 
Bugs related to running with spark.rapids.force.caller.classloader=false NVIDIA#5703 
- Shim ExplainPlanImpl
- Remove unnecessary reference to SparkShimsImpl

Closes NVIDIA#5707 

Signed-off-by: Gera Shegalov <gera@apache.org>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@tgravescs tgravescs tgravescs approved these changes

Assignees

@gerashegalov gerashegalov

Labels
bug Something isn't working build Related to CI / CD or cleanly building task Work required that improves the product but is not user facing
Projects
None yet
Milestone
May 23 - Jun 3
Development

Successfully merging this pull request may close these issues.

[BUG] Fix shim-related bugs
2 participants
@gerashegalov @tgravescs