diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py index 32056a35..59039522 100644 --- a/osbuild/util/fscache.py +++ b/osbuild/util/fscache.py @@ -683,8 +683,11 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): self._info_maximum_size = -1 elif isinstance(info.maximum_size, int): self._info_maximum_size = info.maximum_size - else: + elif info.maximum_size is None: self._info_maximum_size = 0 + else: + raise ValueError( + f"maximum-size can only be set to 'unlimited' or an integer value, got {type(info.maximum_size)}") def _is_active(self): # Internal helper to verify we are in an active context-manager. @@ -942,19 +945,27 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): info["creation-boot-id"] = self._bootid info["size"] = self._calculate_space(path_data) - # Update the total cache-size. If it exceeds the limits, bail out - # but do not trigger an error. It behaves as if the entry was - # committed and immediately deleted by racing cache management. No - # need to tell the caller about it (if that is ever needed, we can - # provide for it). + # Exit early if it never is going to fit + if self._info_maximum_size > -1 and info["size"] > self._info_maximum_size: + return + + # Update the total cache-size. If it exceeds the limits, remove + # least recently used objects until there is enough space. # # Note that if we crash after updating the total cache size, but # before committing the object information, the total cache size - # will be out of sync. However, it is never overcommitted, so we - # will never violate any cache invariants. The cache-size will be - # re-synchronized by any full cache-management operation. + # will be out of sync. + # + # However, it is never overcommitted, so we will never + # violate any cache invariants. Future code needs to resync + # the cache (e.g. on open with some simple journal strategy). if not self._update_cache_size(info["size"]): - return + # try to free space + self._remove_lru(info["size"]) + # and see if the update can happen now + if not self._update_cache_size(info["size"]): + # stil could not free enough space + return try: # Commit the object-information, thus marking it as fully @@ -1146,7 +1157,6 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): break except BlockingIOError: continue - # return True if at least the required size got freed return freed_so_far > required_size diff --git a/test/mod/test_util_fscache.py b/test/mod/test_util_fscache.py index 35478638..b515d957 100644 --- a/test/mod/test_util_fscache.py +++ b/test/mod/test_util_fscache.py @@ -465,38 +465,55 @@ def test_cache_load_updates_last_used_on_noatime(tmp_path): test_cache_load_updates_last_used(mnt_path) +@pytest.mark.skipif(not has_precise_fs_timestamps(), reason="need precise fs timestamps") def test_cache_full_behavior(tmp_path): + def _cache_size_from_file(): + with open(cache._path(cache._filename_cache_size), encoding="utf8") as fp: + return json.load(fp) + cache = fscache.FsCache("osbuild-cache-evict", tmp_path) with cache: - # use big sizes to mask the effect of dirs using 4k of space too - cache.info = cache.info._replace(maximum_size=192 * 1024) - # add one object to the store, we are below the limit - with cache.store("o1") as rpath: - rpath_f1 = os.path.join(tmp_path, rpath, "f1") - with open(rpath_f1, "wb") as fp: - fp.write(b'a' * 64 * 1024) - assert cache._calculate_space(tmp_path) > 64 * 1024 - assert cache._calculate_space(tmp_path) < 128 * 1024 - with cache.load("o1") as o: - assert o != "" - # and one more - with cache.store("o2") as rpath: - rpath_f2 = os.path.join(tmp_path, rpath, "f2") - with open(rpath_f2, "wb") as fp: - fp.write(b'b' * 64 * 1024) - assert cache._calculate_space(tmp_path) > 128 * 1024 - assert cache._calculate_space(tmp_path) < 192 * 1024 - with cache.load("o2") as o: - assert o != "" - # adding a third one will (silently) fail because the cache is full - with cache.store("o3") as rpath: - rpath_f3 = os.path.join(tmp_path, rpath, "f3") - with open(rpath_f3, "wb") as fp: - fp.write(b'b' * 128 * 1024) - assert cache._calculate_space(tmp_path) > 128 * 1024 - assert cache._calculate_space(tmp_path) < 192 * 1024 - with pytest.raises(fscache.FsCache.MissError): - with cache.load("o3") as o: + # use big sizes to mask the effect of {dirs,cache.info} using + # 4k of space too + obj_size = 128 * 1024 + # cache is big enough to hold 4 objects (add buffer) + max_cache_size = (4 * obj_size) + int(0.5 * obj_size) + cache.info = cache.info._replace(maximum_size=max_cache_size) + # add 4 object to the store, we are below the limit + for i in range(1, 5): + with cache.store(f"o{i}") as rpath: + rpath = os.path.join(tmp_path, rpath, f"f{i}") + with open(rpath, "wb") as fp: + fp.write(b'a' * obj_size) + # info file is updated + assert _cache_size_from_file() >= i * obj_size + assert _cache_size_from_file() < (i + 1) * obj_size + # disk is updated + assert cache._calculate_space(tmp_path) >= i * obj_size + assert cache._calculate_space(tmp_path) < (i + 1) * obj_size + with cache.load(f"o{i}") as o: + assert o != "" + sleep_for_fs() + # adding one more + with cache.store("o-full") as rpath: + rpath = os.path.join(tmp_path, rpath, "f-full") + with open(rpath, "wb") as fp: + fp.write(b'b' * obj_size) + # cache file is updated, it will free twice the size of the + # requested obj + assert _cache_size_from_file() >= 2 * obj_size + assert _cache_size_from_file() < max_cache_size + # disk is updated + assert cache._calculate_space(tmp_path) >= 3 * obj_size + assert cache._calculate_space(tmp_path) < max_cache_size + # o1,o2 is least recently used and got removed + for obj in ["o1", "o2"]: + with pytest.raises(fscache.FsCache.MissError): + with cache.load(obj) as o: + pass + # and o-full made it in + for obj in ["o3", "o4", "o-full"]: + with cache.load(obj) as o: pass @@ -556,3 +573,16 @@ def test_cache_remove_lru(tmpdir): # and keep removing is fine cache._remove_lru(1) assert sorted_objs == [] + + +def test_cache_obj_too_big(tmp_path): + cache = fscache.FsCache("osbuild-cache-evict", tmp_path) + with cache: + max_cache_size = 16 * 1024 + cache.info = cache.info._replace(maximum_size=max_cache_size) + with cache.store("o1") as rpath: + with open(os.path.join(tmp_path, rpath, "f1"), "wb") as fp: + fp.write(b'a' * 2 * max_cache_size) + with pytest.raises(fscache.FsCache.MissError): + with cache.load("o1"): + pass