-
Notifications
You must be signed in to change notification settings - Fork 92
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
[COST-4852] Inserting attempts loop to Trino queries #5210
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5210 +/- ##
=====================================
Coverage 94.1% 94.1%
=====================================
Files 375 375
Lines 31411 31422 +11
Branches 4627 4627
=====================================
+ Hits 29545 29560 +15
+ Misses 1190 1189 -1
+ Partials 676 673 -3 |
This reverts commit e15ca07.
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we pull the changes in #5237 into this PR?
In order to retry only on hive metastore errors, we'll need another exception object. class TrnioMetastoreError(Exception):
...
@retry(retry_on=(TrinoNoSuchKeyException, TrinoHiveMetastoreError)
try:
...
except TrinoQueryError as trino_exc:
exc_to_raise = TrinoStatementExecError(
statement=stmt, statement_number=stmt_num, sql_params=s_params, trino_error=trino_exc
)
if "NoSuchKey" in str(trino_exc):
exc_to_raise = TrinoNoSuchKeyException(
message=trino_exc.message,
query_id=trino_exc.query_id,
error_code=trino_exc.error_code,
)
if trino_exc.error_name == "HIVE_METASTORE_ERROR":
exc_to_raise = TrinoHiveMetastoreError(
message=trino_exc.message,
query_id=trino_exc.query_id,
error_code=trino_exc.error_code,
)
LOG.warning(f"{exc_to_raise!s}")
raise exc_to_raise from trino_exc You could create an exception class hierarchy if you wanted for these custom exceptions. |
Co-authored-by: Sam Doran <[email protected]>
Co-authored-by: Sam Doran <[email protected]>
Co-authored-by: Sam Doran <[email protected]>
Co-authored-by: Sam Doran <[email protected]>
Inspecting the attribute is more precise than relying on the representation returned by str.
Use “Error” suffix to be more in line with how exception classes are name. Create a class hierarchy for custom exceptions to better identify errors from within our code.
Since the retry moved from the caller, patching out the entire method for testing will no longer work. Instead, raise an exception within _execute_trino_raw_sql_query_with_description to better simulate the failures durnig which the retry behavior is desired.
9ee1bdf
to
6fea578
Compare
/retest |
/retest |
Jira Ticket
COST-4852
Description
This change will implement an attempts' logic on Trino queries to avoid some Exceptions raisings.
We are removing the loop from the delete function and inserting in the functions that execute insertion queries.
I've set the attempts to three just for Azure for now. Let's monitor the Exceptions.
Testing
tox -e py39 -- hcs.test.database.test_report_db_accessor.TestHCSReportDBAccessor
andtox -e py39 -- masu.test.database.test_azure_report_db_accessor.AzureReportDBAccessorTest
Release Notes