diff --git a/system/loggerd/deleter.py b/system/loggerd/deleter.py index eb8fd35f21813c..d6f64f4613c915 100755 --- a/system/loggerd/deleter.py +++ b/system/loggerd/deleter.py @@ -51,6 +51,26 @@ def deleter_thread(exit_event: threading.Event): out_of_percent = get_available_percent(default=MIN_PERCENT + 1) < MIN_PERCENT if out_of_percent or out_of_bytes: + # delete stray files or broken symlinks first + # fixes issue where non-directories take up space but get ignored + all_items = os.listdir(Paths.log_root()) + file_deleted = False + + for item in all_items: + item_path = os.path.join(Paths.log_root(), item) + if not os.path.isdir(item_path) and not item.endswith(".lock"): + try: + cloudlog.info(f"deleting stray item {item_path}") + os.remove(item_path) + file_deleted = True + break # re-evaluate space usage upon file deletion + except OSError: + cloudlog.exception(f"issue deleting stray {item_path}") + + if file_deleted: + exit_event.wait(.1) + continue + dirs = listdir_by_creation(Paths.log_root()) preserved_dirs = get_preserved_segments(dirs) @@ -58,7 +78,11 @@ def deleter_thread(exit_event: threading.Event): for delete_dir in sorted(dirs, key=lambda d: (d in DELETE_LAST, d in preserved_dirs)): delete_path = os.path.join(Paths.log_root(), delete_dir) - if any(name.endswith(".lock") for name in os.listdir(delete_path)): + try: + if any(name.endswith(".lock") for name in os.listdir(delete_path)): + continue + except OSError: + cloudlog.exception(f"issue listing {delete_path}") continue try: diff --git a/system/loggerd/tests/test_deleter.py b/system/loggerd/tests/test_deleter.py index 6222ea253ba0db..2f8428499c1dd0 100644 --- a/system/loggerd/tests/test_deleter.py +++ b/system/loggerd/tests/test_deleter.py @@ -1,5 +1,6 @@ import time import threading +import os from collections import namedtuple from pathlib import Path from collections.abc import Sequence @@ -7,6 +8,7 @@ import openpilot.system.loggerd.deleter as deleter from openpilot.common.timeout import Timeout, TimeoutException from openpilot.system.loggerd.tests.loggerd_tests_common import UploaderTestCase +from openpilot.system.hardware.hw import Paths Stats = namedtuple("Stats", ['f_bavail', 'f_blocks', 'f_frsize']) @@ -65,30 +67,34 @@ def assertDeleteOrder(self, f_paths: Sequence[Path], timeout: int = 5) -> None: assert deleted_order == f_paths, "Files not deleted in expected order" def test_delete_order(self): - self.assertDeleteOrder([ - self.make_file_with_data(self.seg_format.format(0), self.f_type), - self.make_file_with_data(self.seg_format.format(1), self.f_type), - self.make_file_with_data(self.seg_format2.format(0), self.f_type), - ]) + self.assertDeleteOrder( + [ + self.make_file_with_data(self.seg_format.format(0), self.f_type), + self.make_file_with_data(self.seg_format.format(1), self.f_type), + self.make_file_with_data(self.seg_format2.format(0), self.f_type), + ] + ) def test_delete_many_preserved(self): - self.assertDeleteOrder([ - self.make_file_with_data(self.seg_format.format(0), self.f_type), - self.make_file_with_data(self.seg_format.format(1), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE), - self.make_file_with_data(self.seg_format.format(2), self.f_type), - ] + [ - self.make_file_with_data(self.seg_format2.format(i), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE) - for i in range(5) - ]) + self.assertDeleteOrder( + [ + self.make_file_with_data(self.seg_format.format(0), self.f_type), + self.make_file_with_data(self.seg_format.format(1), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE), + self.make_file_with_data(self.seg_format.format(2), self.f_type), + ] + + [self.make_file_with_data(self.seg_format2.format(i), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE) for i in range(5)] + ) def test_delete_last(self): - self.assertDeleteOrder([ - self.make_file_with_data(self.seg_format.format(1), self.f_type), - self.make_file_with_data(self.seg_format2.format(0), self.f_type), - self.make_file_with_data(self.seg_format.format(0), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE), - self.make_file_with_data("boot", self.seg_format[:-4]), - self.make_file_with_data("crash", self.seg_format2[:-4]), - ]) + self.assertDeleteOrder( + [ + self.make_file_with_data(self.seg_format.format(1), self.f_type), + self.make_file_with_data(self.seg_format2.format(0), self.f_type), + self.make_file_with_data(self.seg_format.format(0), self.f_type, preserve_xattr=deleter.PRESERVE_ATTR_VALUE), + self.make_file_with_data("boot", self.seg_format[:-4]), + self.make_file_with_data("crash", self.seg_format2[:-4]), + ] + ) def test_no_delete_when_available_space(self): f_path = self.make_file_with_data(self.seg_dir, self.f_type) @@ -115,3 +121,58 @@ def test_no_delete_with_lock_file(self): self.join_thread() assert f_path.exists(), "File deleted when locked" + + def test_delete_mixed_contents(self): + fake_seg_path = os.path.join(Paths.log_root(), "2024-05-20--12-00-00--0") + with open(fake_seg_path, 'w') as f: + f.write("file masquerading as a directory") + + stray_log_path = os.path.join(Paths.log_root(), "random.log") + with open(stray_log_path, 'w') as f: + f.write("data") + + broken_link_path = os.path.join(Paths.log_root(), "broken_link") + try: + os.symlink("does_not_exist", broken_link_path) + except OSError: + pass + + real_dir = os.path.join(Paths.log_root(), "real_dir") + os.mkdir(real_dir) + + nested = os.path.join(real_dir, "a", "b", "c") + os.makedirs(nested, exist_ok=True) + for i in range(2): + with open(os.path.join(real_dir, f"root_{i}.txt"), "w") as f: + f.write("x") + with open(os.path.join(nested, f"leaf_{i}.txt"), "w") as f: + f.write("y") + + valid_link_path = os.path.join(Paths.log_root(), "valid_link") + try: + os.symlink(real_dir, valid_link_path) + except OSError: + pass + + valid_seg_path = self.make_file_with_data(self.seg_format.format(1), self.f_type, 1) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for mixed deletions"): + while ( + os.path.exists(fake_seg_path) + or os.path.exists(stray_log_path) + or os.path.exists(broken_link_path) + or valid_seg_path.exists() + ): + time.sleep(0.01) + finally: + self.join_thread() + + assert not os.path.exists(fake_seg_path) + assert not os.path.exists(stray_log_path) + assert not os.path.exists(broken_link_path) + assert not valid_seg_path.exists() + + # valid alias should remain as the target is a real directory tree + assert os.path.exists(valid_link_path)