You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by tv...@apache.org on 2022/07/23 07:52:35 UTC

[buildstream] branch tristan/fix-potential-race created (now 424ed182e)

This is an automated email from the ASF dual-hosted git repository.

tvb pushed a change to branch tristan/fix-potential-race
in repository https://gitbox.apache.org/repos/asf/buildstream.git


      at 424ed182e element.py: Reimplementation of retrying failed builds in non-strict mode

This branch includes the following new commits:

     new 424ed182e element.py: Reimplementation of retrying failed builds in non-strict mode

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[buildstream] 01/01: element.py: Reimplementation of retrying failed builds in non-strict mode

Posted by tv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tvb pushed a commit to branch tristan/fix-potential-race
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 424ed182e774951fb13d50322979188d9b610a4c
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sat Jul 23 16:50:40 2022 +0900

    element.py: Reimplementation of retrying failed builds in non-strict mode
    
    It was observed that we sometimes do not have the ability to calculate
    the strong cache key when loading the pulled artifact in non-strict mode.
    
    This patch moves the logic which discards a failed build artifact away
    from Element._load_artifact() and instead we discard the artifact when
    calculating the non-strict cache key.
---
 src/buildstream/element.py | 73 ++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 45 deletions(-)

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 1d6ea80fe..71957781f 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1878,31 +1878,6 @@ class Element(Plugin):
         # Attempt to pull artifact with the weak cache key
         pulled = pull and artifact.pull(pull_buildtrees=pull_buildtrees)
 
-        # When building in non-strict mode, we ignore a failed artifact unless it has the
-        # expected strong key, this ensures that failed builds will be retried whenever
-        # dependencies have changed.
-        #
-        # When not building (e.g. `bst show`, `bst artifact push` etc), we do not drop
-        # the failed artifact, the retry only occurs at build time.
-        #
-        if context.build and artifact.cached():
-            success, _, _ = artifact.load_build_result()
-            if not success:
-
-                # Calculate what the cache key would be for this artifact, if we were going to build it
-                cache_key = self.__calculate_strong_cache_key()
-                assert cache_key is not None
-
-                if artifact.strong_key != cache_key:
-                    artifact = Artifact(
-                        self,
-                        context,
-                        strict_key=self.__strict_cache_key,
-                        weak_key=self.__weak_cache_key,
-                    )
-                    artifact._cached = False
-                    pulled = False
-
         self.__artifact = artifact
         return pulled
 
@@ -3155,19 +3130,6 @@ class Element(Plugin):
 
         self.__build_result = self.__artifact.load_build_result()
 
-    # __calculate_strong_cache_key():
-    #
-    # Convenience function for calculating the strong cache key
-    #
-    # This will return the strong cache key if all of the dependencies have cache
-    # keys available, otherwise it will return None.
-    #
-    def __calculate_strong_cache_key(self):
-        assert self.__weak_cache_key is not None
-
-        dependencies = [[e.project_name, e.name, e._get_cache_key()] for e in self._dependencies(_Scope.BUILD)]
-        return self._calculate_cache_key(dependencies, self.__weak_cache_key)
-
     # __update_cache_keys()
     #
     # Updates weak and strict cache keys
@@ -3270,13 +3232,34 @@ class Element(Plugin):
             if self._pull_pending():
                 # Effective strong cache key is unknown until after the pull
                 pass
-            elif self._cached():
-                # Load the strong cache key from the artifact
-                strong_key, _, _ = self.__artifact.get_metadata_keys()
-                self.__cache_key = strong_key
-            elif self.__assemble_scheduled or self.__assemble_done:
-                # Artifact will or has been built, not downloaded
-                self.__cache_key = self.__calculate_strong_cache_key()
+
+            else:
+                assert self.__weak_cache_key is not None
+
+                # First calculate the strong key based on available dependencies and weak key
+                dependencies = [[e.project_name, e.name, e._get_cache_key()] for e in self._dependencies(_Scope.BUILD)]
+                cache_key = self._calculate_cache_key(dependencies, self.__weak_cache_key)
+
+                if cache_key is not None and self._cached():
+                    context = self._get_context()
+                    success, _, _ = self._get_build_result()
+                    strong_key, _, _ = self.__artifact.get_metadata_keys()
+
+                    # When building in non-strict mode, we ignore a failed artifact unless it has the
+                    # expected strong key, this ensures that failed builds will be retried whenever
+                    # dependencies have changed.
+                    if context.build and (not success) and strong_key != cache_key:
+                        self.__artifact = Artifact(
+                            self,
+                            context,
+                            strict_key=self.__strict_cache_key,
+                            weak_key=self.__weak_cache_key,
+                        )
+                        self.__artifact._cached = False
+                    else:
+                        cache_key = strong_key
+
+                self.__cache_key = cache_key
 
             if self.__cache_key is None:
                 # Strong cache key could not be calculated yet