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

Use Spark's HybridRowQueue to avoid MemoryConsumer API shim #2000

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Mar 23, 2021

#640 added a copy of Spark's HybridRowQueue and supporting classes, but this breaks on Spark 3.2.0 because the MemoryConsumer API changed. The original reasons for adding a copy was listed as visibility problems, but this can be worked around by placing our code in a package with visibiility, and such a package already exists.

Signed-off-by: Jason Lowe <jlowe@nvidia.com>
@jlowe jlowe added this to the Mar 15 - March 26 milestone Mar 23, 2021
@jlowe jlowe self-assigned this Mar 23, 2021
@jlowe
Copy link
Member Author

jlowe commented Mar 23, 2021

Full disclosure: I was not able to verify this fixed the failing cudf_udf integration test on Spark 3.2, as I am unable to get those tests running in my development environment. @firestarman can you verify these changes or explain how to setup a proper environment for the cudf_udf tests to run?

@jlowe jlowe requested a review from firestarman March 23, 2021 16:25
@jlowe
Copy link
Member Author

jlowe commented Mar 23, 2021

build

@sameerz sameerz added the task Work required that improves the product but is not user facing label Mar 24, 2021
@tgravescs
Copy link
Collaborator

@firestarman were you able to validate?

@jlowe
Copy link
Member Author

jlowe commented Mar 25, 2021

I'm going to merge this. The nightly build is still broken, and this is highly likely to fix it. I wasn't able to get the cudf_udf integration test to run properly, but I can guarantee that if the integration test still fails after this is merged that it will fail differently. I'll kick the snapshot build and integration test build early to see if there are any subsequent failures.

@jlowe jlowe merged commit 24bbaa9 into NVIDIA:branch-0.5 Mar 25, 2021
@firestarman
Copy link
Collaborator

Sorry guys, I missed this. I will pay more attention to review requests. Thanks all for this.

nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@jlowe jlowe deleted the fix-hybridrowqueue branch September 10, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task Work required that improves the product but is not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants