Solve race condition in LocalPageStore causing PageCorruptedExceptions - #18752
Open
ELBouslama wants to merge 1 commit into
Open
Solve race condition in LocalPageStore causing PageCorruptedExceptions#18752ELBouslama wants to merge 1 commit into
ELBouslama wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your pull request. |
Contributor
|
Automated checks report:
Some checks failed. Please fix the reported issues and reply |
Author
|
alluxio-bot, check this please |
Contributor
|
Automated checks report:
Some checks failed. Please fix the reported issues and reply |
Contributor
|
Automated checks report:
All checks passed! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
This PR switches LocalPageStore.put() to an atomic write-then-rename approach. Instead of writing directly to the final cache file path, we now write to a temporary .tmp file, sync it to disk, and then atomically rename it into place.
This way, any concurrent reader will either see the old complete file or the new complete file — never a half-written one that triggers PageCorruptedException.
If anything goes wrong during the write, the temp file is cleaned up so we don't leave stale artifacts behind.
Why are the changes needed?
We've been hitting PageCorruptedException in production under concurrent read/write workloads. The root cause is straightforward: a reader can open a cache file while a writer is still flushing data to it, resulting in a truncated read.
The write-to-temp + atomic rename pattern is a well-established solution for this class of problem. It guarantees readers only ever observe fully-written pages, eliminating the race condition entirely.
Related to this issue on Trino.
trinodb/trino#25899