Skip to content

Commit d03c005

Browse files
7487claude
andcommitted
fix: write file cache entries atomically to avoid read races
FileCache.put opened the cache entry with "wb", truncating it in place. A concurrent reader (e.g. another pytest-xdist worker sharing the cache directory) could open the file between the truncate and the completed write and fail to unpickle it ("Ran out of input"), causing flaky Windows CI runs such as test_multi_repo_macro_references. Write the gzip/pickle payload to a temp file in the cache directory and os.replace it onto the entry path so readers only ever see complete entries. The replace is best-effort: on Windows it can fail if a reader still has the target open, in which case the existing entry is left intact. Fixes #6010 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: 7487 <1042653432@qq.com>
1 parent 869248c commit d03c005

2 files changed

Lines changed: 37 additions & 2 deletions

File tree

‎sqlmesh/utils/cache.py‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
import gzip
44
import logging
5+
import os
56
import pickle
67
import shutil
8+
import tempfile
79
import typing as t
810
from pathlib import Path
911

@@ -125,8 +127,25 @@ def put(self, name: str, entry_id: str = "", *, value: T) -> None:
125127
if not self._path.is_dir():
126128
raise SQLMeshError(f"Cache path '{self._path}' is not a directory.")
127129

128-
with gzip.open(self._cache_entry_path(name, entry_id), "wb", compresslevel=1) as fd:
129-
pickle.dump(value, fd)
130+
# Write to a temporary file and then atomically move it into place. Writing the
131+
# entry in place ("wb" truncates the target file first) means that a concurrent
132+
# reader (e.g. another pytest-xdist worker) could observe a partially written
133+
# entry and fail to unpickle it.
134+
tmp_fd, tmp_name = tempfile.mkstemp(dir=self._path, prefix=f"{self._cache_version}__tmp")
135+
try:
136+
with os.fdopen(tmp_fd, "wb") as raw_fd:
137+
with gzip.open(raw_fd, "wb", compresslevel=1) as fd:
138+
pickle.dump(value, fd)
139+
os.replace(tmp_name, self._cache_entry_path(name, entry_id))
140+
except OSError as ex:
141+
# Storing an entry is best-effort; e.g. on Windows os.replace fails if a
142+
# concurrent reader still has the target file open.
143+
logger.warning("Failed to store a cache entry '%s': %s", name, ex)
144+
finally:
145+
try:
146+
os.unlink(tmp_name)
147+
except OSError:
148+
pass
130149

131150
def exists(self, name: str, entry_id: str = "") -> bool:
132151
"""Returns true if the cache entry with the given name and ID exists, false otherwise.

‎tests/utils/test_cache.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@ def test_file_cache(tmp_path: Path, mocker: MockerFixture):
4242
assert "客户数据" in cache._cache_entry_path("客户数据").name
4343

4444

45+
def test_file_cache_put_is_atomic(tmp_path: Path, mocker: MockerFixture) -> None:
46+
cache: FileCache[_TestEntry] = FileCache(tmp_path)
47+
48+
old_entry = _TestEntry(value="old")
49+
cache.put("test_name", value=old_entry)
50+
51+
# Simulate os.replace failing, e.g. on Windows when a concurrent reader still has the
52+
# target file open. The existing entry must never be truncated / partially overwritten.
53+
mocker.patch("sqlmesh.utils.cache.os.replace", side_effect=PermissionError("file in use"))
54+
cache.put("test_name", value=_TestEntry(value="new"))
55+
56+
assert cache.get("test_name") == old_entry
57+
# The temporary file should have been cleaned up.
58+
assert len(list(tmp_path.glob("*"))) == 1
59+
60+
4561
def test_optimized_query_cache(tmp_path: Path, mocker: MockerFixture):
4662
model = SqlModel(
4763
name="test_model",

0 commit comments

Comments
 (0)