Fix zombie queries on metadata#608
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| data_types = [desc[1] for desc in res.cursor.description] | ||
|
|
||
| if SQLALCHEMY_VERSION >= '2.0.0': | ||
| # Lower versions of SQLAlchemy doesn't support execution of query as ctxt man |
There was a problem hiding this comment.
then why not always do it like the manual way? the context manager is just syntax sugar AFAIK for the same sequence of operations.
There was a problem hiding this comment.
also this path (> 2.0) isn't fetching data anyway so it'll show CANCELLED too.
BTW I don't agree that showing those as CANCELLED is wrong. We are unnecessarily fetching results for cosmetic benefits.
| data_types = [desc[1] for desc in res.cursor.description] | ||
|
|
||
| if SQLALCHEMY_VERSION >= '2.0.0': | ||
| # Lower versions of SQLAlchemy doesn't support execution of query as ctxt man |
There was a problem hiding this comment.
also this path (> 2.0) isn't fetching data anyway so it'll show CANCELLED too.
BTW I don't agree that showing those as CANCELLED is wrong. We are unnecessarily fetching results for cosmetic benefits.
| partition_names = [desc[0] for desc in conn.cursor.description] | ||
| data_types = [desc[1] for desc in conn.cursor.description] | ||
| res = conn.fetchall() # Fetching data to consider query as FINISHED and not CANCELED | ||
| if not conn.closed: |
There was a problem hiding this comment.
fix import order and see CI failures
There was a problem hiding this comment.
can we avoid mocked tests since the sequence of responses is now hand constructed and doesn't really test anything meaningful
2 participants
Description
_get_partitionsmethod were leaving zombie queries in Trino with the current implementation.connection should be closed with or not with context manager usage. The fetch of data here is optionally, but I did implemented because otherwise, query will be consider CANCELLED and not FINISHED.
Non-technical explanation
Fixed zombie queries from partitions metadata queries.
Release notes
( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: