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 uncommitted file clean-up in transactions #218

Merged
merged 3 commits into from Jun 23, 2019

Conversation

Projects
None yet
2 participants
@rdblue
Copy link
Contributor

commented Jun 14, 2019

This adds a callback for deleting files to SnapshotProducer and uses that callback in Transaction
to track deletes instead of running them. When a transaction is committed, the last set of deletes are run so that the actual deletes are for the last commit of each operation in the transaction.

This ensures that unused metadata files are not deleted when operations are committed to the transaction. The set of files that needs to be deleted may change, so only the last set of deletes in a transaction should be run.

This was discovered when appending a manifest file using #201. A manifest was copied, merged with existing manifests, and deleted when committing a change to the transaction. When the transaction committed, it conflicted and re-ran the manifest file append. Manifests from the conflicting commit were merged into existing manifests instead of the copy of the appended manifest, so the copy that had already been deleted was used in table metadata.

rdblue added some commits Jun 14, 2019

Fix uncommitted file clean-up in transactions.
This adds a callback to delete files and adds a callback in Transaction
that keeps track of deletes instead of running them. When a transaction
is committed, the last set of deletes are run so that the actual deletes
are for the last commit of each operation in the transaction.
@rdblue

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@danielcweeks, when you have time could you review this?

@danielcweeks

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

+1

@danielcweeks danielcweeks merged commit 97485a0 into apache:master Jun 23, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.