Fix non-atomic file writes that can corrupt cached data - #212
Open
h0bnobs wants to merge 1 commit into
Open
Conversation
Writes to the parquet price cache, stock_info.json and pickled model files went directly to the final path, so a crash mid-write (OOM, SIGKILL, disk full) left a corrupted file that the next run would read. Add an atomic_write context manager in src/util that writes to a temp file in the destination directory and renames it into place with os.replace, removing the temp file on failure so existing files are never clobbered by a partial write. Fixes #127 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Description
Fixes #127.
Writes to the parquet price cache,
stock_info.jsonand pickled model files were made directly to the final path. If the process crashed mid-write (OOM, SIGKILL, disk full), the file was left in a corrupted partial state, and the next run would either fail to read it or silently load malformed data.This PR adds an
atomic_writecontext manager insrc/util/atomic_io.pythat writes to a temporary file in the destination directory and atomically renames it into place withos.replace. On failure the temp file is removed, so an existing file is never clobbered by a partial write. Readers therefore only ever observe either the old complete file or the new complete file.The issue referenced a
df.to_csv()call that has since becomedf.to_parquet(), but the underlying non-atomic write remained and is fixed here.Changes in this pull request
atomic_writecontext manager insrc/util/atomic_io.py, exported fromsrc.utilStockDataDownloader._download_singlestock_info.jsoninStockDataDownloader.save_stock_infoModel.save_modelandStackedStockPredictor.save_modelatomic_writecovering success, replacement, failure rollback and temp-file cleanupsave_stock_infopreserves the existing cache file🤖 Generated with Claude Code