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

Make RapidsBufferHandle AutoCloseable to prevent extra attempts to remove buffers #7548

Merged

Conversation

abellina
Copy link
Collaborator

@abellina abellina commented Jan 19, 2023

Signed-off-by: Alessandro Bellina [email protected]

This is follow on work from #7512 but the issue was there before my pr was merged because spillable columnar batch could be closed on a finalizer in the broadcast code, attempting to close some buffer that doesn't exist anymore (because this races with the catalog cleanup logic)

This centralizes the close to the RapidsBufferHandle. Note, I made no attempts to invalidate the id in the handle, or throw from the other methods for this handle. I figure we can add that if we see issues or reviewers find that we need to be really strict here and make the handle throw on use.

@abellina
Copy link
Collaborator Author

build

@abellina abellina merged commit 60b3731 into NVIDIA:branch-23.02 Jan 19, 2023
@abellina abellina deleted the spill/make_handle_autocloseable branch January 19, 2023 20:44
@mattahrens mattahrens added the feature request New feature or request label Jan 20, 2023
@sameerz sameerz added performance A performance related task/issue reliability Features to improve reliability or bugs that severly impact the reliability of the plugin and removed feature request New feature or request labels Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance A performance related task/issue reliability Features to improve reliability or bugs that severly impact the reliability of the plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants