Create Table Concurrent query Failure handling in Delta Lake #24250
Conversation
vinay-kl
changed the title
vinay-kl
force-pushed
the
databricks-create-table-concurrent-fix
branch
from
078d491
to
c7628b3
Compare
| @@ -1263,7 +1263,8 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe | |||
| statisticsAccess.deleteExtendedStatistics(session, schemaTableName, location); | |||
| } | |||
| else { | |||
| setRollback(() -> deleteRecursivelyIfExists(fileSystem, deltaLogDirectory)); | |||
| // deleteRecursivelyIfNothingExists ensures current CREATE TABLE doesn't delete directory if there's a conflict | |||
There was a problem hiding this comment.
Why aren't we checking in the catch clause whether we're dealing with a TransactionConflictException instead?
By doing this, we'd likely know whether we're in a concurrency situation.
There was a problem hiding this comment.
@findinpath the rollback happens in a different thread AFAIK, the exception context needs to be passed on as well. Also the rollback initialisation happens in beginCreateTable & createTable calls which is much prior to finishCreateTable which is later.
| } | ||
|
|
||
| @Test | ||
| public void testConcurrentCreateTableAsSelect() |
There was a problem hiding this comment.
Could you move this test to TestDeltaLakeLocalConcurrentWritesTest?
There was a problem hiding this comment.
@ebyhr should it be considered as write?, thought as it's a table getting created first time, thought let me keep it here.
There was a problem hiding this comment.
Yes, we treat CTAS as a write operation. Please merge the test class.
There was a problem hiding this comment.
@ebyhr TestDeltaLakeLocalConcurrentWritesTest's queryRunner is instantiated with delta.unique-table-location=true, These test-cases which is part of CTAS requires delta.unique-table-location=true.
How to proceed here? as these aren't possible to set via session props as well.
ebyhr
changed the title
Description
Create Table [as select] concurrent query failure handling
Additional context and related issues
Fixes #24153
Release notes
( ) 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: