-
Notifications
You must be signed in to change notification settings - Fork 236
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
Move Stack classes to wrapper classes to fix non-deterministic build issue #9576
Conversation
…2.13 Stack classes to handle build issues Signed-off-by: Navin Kumar <[email protected]>
From Jason's simple reproduce with this branch:
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a repro to .github/workflows/mvn-verify-check.yml or premerge
sql-plugin/src/main/scala-2.12/com/nvidia/spark/rapids/RapidsStack.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Navin Kumar <[email protected]>
Signed-off-by: Navin Kumar <[email protected]>
build |
Should we do this in a follow up issue for all the "unshimmed" classes? |
That is a good idea. |
Filed #9578 |
build |
failed core dump. could be related to cudf/jni changes, I will file another ticket #9582. Let me retrigger the CI here
|
build |
hmm still failing many assertion errors on CI machine (A30 + 32 cores cpu). I am not seeing a repo locally, this may related to the core numbers
|
build |
Signed-off-by: Navin Kumar <[email protected]>
build |
Fixes #9571.
This wraps the Scala 2.12/2.13 specific Stack classes in a class called
RapidsStack
, which is then used like the other classes. Previously, we extended the Spark specific classes withScalaStack
, but that created issues in the build due to the bytecode dependencies that linked back to SQLExecPlugin.class.