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

Ignore internal TableNotFoundException in DROP TABLE #19731

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

pajaks
Copy link
Member

@pajaks pajaks commented Nov 14, 2023

Description

Ignore internal TableNotFoundException in DROP TABLE for Iceberg and Hive
Fixes #18848

Additional context and related issues

Release notes

( x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Nov 14, 2023
@github-actions github-actions bot added tests:hive iceberg Iceberg connector hive Hive connector labels Nov 14, 2023
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from 95fbbe5 to b954763 Compare November 14, 2023 10:47
@pajaks
Copy link
Member Author

pajaks commented Nov 14, 2023

ci / pt (default, suite-8-non-generic, ) hit #16315

@findinpath
Copy link
Contributor

TestHiveTransactionalTable > testReadFullAcid [groups: hive_transactional] failure relates to #16315

@pajaks pajaks force-pushed the pajaks/drop_table_retry branch 2 times, most recently from d28c2aa to 0e828b4 Compare November 17, 2023 07:45
@pajaks pajaks marked this pull request as draft November 17, 2023 14:53
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from 337ef40 to fc5e603 Compare December 14, 2023 11:42
@pajaks pajaks marked this pull request as ready for review December 18, 2023 09:43
@pajaks pajaks requested review from findinpath and ebyhr December 18, 2023 11:20
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from 6dba470 to 85294b6 Compare December 19, 2023 13:47
@pajaks pajaks requested a review from findinpath December 19, 2023 13:48
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from 85294b6 to b21765a Compare December 20, 2023 13:19
@pajaks pajaks requested a review from findinpath December 20, 2023 13:22
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from b21765a to 3cabe12 Compare December 21, 2023 08:21
@pajaks
Copy link
Member Author

pajaks commented Dec 21, 2023

CI hit: #16315

@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from 3cabe12 to 60de40b Compare December 22, 2023 09:21
@pajaks pajaks requested a review from findepi December 22, 2023 11:35
Comment on lines 1089 to 1090
// If table is not found on consecutive attempts it was probably dropped on first attempt and timeout occurred.
// Exception in such case can be safely ignored.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There're 2 places NoSuchObjectException can happen - getTable & dropTable. This comment is true for getTable, but what about dropTable?

Does dropTable method throw the exception only for table and doesn't throw the same exception for other reasons (e.g. failed to drop partitions due to concurrent operation)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about wrapping in an additional try/catch the following line to make sure that the handling is done exclusively for the getTable metastore call:

Table table = client.getTable(databaseName, tableName);

@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from a90a658 to fe16931 Compare January 2, 2024 11:48
@pajaks pajaks force-pushed the pajaks/drop_table_retry branch from fe16931 to a4bc7e8 Compare January 3, 2024 09:22
@pajaks pajaks requested a review from ebyhr January 3, 2024 12:05
@ebyhr ebyhr merged commit a6eb444 into trinodb:master Jan 10, 2024
57 checks passed
@github-actions github-actions bot added this to the 436 milestone Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed hive Hive connector iceberg Iceberg connector
Development

Successfully merging this pull request may close these issues.

DROP TABLE may fail if drop_table metastore call is retried
4 participants