Skip to content

Commit 662ee1f

Browse files
committed
Add regression tests for per-file diskcache reuse when streaming remote files
Streaming a proxied remote file (RemoteFile) reopened the per-file chunk diskcache on essentially every read block, which is cheap on fast storage but pathologically slow on SD cards. Cover: - the diskcache is constructed a bounded number of times regardless of file size (not once per chunk) - concurrent readers of the same file, and single-read correctness - the handle self-heals when the chunk directory is evicted and recreated mid-stream, including dropping a borrowed handle without closing it - the self-heal mechanism and Windows-safety invariant cross-platform: _open_cache reopens on an incarnation change, and delete() releases the handle before removing the directory - the marker-file fallback used when the filesystem reports no inode - eviction skips a directory that cannot be removed rather than aborting The end-to-end deletion flavors are POSIX-only, since Windows forbids removing a directory whose diskcache is held open; tests release the handle before removing a directory so it does not block removal there. Claude-Session: https://claude.ai/code/session_01MnAmUbaCBpL4wBKFYUyCwt
1 parent f7cafd4 commit 662ee1f

2 files changed

Lines changed: 352 additions & 2 deletions

File tree

kolibri/utils/tests/test_chunked_file.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import tempfile
66
import unittest
77

8+
from mock import patch
9+
810
from kolibri.utils.file_transfer import CHUNK_SUFFIX
911
from kolibri.utils.file_transfer import ChunkedFile
1012
from kolibri.utils.file_transfer import ChunkedFileDirectoryManager
@@ -48,6 +50,9 @@ def setUp(self):
4850
self.data = _write_test_data_to_chunked_file(self.chunked_file)
4951

5052
def tearDown(self):
53+
# Release the reused diskcache handle before removing the directory, or
54+
# the open SQLite file blocks removal on Windows and pollutes later tests.
55+
self.chunked_file.close()
5156
shutil.rmtree(self.chunked_file.chunk_dir, ignore_errors=True)
5257
shutil.rmtree(self.file_path, ignore_errors=True)
5358

@@ -167,7 +172,9 @@ def test_get_missing_chunk_ranges_slice(self):
167172
self.assertEqual(missing_ranges, expected_ranges)
168173

169174
def test_get_missing_chunk_ranges_slice_no_download(self):
170-
# Remove some chunks
175+
# Remove some chunks. Release our handle first so the directory can be
176+
# removed on Windows (an open SQLite file blocks removal there).
177+
self.chunked_file.close()
171178
shutil.rmtree(self.chunked_file.chunk_dir)
172179

173180
start = self.chunk_size
@@ -182,7 +189,9 @@ def test_get_missing_chunk_ranges_slice_no_download(self):
182189
self.assertEqual(missing_ranges, expected_ranges)
183190

184191
def test_get_missing_chunk_ranges_slice_no_download_not_chunk_size_ranges(self):
185-
# Remove some chunks
192+
# Remove some chunks. Release our handle first so the directory can be
193+
# removed on Windows (an open SQLite file blocks removal there).
194+
self.chunked_file.close()
186195
shutil.rmtree(self.chunked_file.chunk_dir)
187196

188197
start = self.chunk_size // 3
@@ -274,6 +283,9 @@ def test_finalize_file_md5(self):
274283
os.remove(self.file_path)
275284

276285
def test_file_removed_by_parallel_process_after_opening(self):
286+
# Release our handle so the directory can actually be removed on Windows
287+
# (an open SQLite file blocks removal there).
288+
self.chunked_file.close()
277289
shutil.rmtree(self.chunked_file.chunk_dir, ignore_errors=True)
278290
self.chunked_file._file_size = None
279291
with self.assertRaises(ChunkedFileDoesNotExist):
@@ -391,6 +403,29 @@ def test_evict_files_more_than_file_size_sum(self):
391403
sorted([]),
392404
)
393405

406+
def test_evict_files_skips_undeletable_directory(self):
407+
# If a directory cannot be removed (e.g. a diskcache handle held open
408+
# while streaming, which blocks removal on Windows), eviction must skip
409+
# it - not count it as freed, and not abort the whole pass.
410+
manager = ChunkedFileDirectoryManager(self.base_dir)
411+
blocked = sorted(manager._get_chunked_file_dirs())[0]
412+
real_rmtree = shutil.rmtree
413+
414+
def guarded_rmtree(path, *args, **kwargs):
415+
if path == blocked:
416+
raise OSError("directory in use")
417+
return real_rmtree(path, *args, **kwargs)
418+
419+
with patch(
420+
"kolibri.utils.file_transfer.shutil.rmtree", side_effect=guarded_rmtree
421+
):
422+
evicted = manager.evict_files(TOTAL_CHUNKED_FILE_SIZE * 3)
423+
424+
# Two of the three directories are evicted; the blocked one is skipped
425+
# and its size is not counted toward the freed total.
426+
self.assertEqual(evicted, TOTAL_CHUNKED_FILE_SIZE * 2)
427+
self.assertEqual(sorted(manager._get_chunked_file_dirs()), [blocked])
428+
394429
def test_evict_files_exact_file_size(self):
395430
manager = ChunkedFileDirectoryManager(self.base_dir)
396431
self.assertEqual(

0 commit comments

Comments
 (0)