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(rust): implement abort commit for S3DynamoDBLogStore #2452

Merged
merged 3 commits into from
May 4, 2024

Conversation

PeterKeDer
Copy link
Contributor

Description

This PR ensures when using S3DynamoDBLogStore, temp commit files are only deleted when the DynamoDB log entry is deleted.

Add abort_commit_entry(version, tmp_commit) to LogStore

  • By default, this just deletes the tmp_commit file
  • For S3DynamoDBLogStore, it first deletes the DynamoDB entry. Then, it only deletes tmp_commit if the entry is deleted successfully

Context

This fixes a rare bug when using DynamoDB as the locking provider. Consider the following events:

  1. S3DynamoDbLogStore.write_commit_entry succeeds at creating a DynamoDB entry, but fails during repair_entry
  2. The error handling deletes the tmp_commit file
  3. The next time repair_entry is called on this entry, it will lead to ObjectStoreError::NotFound since the temp commit file was deleted
  4. We infer the temp commit json was already moved, so we update the DynamoDB entry as complete

This results in a complete DynamoDB log entry, even though the corresponding transaction log does not exist in _delta_log.

Related Issue(s)

@rtyler rtyler force-pushed the peter/abort-commit-entry branch from 45c3248 to 127fd55 Compare May 4, 2024 15:50
@rtyler rtyler enabled auto-merge (rebase) May 4, 2024 15:51
@rtyler rtyler merged commit e7af965 into delta-io:main May 4, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate storage/aws AWS S3 storage related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants