Skip to content

Retry dropping delta tables registered on AWS Glue#16092

Merged
ebyhr merged 3 commits intotrinodb:masterfrom
findinpath:findinpath/delta-databricks-retry-drop-table
Feb 21, 2023
Merged

Retry dropping delta tables registered on AWS Glue#16092
ebyhr merged 3 commits intotrinodb:masterfrom
findinpath:findinpath/delta-databricks-retry-drop-table

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

@findinpath findinpath commented Feb 13, 2023

Description

Retry dropping the delta table, in the unlikely situation that the Databricks cluster may be calling
asynchronously in the background glue:UpdateTable (e.g.: update table schema) on AWS Glue.

Additional context and related issues

Fixes #13199

Release notes

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

Hive, Delta
* Retry dropping delta tables registered on AWS Glue in case of failures due to concurrent modifications

@cla-bot cla-bot bot added the cla-signed label Feb 13, 2023
@findinpath findinpath added the no-release-notes This pull request does not require release notes entry label Feb 13, 2023
@findinpath findinpath requested a review from ebyhr February 13, 2023 16:46
Copy link
Copy Markdown
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

This is fine, but I think we could make a real code change instead of just a test change, retry drop table on Glue concurrent modification exceptions.

Usually it's not safe to retry unless you look at the changes from the other modification and reconcile them, but we're just dropping the table anyway so there shouldn't be anything to reconcile.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 13, 2023

/test-with-secrets sha=cfb2e40e654abc6b033cb6e4b3cfad8015e924a2

ebyhr
ebyhr previously requested changes Feb 13, 2023
@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch 2 times, most recently from 28fd876 to 5bc9a54 Compare February 14, 2023 06:42
@ebyhr ebyhr dismissed their stale review February 14, 2023 22:11

Addressed

@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch 2 times, most recently from 1f210b2 to e61de80 Compare February 14, 2023 22:33
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Usually you'd just say .handle(ConcurrentModificationException.class), unless the exception is wrapped in something else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is wrapped by a TrinoException

@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch 3 times, most recently from 8f34d2d to 32f2dbd Compare February 15, 2023 11:59
@findinpath findinpath added release-notes and removed no-release-notes This pull request does not require release notes entry labels Feb 15, 2023
@findinpath findinpath self-assigned this Feb 15, 2023
@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch from dfc7d59 to 1237526 Compare February 16, 2023 05:27
@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch from 1237526 to 9e2d9d3 Compare February 21, 2023 05:43
@findinpath
Copy link
Copy Markdown
Contributor Author

Rebasing on master to address conflicts.

@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch from 9e2d9d3 to 220b529 Compare February 21, 2023 05:49
Retry the query dropping the AWS Glue table, in the unlikely
situation that another query engine may be calling asynchronously
in the background an update operation on the table.
@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch from 220b529 to 2cced9b Compare February 21, 2023 05:56
Retry the query dropping the delta table on the Databricks cluster,
in the unlikely situation that the Databricks cluster may be
calling asynchronously in the background an update operation
on the table.
@findinpath findinpath force-pushed the findinpath/delta-databricks-retry-drop-table branch from 2cced9b to 58ae39e Compare February 21, 2023 05:59
@findinpath
Copy link
Copy Markdown
Contributor Author

@ebyhr I have addressed the comments. Could you please trigger a new round of running the build with secrets?

@findinpath findinpath requested a review from ebyhr February 21, 2023 06:00
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 21, 2023

/test-with-secrets sha=58ae39e511e35fa39dde6c78bea78b513d253f3b

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 21, 2023

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4230697367

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Table being modified concurrently happens in Glue tests

3 participants