Enhance Cursor close handling and context manager exception management to prevent server side resource leaks #554
Conversation
madhav-db
requested review from
deeksha-db,
samikshya-db,
jprakash-db,
yunbodeng-db,
jackyhu-db and
benc-db
as code owners
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
madhav-db
changed the title
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
…nsure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors.
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
…t to prevent server side resource leaks (databricks#554) * Enhance Cursor close handling and context manager exception management * tests * fmt * Fix Cursor.close() to properly handle CursorAlreadyClosedError * Remove specific test message from Cursor.close() error handling * Improve error handling in connection and cursor context managers to ensure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors. * add * add Signed-off-by: varun-edachali-dbx <[email protected]>
…t to prevent server side resource leaks (databricks#554) * Enhance Cursor close handling and context manager exception management * tests * fmt * Fix Cursor.close() to properly handle CursorAlreadyClosedError * Remove specific test message from Cursor.close() error handling * Improve error handling in connection and cursor context managers to ensure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors. * add * add Signed-off-by: varun-edachali-dbx <[email protected]>
What type of PR is this?
Description
This PR attempts to fix an issue where database connections were not properly closing, resulting in "lame duck" sessions that kept SQL warehouses active unnecessarily.
When clients disconnected, operation handles were not properly being closed on the server side, potentially leading to resource leaks. The
Cursor.close()methodwas simply nullifying the operation handle reference without properly closing it on the thrift backend.Besides this, in
Cursor.exit(), added better error handling to prevent exceptions during cursor closure from suppressing original exceptions, which is particularly important for user introduced interruptions (eg. Keyboard interrupt). Before this fix, when users interrupted long-running queries, the exception from cursor.close() could mask the KeyboardInterrupt, preventing the application from properly responding to cancellation requests and potentially leaving resources uncleaned.How is this tested?
Added new unit tests to verify behaviour
Related Tickets & Documents