Skip to content

Commit

Permalink
osbuild: update LRU cache eviction PR with latest from upstream
Browse files Browse the repository at this point in the history
  • Loading branch information
dustymabe committed Feb 1, 2024
1 parent 758179e commit 3ea3e92
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 69 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From 0fd9104f51b8d8a58bfa7e9a32817a6f907fe166 Mon Sep 17 00:00:00 2001
From 4a9831fa889b073ccb14568c4571f57d28dfe84f Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
Date: Tue, 12 Dec 2023 21:20:42 +0100
Subject: [PATCH 1/4] fscache: add new `FsCache._last_used_objs()' helper
Expand Down
48 changes: 34 additions & 14 deletions src/0002-fscache-add-FsCache._remove_lru-to-remove-entries.patch
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
From 056d2fd5a419ab5f5974864b513230d3d133f7cf Mon Sep 17 00:00:00 2001
From fff7dd5e1ab967165c16edec66b989adc70e39b7 Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
Date: Wed, 13 Dec 2023 10:07:32 +0100
Subject: [PATCH 2/4] fscache: add FsCache._remove_lru() to remove entries

The FsCache._remove_lru() removes the least recently used entry
from the cache.
---
osbuild/util/fscache.py | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
osbuild/util/fscache.py | 64 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py
index 58c9a310..21e03f95 100644
index 58c9a310..32056a35 100644
--- a/osbuild/util/fscache.py
+++ b/osbuild/util/fscache.py
@@ -1086,6 +1086,50 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
@@ -1086,6 +1086,70 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
objs.append(FsCacheObjectInfo(name=name, last_used=last_used))
return sorted(objs, key=lambda obj: obj.last_used)

+ def _remove_lru(self):
+ """"Remove the least recently used entry from the cache."""
+ def _remove_lru(self, required_size):
+ """"
+ Make room in the cache for "required_size" by remove the least
+ recently used entry from the cache. Note that the cache may
+ clear more than required_size.
+ """
+ # To avoid having to take a global cache lock the strategy is:
+ # 1. Get list of (object, last_used) sorted from oldest to newest.
+ # This is racy so we need to take care of that in step(2).
Expand All @@ -32,6 +36,11 @@ index 58c9a310..21e03f95 100644
+ # Note that there is a risk to get out-of-sync in (3). If the
+ # process dies while removing and before updating the cache
+ # size the cache will be over reported.
+
+ # Try to clean at least twice the requested size to avoid having
+ # to do this all over again
+ try_to_free = required_size * 2
+ freed_so_far = 0
+ for name, last_used in self._last_used_objs():
+ # take write lock for the indivdual object
+ rpath = os.path.join(self._dirname_objects, name)
Expand All @@ -48,18 +57,29 @@ index 58c9a310..21e03f95 100644
+ ):
+ if last_used != self._last_used(name):
+ continue
+ # This is racy right now. To fix it we need to (atomic)
+ # rename the "object.info" file in _rm_r_object() to
+ # something like "object.removing". Then when opening
+ # the cache scan for leftover "object.removing" files
+ # and finish the cleanup and update the cache size
+ # based on the size recorded inside "object.removing".
+ # This is racy right now if the process is killed
+ # during "_rm_r_object(rpath)" because then the
+ # cache size is never reduced by the amount that
+ # was about to be deleted.
+ #
+ # To fix it we need to (atomic) rename the
+ # "object.info" file in _rm_r_object() to
+ # something like "object.removing". Then when
+ # opening the cache scan for leftover
+ # "object.removing" files and finish the cleanup
+ # and update the cache size based on the size
+ # recorded inside "object.removing".
+ size = self._calculate_space(self._path(rpath))
+ self._rm_r_object(rpath)
+ self._update_cache_size(-size)
+ return
+ freed_so_far += size
+ if freed_so_far >= try_to_free:
+ break
+ except BlockingIOError:
+ continue
+
+ # return True if at least the required size got freed
+ return freed_so_far > required_size
+
@property
def info(self) -> FsCacheInfo:
Expand Down
74 changes: 25 additions & 49 deletions src/0003-fscache-use-remove_lru-to-reclaim-space-when-the-cac.patch
Original file line number Diff line number Diff line change
@@ -1,27 +1,37 @@
From d33447368180a414f17a9a08346857129caf2556 Mon Sep 17 00:00:00 2001
From 6ac1d20c8c52f3cbc6d890ed8a008f8f2d4c147b Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
Date: Wed, 13 Dec 2023 17:39:55 +0100
Subject: [PATCH 3/4] fscache: use remove_lru() to reclaim space when the cache
is full

This commit adds code that will remove the least recently used
entries when a store() operation does not succeeds because the
cache is full.
cache is full. To be more efficient it will try to free
twice the requested size (this can be configured in the code).
---
osbuild/util/fscache.py | 48 ++++++++++++++++++++++++++++++-----------
1 file changed, 36 insertions(+), 12 deletions(-)
osbuild/util/fscache.py | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py
index 21e03f95..9057d53c 100644
index 32056a35..59039522 100644
--- a/osbuild/util/fscache.py
+++ b/osbuild/util/fscache.py
@@ -941,20 +941,31 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
info: Dict[str, Any] = {}
@@ -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)
+ # account for the extra block of the object-information file
+ # (find a better way to account for the block size)
+ info["size"] += 4096

- # 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
Expand Down Expand Up @@ -56,48 +66,14 @@ index 21e03f95..9057d53c 100644

try:
# Commit the object-information, thus marking it as fully
@@ -1086,8 +1097,12 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
objs.append(FsCacheObjectInfo(name=name, last_used=last_used))
return sorted(objs, key=lambda obj: obj.last_used)

- def _remove_lru(self):
- """"Remove the least recently used entry from the cache."""
+ def _remove_lru(self, required_size):
+ """"
+ Make room in the cache for "required_size" by remove the least
+ recently used entry from the cache. Note that the cache may
+ clear more than required_size.
+ """
# To avoid having to take a global cache lock the strategy is:
# 1. Get list of (object, last_used) sorted from oldest to newest.
# This is racy so we need to take care of that in step(2).
@@ -1101,6 +1116,11 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
# Note that there is a risk to get out-of-sync in (3). If the
# process dies while removing and before updating the cache
# size the cache will be over reported.
+ #
+ # try to clean at least twice the requested size to avoid having
+ # to do this all over again
+ try_to_free = required_size * 2
+ freed_so_far = 0
for name, last_used in self._last_used_objs():
# take write lock for the indivdual object
rpath = os.path.join(self._dirname_objects, name)
@@ -1126,9 +1146,13 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
size = self._calculate_space(self._path(rpath))
self._rm_r_object(rpath)
self._update_cache_size(-size)
- return
+ freed_so_far += size
+ if freed_so_far >= try_to_free:
+ break
@@ -1146,7 +1157,6 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
break
except BlockingIOError:
continue
+ # return if at least the required size got freed
+ return freed_so_far > required_size
-
# return True if at least the required size got freed
return freed_so_far > required_size

@property
def info(self) -> FsCacheInfo:
--
2.43.0

10 changes: 5 additions & 5 deletions src/0004-fscache-add-eviction-log-statement.patch
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From adb3016947277a272bbc586053569c92448f2fa8 Mon Sep 17 00:00:00 2001
From 3e63810210760998f197437e6c3fb82df87483b8 Mon Sep 17 00:00:00 2001
From: Dusty Mabe <[email protected]>
Date: Mon, 15 Jan 2024 16:14:50 -0500
Subject: [PATCH 4/4] fscache: add eviction log statement
Expand All @@ -9,12 +9,12 @@ This will help us know when the cache LRU policy is working.
1 file changed, 1 insertion(+)

diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py
index 9057d53c..a2e6d374 100644
index 59039522..3aa6a1a4 100644
--- a/osbuild/util/fscache.py
+++ b/osbuild/util/fscache.py
@@ -1144,6 +1144,7 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
# and finish the cleanup and update the cache size
# based on the size recorded inside "object.removing".
@@ -1150,6 +1150,7 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike):
# and update the cache size based on the size
# recorded inside "object.removing".
size = self._calculate_space(self._path(rpath))
+ print(f"CACHE: evicting {rpath}")
self._rm_r_object(rpath)
Expand Down

0 comments on commit 3ea3e92

Please sign in to comment.