From 7b117e407ed353529c4ec5b890cba44c987e23a4 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Tue, 18 Sep 2018 09:36:57 +0100 Subject: _artifactcache/artifactcache.py: Ensure no double-setup of remotes Since ArtifactCache.setup_remotes() can be expensive and should only happen once, this commit will assert() if it is called a second time on an artifact cache instance. Signed-off-by: Daniel Silverstone --- buildstream/_artifactcache/artifactcache.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py index eb7759d83..b5c27a165 100644 --- a/buildstream/_artifactcache/artifactcache.py +++ b/buildstream/_artifactcache/artifactcache.py @@ -91,6 +91,7 @@ class ArtifactCache(): self._cache_size = None # The current cache size, sometimes it's an estimate self._cache_quota = None # The cache quota self._cache_lower_threshold = None # The target cache size for a cleanup + self._remotes_setup = False # Check to prevent double-setup of remotes os.makedirs(self.extractdir, exist_ok=True) os.makedirs(self.tmpdir, exist_ok=True) @@ -143,6 +144,10 @@ class ArtifactCache(): # def setup_remotes(self, *, use_config=False, remote_url=None): + # Ensure we do not double-initialise since this can be expensive + assert(not self._remotes_setup) + self._remotes_setup = True + # Initialize remote artifact caches. We allow the commandline to override # the user config in some cases (for example `bst push --remote=...`). has_remote_caches = False -- cgit v1.2.1 From 345f5f4978f0379a8bc3bfa216fa07e38154b207 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Tue, 18 Sep 2018 09:37:53 +0100 Subject: tests/artifactcache/pull.py: Do not double-initialize remotes The initialization of remotes is done by ArtifactCache.setup_remotes() and as such it was wrong for these tests to be calling CASCache.initialize_remotes() a second time. Signed-off-by: Daniel Silverstone --- tests/artifactcache/pull.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/artifactcache/pull.py b/tests/artifactcache/pull.py index 6336e7ab1..45560aabf 100644 --- a/tests/artifactcache/pull.py +++ b/tests/artifactcache/pull.py @@ -137,7 +137,6 @@ def _test_pull(user_config_file, project_dir, artifact_dir, # Manually setup the CAS remote cas.setup_remotes(use_config=True) - cas.initialize_remotes() if cas.has_push_remotes(element=element): # Push the element's artifact @@ -274,7 +273,6 @@ def _test_push_tree(user_config_file, project_dir, artifact_dir, artifact_digest # Manually setup the CAS remote cas.setup_remotes(use_config=True) - cas.initialize_remotes() if cas.has_push_remotes(): directory = remote_execution_pb2.Directory() @@ -310,7 +308,6 @@ def _test_pull_tree(user_config_file, project_dir, artifact_dir, artifact_digest # Manually setup the CAS remote cas.setup_remotes(use_config=True) - cas.initialize_remotes() if cas.has_push_remotes(): # Pull the artifact using the Tree object -- cgit v1.2.1 From e32221b6965f0fc1c3b3e348af3ebe25d7aac46e Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Fri, 14 Sep 2018 15:52:42 +0100 Subject: sandbox/_sandboxremote.py: Acquire artifact cache via Platform The SandboxRemote used to construct its own CASCache which was considered dangerous. This patch replaces that with acquisition of the cache via the Platform singleton, hopefully eliminating issues from having more than one artifact cache object in a single process. Signed-off-by: Daniel Silverstone --- buildstream/sandbox/_sandboxremote.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/buildstream/sandbox/_sandboxremote.py b/buildstream/sandbox/_sandboxremote.py index 296b20351..82968ecb0 100644 --- a/buildstream/sandbox/_sandboxremote.py +++ b/buildstream/sandbox/_sandboxremote.py @@ -27,7 +27,7 @@ from . import Sandbox from ..storage._filebaseddirectory import FileBasedDirectory from ..storage._casbaseddirectory import CasBasedDirectory from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remote_execution_pb2_grpc -from .._artifactcache.cascache import CASCache +from .._platform import Platform class SandboxError(Exception): @@ -43,7 +43,6 @@ class SandboxRemote(Sandbox): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.cascache = None url = urlparse(kwargs['server_url']) if not url.scheme or not url.hostname or not url.port: @@ -56,12 +55,6 @@ class SandboxRemote(Sandbox): self.server_url = '{}:{}'.format(url.hostname, url.port) - def _get_cascache(self): - if self.cascache is None: - self.cascache = CASCache(self._get_context()) - self.cascache.setup_remotes(use_config=True) - return self.cascache - def run_remote_command(self, command, input_root_digest, working_directory, environment): # Sends an execution request to the remote execution server. # @@ -78,8 +71,8 @@ class SandboxRemote(Sandbox): output_files=[], output_directories=[self._output_directory], platform=None) - - cascache = self._get_cascache() + platform = Platform.get_platform() + cascache = platform.artifactcache # Upload the Command message to the remote CAS server command_digest = cascache.push_message(self._get_project(), remote_command) if not command_digest or not cascache.verify_digest_pushed(self._get_project(), command_digest): @@ -141,7 +134,8 @@ class SandboxRemote(Sandbox): if tree_digest is None or not tree_digest.hash: raise SandboxError("Output directory structure had no digest attached.") - cascache = self._get_cascache() + platform = Platform.get_platform() + cascache = platform.artifactcache # Now do a pull to ensure we have the necessary parts. dir_digest = cascache.pull_tree(self._get_project(), tree_digest) if dir_digest is None or not dir_digest.hash or not dir_digest.size_bytes: @@ -176,7 +170,8 @@ class SandboxRemote(Sandbox): upload_vdir.recalculate_hash() - cascache = self._get_cascache() + platform = Platform.get_platform() + cascache = platform.artifactcache # Now, push that key (without necessarily needing a ref) to the remote. vdir_digest = cascache.push_directory(self._get_project(), upload_vdir) if not vdir_digest or not cascache.verify_digest_pushed(self._get_project(), vdir_digest): -- cgit v1.2.1