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

Fix potentially incorrect table read version for writes on Delta Lake #21330

Merged

Conversation

findinpath
Copy link
Contributor

Description

In the context of concurrent operations, it may potentially happen that between the time when the version of table has been initially read for scanning and the time when the planning of the write operation begins that a new version of the table is current. Nevertheless, use in performing the write operations the version of the table initially read as reference for beginning the operation to ensure the consistency of the operation.

This strategy helps in unintentionally skipping the processing of transaction log information corresponding to other concurrent operations.

Additional context and related issues

Fixes #21324

Release notes

( ) This is not user-visible or is 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:

# Delta Lake
* Ensure read consistency while performing write operations. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Mar 30, 2024
@github-actions github-actions bot added the delta-lake Delta Lake connector label Mar 30, 2024
@findinpath findinpath requested review from ebyhr and pajaks March 30, 2024 07:48
@findinpath findinpath force-pushed the findinpath/delta-fix-concurrency-issue branch from 42f46bb to 9b75b58 Compare March 31, 2024 06:02
@findinpath findinpath self-assigned this Apr 1, 2024
@ebyhr ebyhr requested a review from alexjo2144 April 1, 2024 08:58
@ebyhr
Copy link
Member

ebyhr commented Apr 1, 2024

/test-with-secrets sha=9b75b58eb3ff26c91f98196be90aece77c33c617

Copy link

github-actions bot commented Apr 1, 2024

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

@findinpath findinpath force-pushed the findinpath/delta-fix-concurrency-issue branch from 9b75b58 to 14cfb6d Compare April 2, 2024 06:34
@findinpath findinpath requested a review from ebyhr April 2, 2024 06:34
@findinpath
Copy link
Contributor Author

@ebyhr pls trigger a new build with secrets

@ebyhr
Copy link
Member

ebyhr commented Apr 2, 2024

/test-with-secrets sha=14cfb6df6e2e1d2ce7f3655ed30db7fe801c391c

Copy link

github-actions bot commented Apr 2, 2024

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8518514728

@findinpath
Copy link
Contributor Author

From the build https://github.com/trinodb/trino/actions/runs/8518514728/job/23330834685 , I see that the delta-lake cloud-tests finished successfully.

table.getMetadataEntry(),
table.getProtocolEntry(),
inputColumns,
getMandatoryCurrentVersion(fileSystem, tableLocation, table.getReadVersion()),
Copy link
Member

Choose a reason for hiding this comment

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

agreed, this line was incorrect

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

A question about tests.

In the context of concurrent operations, it may potentially happen
that between the time when the version of table has been initially
read for scanning and the time when the planning of the write
operation begins that a new version of the table is current.
Nevertheless, use in performing the write operations the version
of the table initially read as reference for beginning the
operation to ensure the consistency of the operation.

This strategy helps in unintentionally skipping the processing
of transaction log information corresponding to other concurrent
operations.
Without the fix in the previous commit, in the concurrency
tests for the non-blind INSERT operations it happened sometime that
the source table handles processed in `finishInsert` operation
were wrongly skipped because they were considered time travel
references of the target table which led to the situation in
which the INSERT operation was inaccurately considered blind.
@findinpath findinpath force-pushed the findinpath/delta-fix-concurrency-issue branch from 14cfb6d to 1941f3f Compare April 2, 2024 19:05
@findinpath findinpath changed the title Ensure read consistency while performing writes on Delta Lake Fix potentially incorrect table read version for writes on Delta Lake Apr 2, 2024
@ebyhr ebyhr merged commit fe82520 into trinodb:master Apr 3, 2024
25 checks passed
@github-actions github-actions bot added this to the 444 milestone Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed delta-lake Delta Lake connector
Development

Successfully merging this pull request may close these issues.

Delta Lake Inconsistent INSERT outcome in the context of performing concurrent operations
3 participants