[PATCH 1/3] Use cache for base repo if possible

Vratislav Podzimek vpodzime at redhat.com
Wed Oct 16 13:26:21 UTC 2013


The baseRepo property is tricky, because it needs yum_lock to be evaluated.
However, in many cases, nothing changes in the configuration and we can simply
return the same value as in the previous call. If there is some change that can
potentially change the value, the cache needs to be made invalid and refreshed.

This speeds things up and makes the getBaseRepo method cheap enough to be used
in additional places that need it.

Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
---
 pyanaconda/constants.py              |  1 +
 pyanaconda/packaging/__init__.py     | 21 +++++++--
 pyanaconda/packaging/dnfpayload.py   |  4 +-
 pyanaconda/packaging/yumpayload.py   | 87 +++++++++++++++++++++++++++++++++---
 pyanaconda/ui/gui/spokes/network.py  |  4 +-
 pyanaconda/ui/gui/spokes/software.py |  2 +-
 pyanaconda/ui/gui/spokes/source.py   | 12 ++---
 pyanaconda/ui/gui/spokes/storage.py  |  2 +-
 pyanaconda/ui/tui/spokes/software.py |  2 +-
 pyanaconda/ui/tui/spokes/source.py   |  8 ++--
 10 files changed, 116 insertions(+), 27 deletions(-)

diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
index 7af5014..115ef28 100644
--- a/pyanaconda/constants.py
+++ b/pyanaconda/constants.py
@@ -108,6 +108,7 @@ THREAD_GEOLOCATION_REFRESH = "AnaGeolocationRefreshThread"
 THREAD_DATE_TIME = "AnaDateTimeThread"
 THREAD_TIME_INIT = "AnaTimeInitThread"
 THREAD_XKL_WRAPPER_INIT = "AnaXklWrapperInitThread"
+THREAD_REFRESH_BASE_REPO = "AnaBaseRepoRefreshThread"
 
 # Geolocation constants
 
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py
index 74adf7f..a5bcb01 100644
--- a/pyanaconda/packaging/__init__.py
+++ b/pyanaconda/packaging/__init__.py
@@ -64,6 +64,8 @@ from pyanaconda.product import productName, productVersion
 import urlgrabber
 urlgrabber.grabber.default_grabber.opts.user_agent = "%s (anaconda)/%s" %(productName, productVersion)
 
+REPO_NOT_SET = False
+
 ###
 ### ERROR HANDLING
 ###
@@ -145,9 +147,22 @@ class Payload(object):
         """ A list of addon repo identifiers. """
         return [r.name for r in self.data.repo.dataList()]
 
-    @property
-    def baseRepo(self):
-        """ The identifier of the current base repo. """
+    def getBaseRepo(self, wait=True, callback=None):
+        """
+        Get the identifier of the current base repo.
+
+        :param wait: whether to block until the identifier is derived from the
+                     configuration (may take a long time) or just try to use
+                     the cached value and return REPO_NOT_SET if there is none
+        :type wait: bool
+        :param callback: callback that will be called once the indentifier is
+                         derived from the configuration (gets the repo ID as
+                         the first argument) if it is not returned instantly
+        :type callback: str -> None
+        :returns: id of the current base repo or None if wait=True is used or
+                  REPO_NOT_SET if wait=False is used and the value is not cached
+
+        """
         return None
 
     @property
diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
index d0bda57..3e5e01b 100644
--- a/pyanaconda/packaging/dnfpayload.py
+++ b/pyanaconda/packaging/dnfpayload.py
@@ -217,8 +217,8 @@ class DNFPayload(packaging.PackagePayload):
         # addon repos via kickstart
         return [r.name for r in self.data.repo.dataList()]
 
-    @property
-    def baseRepo(self):
+    def getBaseRepo(self, wait=True, callback=None):
+        # is any locking needed here as in the yumpayload?
         repo_names = [constants.BASE_REPO_NAME] + DEFAULT_REPOS
         for repo in self._base.repos.iter_enabled():
             if repo.id in repo_names:
diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py
index 0eb5cf1..4225649 100644
--- a/pyanaconda/packaging/yumpayload.py
+++ b/pyanaconda/packaging/yumpayload.py
@@ -61,7 +61,7 @@ except ImportError:
     log.error("import of yum failed")
     yum = None
 
-from pyanaconda.constants import BASE_REPO_NAME, DRACUT_ISODIR, INSTALL_TREE, ISO_DIR, MOUNT_DIR, ROOT_PATH
+from pyanaconda.constants import BASE_REPO_NAME, DRACUT_ISODIR, INSTALL_TREE, ISO_DIR, MOUNT_DIR, ROOT_PATH, THREAD_REFRESH_BASE_REPO
 from pyanaconda.flags import flags
 
 from pyanaconda import iutil
@@ -77,7 +77,9 @@ from pyanaconda.errors import ERROR_RAISE, errorHandler
 from pyanaconda.packaging import DependencyError, MetadataError, NoNetworkError, NoSuchGroup, \
                                  NoSuchPackage, PackagePayload, PayloadError, PayloadInstallError, \
                                  PayloadSetupError
+from pyanaconda.packaging import REPO_NOT_SET
 from pyanaconda.progress import progressQ
+from pyanaconda.threads import threadMgr, AnacondaThread
 
 from pyanaconda.localization import langcode_matches_locale
 
@@ -135,6 +137,13 @@ class YumPayload(PackagePayload):
         self._requiredPackages = []
         self._requiredGroups = []
 
+        # base repo caching
+        self._cached_base_repo = None
+        self._br_cache_valid = False
+        self._br_callbacks = []
+        self._br_cache_valid_lock = threading.Lock()
+        self._br_refresh_lock = threading.Lock()
+
         self.reset()
 
     def reset(self, root=None):
@@ -372,18 +381,55 @@ reposdir=%s
 
         return _repos
 
-    @property
-    def baseRepo(self):
+    def getBaseRepo(self, wait=True, callback=None):
+        # prevent potential thread calling callbacks from finishing if we want
+        # to add some more
+        with self._br_refresh_lock:
+            # test and set atomically
+            with self._br_cache_valid_lock:
+                if self._br_cache_valid:
+                    return self._cached_base_repo
+
+            # cache not valid, should we start a thread to refresh it?
+            refresh_thread = threadMgr.get(THREAD_REFRESH_BASE_REPO)
+            if not refresh_thread:
+                threadMgr.add(AnacondaThread(name=THREAD_REFRESH_BASE_REPO, target=self._refreshBaseRepo))
+
+            if callback:
+                self._br_callbacks.append(callback)
+
+        if wait:
+            threadMgr.wait(THREAD_REFRESH_BASE_REPO)
+            return self._cached_base_repo
+        else:
+            # not waiting for the thread and cache was not valid
+            return REPO_NOT_SET
+
+    def _refreshBaseRepo(self):
         repo_names = [BASE_REPO_NAME] + default_repos
-        base_repo_name = None
         with _yum_lock:
             for repo_name in repo_names:
                 if repo_name in self.repos and \
                    self._yum.repos.getRepo(repo_name).enabled:
-                    base_repo_name = repo_name
+                    with self._br_cache_valid_lock:
+                        # cache the value for multiple use
+                        self._cached_base_repo = repo_name
+                        self._br_cache_valid = True
+                        current_cached = self._cached_base_repo
                     break
-
-        return base_repo_name
+            else:
+                # didn't find any base repo set and enabled
+                with self._br_cache_valid_lock:
+                    # set the cache to None, but make it valid
+                    self._cached_base_repo = None
+                    self._br_cache_valid = True
+                    current_cached = self._cached_base_repo
+
+        with self._br_refresh_lock:
+            # go through scheduled callbacks (if any) and remove them
+            for callback in self._br_callbacks:
+                callback(current_cached)
+                self._br_callbacks.remove(callback)
 
     @property
     def mirrorEnabled(self):
@@ -421,6 +467,9 @@ reposdir=%s
         """
         log.info("updating base repo")
 
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         # start with a fresh YumBase instance
         self.reset(root=root)
 
@@ -511,6 +560,9 @@ reposdir=%s
                     self.disableRepo(repo.id)
 
     def gatherRepoMetadata(self):
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         # now go through and get metadata for all enabled repos
         log.info("gathering repo metadata")
         for repo_id in self.repos:
@@ -552,6 +604,10 @@ reposdir=%s
             If checkmount is true, check the dracut mount to see if we have
             usable media mounted.
         """
+
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         log.info("configuring base repo")
         url, mirrorlist, sslverify = self._setupInstallDevice(storage, checkmount)
         method = self.data.method
@@ -677,6 +733,9 @@ reposdir=%s
         """ Retrieve repo metadata if we don't already have it. """
         from yum.Errors import RepoError, RepoMDError
 
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         # And try to grab its metadata.  We do this here so it can be done
         # on a per-repo basis, so we can then get some finer grained error
         # handling and recovery.
@@ -720,6 +779,9 @@ reposdir=%s
         """ Add a yum repo to the YumBase instance. """
         from yum.Errors import RepoError
 
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         needsAdding = True
 
         # First, delete any pre-existing repo with the same name.
@@ -794,6 +856,9 @@ reposdir=%s
         """ Remove a repo as specified by id. """
         log.debug("removing repo %s", repo_id)
 
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         # if this is an NFS repo, we'll want to unmount the NFS mount after
         # removing the repo
         mountpoint = None
@@ -814,6 +879,10 @@ reposdir=%s
 
     def enableRepo(self, repo_id):
         """ Enable a repo as specified by id. """
+
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         log.debug("enabling repo %s", repo_id)
         if repo_id in self.repos:
             with _yum_lock:
@@ -822,6 +891,10 @@ reposdir=%s
 
     def disableRepo(self, repo_id):
         """ Disable a repo as specified by id. """
+
+        with self._br_cache_valid_lock:
+            self._br_cache_valid = False
+
         log.debug("disabling repo %s", repo_id)
         if repo_id in self.repos:
             with _yum_lock:
diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index a2dbdcb..8677f92 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -1347,8 +1347,8 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
 
         self._now_available = self.completed
 
-        log.debug("network standalone spoke (apply) payload: %s completed: %s", self.payload.baseRepo, self._now_available)
-        if not self.payload.baseRepo and not self._initially_available and self._now_available:
+        log.debug("network standalone spoke (apply) payload: %s completed: %s", self.payload.getBaseRepo(), self._now_available)
+        if not self.payload.getBaseRepo() and not self._initially_available and self._now_available:
             from pyanaconda.packaging import payloadInitialize
             from pyanaconda.threads import threadMgr, AnacondaThread
 
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index f4b7995..6820d1a 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -152,7 +152,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
         return (not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
                 not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
                 not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
-                self.payload.baseRepo is not None)
+                self.payload.getBaseRepo())
 
     @property
     def showable(self):
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index d9216ab..897ec98 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -348,7 +348,7 @@ class SourceSpoke(NormalSpoke):
         elif self._mirror_active():
             # this preserves the url for later editing
             self.data.method.method = None
-            if not old_source.method and self.payload.baseRepo and \
+            if not old_source.method and self.payload.getBaseRepo(wait=True) and \
                not self._proxyChange:
                 return False
         elif self._http_active() or self._ftp_active():
@@ -448,7 +448,7 @@ class SourceSpoke(NormalSpoke):
             hubQ.send_message(self.__class__.__name__, _(METADATA_DOWNLOAD_MESSAGE))
             self.payload.gatherRepoMetadata()
             self.payload.release()
-            if not self.payload.baseRepo:
+            if not self.payload.getBaseRepo(wait=True):
                 hubQ.send_message(self.__class__.__name__, _(METADATA_ERROR_MESSAGE))
                 hubQ.send_ready(self.__class__.__name__, False)
                 self._error = True
@@ -480,10 +480,10 @@ class SourceSpoke(NormalSpoke):
 
     @property
     def completed(self):
-        if flags.automatedInstall and (not self.data.method.method or not self.payload.baseRepo):
+        if flags.automatedInstall and (not self.data.method.method or not self.payload.getBaseRepo(wait=False)):
             return False
         else:
-            return not self._error and self.ready and (self.data.method.method or self.payload.baseRepo)
+            return not self._error and self.ready and (self.data.method.method or self.payload.getBaseRepo(wait=False))
 
     @property
     def mandatory(self):
@@ -502,7 +502,7 @@ class SourceSpoke(NormalSpoke):
             return _("Checking software dependencies...")
         elif not self.ready:
             return _(BASEREPO_SETUP_MESSAGE)
-        elif self._error or not self.payload.baseRepo:
+        elif self._error or not self.payload.getBaseRepo(wait=True):
             return _("Error setting up software source")
         elif self.data.method.method == "url":
             return self.data.method.url or self.data.method.mirrorlist
@@ -514,7 +514,7 @@ class SourceSpoke(NormalSpoke):
             if not self._currentIsoFile:
                 return _("Error setting up software source")
             return os.path.basename(self._currentIsoFile)
-        elif self.payload.baseRepo:
+        elif self.payload.getBaseRepo(wait=True):
             return _("Closest mirror")
         else:
             return _("Nothing selected")
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 43b1bf6..5a1b26b 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -187,7 +187,7 @@ class InstallOptions1Dialog(GUIObject):
                 not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
                 not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
                 not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
-                self.payload.baseRepo is not None)
+                self.payload.getBaseRepo() is not None)
 
     def _check_for_storage_thread(self, button):
         if self._software_is_ready():
diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py
index 9b37f55..8b3d19d 100644
--- a/pyanaconda/ui/tui/spokes/software.py
+++ b/pyanaconda/ui/tui/spokes/software.py
@@ -161,7 +161,7 @@ class SoftwareSpoke(NormalTUISpoke):
         return (not threadMgr.get(THREAD_SOFTWARE_WATCHER) and
                 not threadMgr.get(THREAD_PAYLOAD_MD) and
                 not threadMgr.get(THREAD_CHECK_SOFTWARE) and
-                self.payload.baseRepo is not None)
+                self.payload.getBaseRepo() is not None)
 
     def apply(self):
         """ Apply our selections """
diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
index 25a60c4..9aca949 100644
--- a/pyanaconda/ui/tui/spokes/source.py
+++ b/pyanaconda/ui/tui/spokes/source.py
@@ -85,7 +85,7 @@ class SourceSpoke(EditTUISpoke):
             return _("NFS server %s") % self.data.method.server
         elif self.data.method.method == "cdrom":
             return _("CD/DVD drive")
-        elif self.payload.baseRepo:
+        elif self.payload.getBaseRepo(wait=True):
             return _("Closest mirror")
         else:
             return _("Nothing selected")
@@ -110,10 +110,10 @@ class SourceSpoke(EditTUISpoke):
 
     @property
     def completed(self):
-        if flags.automatedInstall and (not self.data.method.method or not self.payload.baseRepo):
+        if flags.automatedInstall and (not self.data.method.method or not self.payload.getBaseRepo(wait=False)):
             return False
         else:
-            return not self.errors and self.ready and (self.data.method.method or self.payload.baseRepo)
+            return not self.errors and self.ready and (self.data.method.method or self.payload.getBaseRepo(wait=False))
 
     def refresh(self, args=None):
         EditTUISpoke.refresh(self, args)
@@ -195,7 +195,7 @@ class SourceSpoke(EditTUISpoke):
         else:
             self.payload.gatherRepoMetadata()
             self.payload.release()
-            if not self.payload.baseRepo:
+            if not self.payload.getBaseRepo(wait=True):
                 self.errors.append(_("Error downloading package metadata"))
             else:
                 try:
-- 
1.7.11.7



More information about the anaconda-patches mailing list