From 17fd0b24086f25d9fcd83e5b75b391953377e618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Thu, 19 Apr 2018 08:23:54 +0200 Subject: element.py: Do not query remote artifact caches ahead of pull/push --- buildstream/_frontend/widget.py | 2 - buildstream/_pipeline.py | 2 +- buildstream/_scheduler/pullqueue.py | 5 +- buildstream/element.py | 119 ++++++++++++++++-------------------- 4 files changed, 54 insertions(+), 74 deletions(-) diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py index 5b405682a..fe7229e8a 100644 --- a/buildstream/_frontend/widget.py +++ b/buildstream/_frontend/widget.py @@ -371,8 +371,6 @@ class LogLine(Widget): else: if element._cached(): line = p.fmt_subst(line, 'state', "cached", fg='magenta') - elif element._remotely_cached(): - line = p.fmt_subst(line, 'state', "downloadable", fg='cyan') elif consistency == Consistency.RESOLVED: line = p.fmt_subst(line, 'state', "fetch needed", fg='red') elif element._buildable(): diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py index 0bf57cd3d..04979bc7c 100644 --- a/buildstream/_pipeline.py +++ b/buildstream/_pipeline.py @@ -475,7 +475,7 @@ class _Planner(): self.plan_element(dep, depth) # Dont try to plan builds of elements that are cached already - if not element._cached() and not element._remotely_cached(): + if not element._cached(): for dep in element.dependencies(Scope.BUILD, recurse=False): self.plan_element(dep, depth + 1) diff --git a/buildstream/_scheduler/pullqueue.py b/buildstream/_scheduler/pullqueue.py index 5630ef7c0..0d6b5dbfb 100644 --- a/buildstream/_scheduler/pullqueue.py +++ b/buildstream/_scheduler/pullqueue.py @@ -52,10 +52,7 @@ class PullQueue(Queue): if not success: return False - if not result: - element._pull_failed() - - element._update_state() + element._pull_done() # Element._pull() returns True if it downloaded an artifact, # here we want to appear skipped if we did not download. diff --git a/buildstream/element.py b/buildstream/element.py index adcccc8cf..29b3bd59a 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -215,13 +215,11 @@ class Element(Plugin): self.__consistency = Consistency.INCONSISTENT # Cached overall consistency state self.__cached = None # Whether we have a cached artifact self.__strong_cached = None # Whether we have a cached artifact - self.__remotely_cached = None # Whether we have a remotely cached artifact - self.__remotely_strong_cached = None # Whether we have a remotely cached artifact self.__assemble_scheduled = False # Element is scheduled to be assembled self.__assemble_done = False # Element is assembled self.__tracking_scheduled = False # Sources are scheduled to be tracked self.__tracking_done = False # Sources have been tracked - self.__pull_failed = False # Whether pull was attempted but failed + self.__pull_done = False # Whether pull was attempted self.__log_path = None # Path to dedicated log file or None self.__splits = None # Resolved regex objects for computing split domains self.__whitelist_regex = None # Resolved regex object to check if file is allowed to overlap @@ -954,15 +952,6 @@ class Element(Plugin): def _cached(self): return self.__cached - # _remotely_cached(): - # - # Returns: - # (bool): Whether this element is already present in - # the remote artifact cache - # - def _remotely_cached(self): - return self.__remotely_cached - # _buildable(): # # Returns: @@ -1049,8 +1038,6 @@ class Element(Plugin): self.__weak_cache_key = None self.__strict_cache_key = None self.__strong_cached = None - self.__remotely_cached = None - self.__remotely_strong_cached = None return if self.__weak_cache_key is None: @@ -1081,9 +1068,8 @@ class Element(Plugin): # are sufficient. However, don't update the `cached` attributes # until the full cache query below. cached = self.__artifacts.contains(self, self.__weak_cache_key) - remotely_cached = self.__artifacts.remote_contains(self, self.__weak_cache_key) if (not self.__assemble_scheduled and not self.__assemble_done and - not cached and not remotely_cached): + not cached and not self._pull_pending()): self._schedule_assemble() return @@ -1101,15 +1087,11 @@ class Element(Plugin): key_for_cache_lookup = self.__strict_cache_key if context.get_strict() else self.__weak_cache_key if not self.__cached: self.__cached = self.__artifacts.contains(self, key_for_cache_lookup) - if not self.__remotely_cached: - self.__remotely_cached = self.__artifacts.remote_contains(self, key_for_cache_lookup) if not self.__strong_cached: self.__strong_cached = self.__artifacts.contains(self, self.__strict_cache_key) - if not self.__remotely_strong_cached: - self.__remotely_strong_cached = self.__artifacts.remote_contains(self, self.__strict_cache_key) if (not self.__assemble_scheduled and not self.__assemble_done and - not self.__cached and not self.__remotely_cached): + not self.__cached and not self._pull_pending()): # Workspaced sources are considered unstable if a build is pending # as the build will modify the contents of the workspace. # Determine as early as possible if a build is pending to discard @@ -1527,31 +1509,55 @@ class Element(Plugin): # (bool): Whether a pull operation is pending # def _pull_pending(self): - if self.__pull_failed: - # Consider this equivalent to artifact being unavailable in - # remote cache + if self.__strong_cached: + # Artifact already in local cache return False - if not self.__strong_cached and self.__remotely_strong_cached: - # Pull pending using strict cache key - return True - elif not self.__cached and self.__remotely_cached: - # Pull pending using weak cache key - return True - else: - # No pull pending - return False + # Pull is pending if artifact remote server available + # and pull has not been attempted yet + return self.__artifacts.has_fetch_remotes(element=self) and not self.__pull_done - # _pull_failed() + # _pull_done() # - # Indicate that pull was attempted but failed. + # Indicate that pull was attempted. # - # This needs to be called in the main process after - # a pull fails so that we properly update the main + # This needs to be called in the main process after a pull + # succeeds or fails so that we properly update the main # process data model # - def _pull_failed(self): - self.__pull_failed = True + # This will result in updating the element state. + # + def _pull_done(self): + self.__pull_done = True + + self._update_state() + + def _pull_strong(self, *, progress=None): + weak_key = self._get_cache_key(strength=_KeyStrength.WEAK) + + key = self.__strict_cache_key + if not self.__artifacts.pull(self, key, progress=progress): + return False + + # update weak ref by pointing it to this newly fetched artifact + self.__artifacts.link_key(self, key, weak_key) + + return True + + def _pull_weak(self, *, progress=None): + weak_key = self._get_cache_key(strength=_KeyStrength.WEAK) + + if not self.__artifacts.pull(self, weak_key, progress=progress): + return False + + # extract strong cache key from this newly fetched artifact + self._pull_done() + + # create tag for strong cache key + key = self._get_cache_key(strength=_KeyStrength.STRONG) + self.__artifacts.link_key(self, weak_key, key) + + return True # _pull(): # @@ -1565,26 +1571,14 @@ class Element(Plugin): def progress(percent, message): self.status(message) - weak_key = self._get_cache_key(strength=_KeyStrength.WEAK) - - if self.__remotely_strong_cached: - key = self.__strict_cache_key - assert self.__artifacts.pull(self, key, progress=progress) - - # update weak ref by pointing it to this newly fetched artifact - self.__artifacts.link_key(self, key, weak_key) - elif not context.get_strict() and self.__remotely_cached: - assert self.__artifacts.pull(self, weak_key, progress=progress) + # Attempt to pull artifact without knowing whether it's available + pulled = self._pull_strong(progress=progress) - # extract strong cache key from this newly fetched artifact - self._update_state() + if not pulled and not self._cached() and not context.get_strict(): + pulled = self._pull_weak(progress=progress) - # create tag for strong cache key - key = self._get_cache_key(strength=_KeyStrength.STRONG) - self.__artifacts.link_key(self, weak_key, key) - else: - raise ElementError("Attempt to pull unavailable artifact for element {}" - .format(self.name)) + if not pulled: + return False # Notify successfull download display_key = self.__get_brief_display_key() @@ -1610,16 +1604,7 @@ class Element(Plugin): if self.__get_tainted(): return True - # Use the strong cache key to check whether a remote already has the artifact. - # In non-strict mode we want to push updated artifacts even if the - # remote already has an artifact with the same weak cache key. - key = self._get_cache_key(strength=_KeyStrength.STRONG) - - # Skip if every push remote contains this element already. - if self.__artifacts.push_needed(self, key): - return False - else: - return True + return False # _push(): # -- cgit v1.2.1