Skip to content

Commit 6cf9e98

Browse files
Write spike clusters atomically to avoid truncation on crash
np.save() opens the destination for writing and truncates it immediately, so a process that dies, is killed, or fills the disk part way through save_spike_clusters() destroys the existing spike_clusters.npy and leaves an incomplete one in its place. That is unrecoverable loss of a manual curation session, as reported in cortex-lab/phy#1249. Write the array to a temporary file in the same directory as the destination, fsync it, and move it into place with os.replace(), which is atomic when both paths are on the same filesystem. If the write fails the temporary file is removed and the existing file is untouched.
1 parent d99464b commit 6cf9e98

3 files changed

Lines changed: 73 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
- Development checkouts now report a development version with a Git commit suffix.
1515
- Template datasets without waveform templates can be reloaded after cluster assignments change,
1616
and stored spike-waveform subsets provide their waveform sample count.
17+
- Cluster assignments are written atomically, so a crash during a save no longer truncates `spike_clusters.npy`.
1718

1819
## [2.7.0] 2025-12-10
1920

‎phylib/io/model.py‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from operator import itemgetter
1414
from pathlib import Path
1515
import shutil
16+
import tempfile
1617

1718
import numpy as np
1819
# from numpy.lib.format import open_memmap
@@ -62,6 +63,36 @@ def write_array(name, arr):
6263
np.save(name, arr)
6364

6465

66+
def _save_npy_atomic(path, arr):
67+
"""Save an array to a npy file without ever truncating the existing file.
68+
69+
The array is written to a temporary file in the same directory as the destination, and only
70+
then moved into place with `os.replace()`, which is atomic as long as both paths are on the
71+
same filesystem. If anything goes wrong while writing, the temporary file is removed and the
72+
existing file is left untouched.
73+
74+
"""
75+
path = Path(path)
76+
# NOTE: the temporary file must not be named `*.npy`, otherwise a leftover file would be
77+
# picked up by the `spike_*.npy` glob in `_load_spike_attributes()`.
78+
tmp = tempfile.NamedTemporaryFile(
79+
delete=False, dir=str(path.parent), prefix='.' + path.name + '.', suffix='.tmp')
80+
try:
81+
with tmp:
82+
# NOTE: np.save() appends `.npy` to a file *name* that lacks it, but not to an
83+
# already-open file object, which is what is passed here.
84+
np.save(tmp, arr)
85+
tmp.flush()
86+
os.fsync(tmp.fileno())
87+
os.replace(tmp.name, path)
88+
except BaseException:
89+
try:
90+
os.remove(tmp.name)
91+
except OSError: # pragma: no cover
92+
pass
93+
raise
94+
95+
6596
def from_sparse(data, cols, channel_ids):
6697
"""Convert a sparse structure into a dense one.
6798
@@ -1357,7 +1388,7 @@ def save_spike_clusters(self, spike_clusters):
13571388
"""Save the spike clusters."""
13581389
path = self._find_path('spike_clusters.npy', 'spikes.clusters.npy', multiple_ok=False)
13591390
logger.debug("Save spike clusters to `%s`.", path)
1360-
np.save(path, spike_clusters)
1391+
_save_npy_atomic(path, spike_clusters)
13611392

13621393
def save_spikes_subset_waveforms(self, max_n_spikes_per_template=None, max_n_channels=None,
13631394
sample2unit=1.):

‎phylib/io/tests/test_model.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
# from phylib.utils import Bunch
1616
from phylib.utils.testing import captured_output
17+
from .. import model as model_module
1718
from ..model import from_sparse, load_model
1819

1920
logger = logging.getLogger(__name__)
@@ -135,6 +136,45 @@ def test_model_save(template_model_full):
135136
m.save_spike_clusters(m.spike_clusters)
136137

137138

139+
def test_model_save_spike_clusters_atomic(template_model, monkeypatch):
140+
# Regression test for cortex-lab/phy#1249: a save that dies half-way through must not
141+
# leave spike_clusters.npy truncated or empty.
142+
m = template_model
143+
path = m.dir_path / 'spike_clusters.npy'
144+
145+
# Happy path: a successful save round-trips and does not create `spike_clusters.npy.npy`.
146+
original = m.spike_clusters.copy()
147+
m.save_spike_clusters(original)
148+
assert not (m.dir_path / 'spike_clusters.npy.npy').exists()
149+
ae(np.load(path), original)
150+
151+
original_bytes = path.read_bytes()
152+
assert original_bytes
153+
files_before = sorted(p.name for p in m.dir_path.iterdir())
154+
155+
def _partial_save(file, arr, *args, **kwargs):
156+
# Mimic np.save() dying half-way through: whatever it was given is opened for writing
157+
# (which truncates it) and a few bytes are written before the failure.
158+
if hasattr(file, 'write'):
159+
file.write(b'\x93NUMPY')
160+
else:
161+
with open(str(file), 'wb') as f:
162+
f.write(b'\x93NUMPY')
163+
raise IOError("No space left on device")
164+
165+
monkeypatch.setattr(model_module.np, 'save', _partial_save)
166+
with raises(IOError):
167+
m.save_spike_clusters(original + 1)
168+
169+
# The original file must still be there, byte for byte.
170+
assert path.exists()
171+
assert path.read_bytes() == original_bytes
172+
ae(np.load(path), original)
173+
174+
# No temporary file must be left behind.
175+
assert sorted(p.name for p in m.dir_path.iterdir()) == files_before
176+
177+
138178
def test_model_reload_curated_dataset_without_templates(template_path):
139179
model = load_model(template_path)
140180
model.save_spikes_subset_waveforms(10, 16)

0 commit comments

Comments
 (0)