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

Fix column->row conversion GPU check: #5190

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

mythrocks
Copy link
Collaborator

Fixes #5181.

The change in #5122 modifies the row->column conversion code to run
differently on Pascal GPUs. It does so by checking the GPU architecture
to detect whether the current device is Pascal.
In a distributed setup, this check should run on the Spark executor. As it
currently stands, the check is erroneously run on the driver.

This fix postpones the GPU architecture check till the executor task attempts
to fetch result rows.

Signed-off-by: MithunR mythrocks@gmail.com

Fixes NVIDIA#5181.

The change in NVIDIA#5122 modifies the row->column conversion code to run
differently on Pascal GPUs. It does so by checking the GPU architecture
to detect whether the current device is Pascal.
In a distributed setup, this check should run on the Spark executor. As it
currently stands, the check is erroneously run on the driver.

This fix postpones the GPU architecture check till the executor task attempts
to fetch result rows.

Signed-off-by: MithunR <mythrocks@gmail.com>
@mythrocks mythrocks self-assigned this Apr 8, 2022
@mythrocks mythrocks marked this pull request as draft April 8, 2022 23:10
@mythrocks
Copy link
Collaborator Author

I have yet to verify that this works in a distributed setup. I'll move this out of draft as soon as that's verified.

@mythrocks
Copy link
Collaborator Author

Build

jlowe
jlowe previously approved these changes Apr 9, 2022
@jlowe
Copy link
Contributor

jlowe commented Apr 9, 2022

@mythrocks did you intend to target this to 22.04?

@mythrocks
Copy link
Collaborator Author

Yes, sir. The GPU arch check is in 22.04 already. :/

@mythrocks mythrocks marked this pull request as ready for review April 9, 2022 04:47
@mythrocks mythrocks marked this pull request as draft April 9, 2022 04:49
@mythrocks
Copy link
Collaborator Author

mythrocks commented Apr 9, 2022

Sorry, I haven't yet confirmed that this change is adequate.

@mythrocks
Copy link
Collaborator Author

Nope, sorry. I'll have to pick this up on Monday.

@sameerz sameerz added the bug Something isn't working label Apr 9, 2022
@sameerz sameerz added this to the Apr 4 - Apr 15 milestone Apr 9, 2022
@mythrocks mythrocks marked this pull request as ready for review April 11, 2022 21:22
@mythrocks
Copy link
Collaborator Author

Apologies for the prolonged delay. I have now confirmed that this patch fixes #5181.
(Maximum gratitude to @gerashegalov for tag-teaming to debug the remote-cluster problem.)

@jlowe jlowe changed the base branch from branch-22.06 to branch-22.04 April 11, 2022 22:15
@jlowe jlowe dismissed their stale review April 11, 2022 22:15

The base branch was changed.

@jlowe
Copy link
Contributor

jlowe commented Apr 11, 2022

build

@mythrocks
Copy link
Collaborator Author

Build

@sameerz
Copy link
Collaborator

sameerz commented Apr 12, 2022

build

@mythrocks
Copy link
Collaborator Author

@pxLi : Thank you for checking this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Dataproc tests failing when trying to detect for accelerated row conversions
4 participants