[Notes] [Git][BuildStream/buildstream][tpollard/494] WIP: Don't pull artifact buildtrees by default

[Tom Pollard <gitlab mg gitlab com>]

Title: GitLab

Tom Pollard pushed to branch tpollard/494 at BuildStream / buildstream

Commits:

• 5a1e8f33
  by Tom Pollard at 2018-09-21T15:24:01Z

  WIP: Don't pull artifact buildtrees by default
  
  - Set default pull to not include buildtree artifact dir
  - PullQueue configurable buildtree attribute
  - Add 'pullbuildtrees' option to existing artifact artifactcache()
    config
  - pullbuildtrees set on a per server basis from user conf
  - Add --pull-buildtrees flag to bst build cli
  - Add --pull-buildtrees flag to bst pull cli
  - Add helper function _fetch_subdir to cascache, to fetch buildtree
    or any other subdir digest
  - Make element._pull_pending not assume no need to process pull if
    artifact is cached if buildtrees are set to be pulled
  - Ensure cascache.py doesn't try to checkout/extract a dangling ref
  
  ToDo:
  - Tests
  

7 changed files:

• buildstream/_artifactcache/artifactcache.py
• buildstream/_artifactcache/cascache.py
• buildstream/_context.py
• buildstream/_frontend/cli.py
• buildstream/_scheduler/queues/pullqueue.py
• buildstream/_stream.py
• buildstream/element.py

Changes:

buildstream/_artifactcache/artifactcache.py

---

...	...	@@ -38,8 +38,9 @@ CACHE_SIZE_FILE = "cache_size"
38	38	# url (str): Location of the remote artifact cache
39	39	# push (bool): Whether we should attempt to push artifacts to this cache,
40	40	# in addition to pulling from it.
	41	+# buildtrees (bool): Whether the default action of pull should include the artifact buildtree
41	42	#
42		-class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert client_key client_cert')):
	43	+class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert client_key client_cert buildtrees')):
43	44
44	45	# _new_from_config_node
45	46	#
...	...	@@ -47,9 +48,10 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl
47	48	#
48	49	@staticmethod
49	50	def _new_from_config_node(spec_node, basedir=None):
50		- _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert'])
	51	+ _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert', 'pullbuildtrees'])
51	52	url = _yaml.node_get(spec_node, str, 'url')
52	53	push = _yaml.node_get(spec_node, bool, 'push', default_value=False)
	54	+ buildtrees = _yaml.node_get(spec_node, bool, 'pullbuildtrees', default_value=False)
53	55	if not url:
54	56	provenance = _yaml.node_get_provenance(spec_node)
55	57	raise LoadError(LoadErrorReason.INVALID_DATA,
...	...	@@ -67,7 +69,7 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl
67	69	if client_cert and basedir:
68	70	client_cert = os.path.join(basedir, client_cert)
69	71
70		- return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert)
	72	+ return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert, buildtrees)
71	73
72	74
73	75	ArtifactCacheSpec.__new__.__defaults__ = (None, None, None)
...	...	@@ -404,6 +406,22 @@ class ArtifactCache():
404	406	raise ImplError("Cache '{kind}' does not implement contains()"
405	407	.format(kind=type(self).__name__))
406	408
	409	+ # contains_subdir_artifact():
	410	+ #
	411	+ # Check whether an artifact element contains a digest for a subdir
	412	+ # which is populated in the cache, i.e non dangling.
	413	+ #
	414	+ # Args:
	415	+ # element (Element): The Element to check
	416	+ # key (str): The cache key to use
	417	+ # subdir (str): The subdir to check
	418	+ #
	419	+ # Returns: True if the subdir exists & is populated in the cache, False otherwise
	420	+ #
	421	+ def contains_subdir_artifact(self, element, key, subdir):
	422	+ raise ImplError("Cache '{kind}' does not implement contains_subdir_artifact()"
	423	+ .format(kind=type(self).__name__))
	424	+
407	425	# list_artifacts():
408	426	#
409	427	# List artifacts in this cache in LRU order.
...	...	@@ -529,11 +547,12 @@ class ArtifactCache():
529	547	# element (Element): The Element whose artifact is to be fetched
530	548	# key (str): The cache key to use
531	549	# progress (callable): The progress callback, if any
	550	+ # buildtree (bool): If buildtrees are to be pulled from the remote cache
532	551	#
533	552	# Returns:
534	553	# (bool): True if pull was successful, False if artifact was not available
535	554	#
536		- def pull(self, element, key, *, progress=None):
	555	+ def pull(self, element, key, *, progress=None, subdir=None, excluded_subdirs=None):
537	556	raise ImplError("Cache '{kind}' does not implement pull()"
538	557	.format(kind=type(self).__name__))
539	558

buildstream/_artifactcache/cascache.py

---

...	...	@@ -63,7 +63,6 @@ class CASCache(ArtifactCache):
63	63	self.casdir = os.path.join(context.artifactdir, 'cas')
64	64	os.makedirs(os.path.join(self.casdir, 'refs', 'heads'), exist_ok=True)
65	65	os.makedirs(os.path.join(self.casdir, 'objects'), exist_ok=True)
66		-
67	66	self._calculate_cache_quota()
68	67
69	68	self._enable_push = enable_push
...	...	@@ -84,6 +83,15 @@ class CASCache(ArtifactCache):
84	83	# This assumes that the repository doesn't have any dangling pointers
85	84	return os.path.exists(refpath)
86	85
	86	+ def contains_subdir_artifact(self, element, key, subdir):
	87	+ tree = self.resolve_ref(self.get_artifact_fullname(element, key))
	88	+
	89	+ # This assumes that the subdir digest is present in the element tree
	90	+ subdirdigest = self._get_subdir(tree, subdir)
	91	+ objpath = self.objpath(subdirdigest)
	92	+
	93	+ return os.path.exists(objpath)
	94	+
87	95	def extract(self, element, key):
88	96	ref = self.get_artifact_fullname(element, key)
89	97
...	...	@@ -220,7 +228,7 @@ class CASCache(ArtifactCache):
220	228	remotes_for_project = self._remotes[element._get_project()]
221	229	return any(remote.spec.push for remote in remotes_for_project)
222	230
223		- def pull(self, element, key, *, progress=None):
	231	+ def pull(self, element, key, *, progress=None, subdir=None, excluded_subdirs=None):
224	232	ref = self.get_artifact_fullname(element, key)
225	233
226	234	project = element._get_project()
...	...	@@ -239,8 +247,13 @@ class CASCache(ArtifactCache):
239	247	tree.hash = response.digest.hash
240	248	tree.size_bytes = response.digest.size_bytes
241	249
242		- self._fetch_directory(remote, tree)
	250	+ self._fetch_directory(remote, tree, excluded_subdirs=excluded_subdirs)
	251	+
	252	+ # Check if the subdir digest needs to be fetched
	253	+ if subdir:
	254	+ self._fetch_subdir(remote, tree, subdir)
243	255
	256	+ # tree is the remote value, so is the same without or without dangling ref locally
244	257	self.set_ref(ref, tree)
245	258
246	259	# no need to pull from additional remotes
...	...	@@ -635,7 +648,6 @@ class CASCache(ArtifactCache):
635	648	################################################
636	649	# Local Private Methods #
637	650	################################################
638		-
639	651	def _checkout(self, dest, tree):
640	652	os.makedirs(dest, exist_ok=True)
641	653
...	...	@@ -654,8 +666,10 @@ class CASCache(ArtifactCache):
654	666	stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
655	667
656	668	for dirnode in directory.directories:
657		- fullpath = os.path.join(dest, dirnode.name)
658		- self._checkout(fullpath, dirnode.digest)
	669	+ # Don't try to checkout a dangling ref
	670	+ if os.path.exists(self.objpath(dirnode.digest)):
	671	+ fullpath = os.path.join(dest, dirnode.name)
	672	+ self._checkout(fullpath, dirnode.digest)
659	673
660	674	for symlinknode in directory.symlinks:
661	675	# symlink
...	...	@@ -860,7 +874,7 @@ class CASCache(ArtifactCache):
860	874	# remote (Remote): The remote to use.
861	875	# dir_digest (Digest): Digest object for the directory to fetch.
862	876	#
863		- def _fetch_directory(self, remote, dir_digest):
	877	+ def _fetch_directory(self, remote, dir_digest, excluded_subdirs=None):
864	878	objpath = self.objpath(dir_digest)
865	879	if os.path.exists(objpath):
866	880	# already in local cache
...	...	@@ -887,13 +901,24 @@ class CASCache(ArtifactCache):
887	901	assert digest.hash == filenode.digest.hash
888	902
889	903	for dirnode in directory.directories:
890		- self._fetch_directory(remote, dirnode.digest)
891		-
892		- # Place directory blob only in final location when we've
893		- # downloaded all referenced blobs to avoid dangling
894		- # references in the repository.
	904	+ if excluded_subdirs:
	905	+ if dirnode.name not in excluded_subdirs:
	906	+ self._fetch_directory(remote, dirnode.digest)
	907	+ else:
	908	+ self._fetch_directory(remote, dirnode.digest)
	909	+ # place directory blob only in final location when we've downloaded
	910	+ # all referenced blobs to avoid dangling references in the repository
895	911	digest = self.add_object(path=out.name)
896		- assert digest.hash == dir_digest.hash
	912	+
	913	+ if not excluded_subdirs:
	914	+ assert digest.hash == dir_digest.hash
	915	+
	916	+ return digest.hash
	917	+
	918	+ def _fetch_subdir(self, remote, tree, subdir):
	919	+ subdirdigest = self._get_subdir(tree, subdir)
	920	+ self._fetch_directory(remote, subdirdigest)
	921	+
897	922
898	923	def _fetch_tree(self, remote, digest):
899	924	# download but do not store the Tree object

buildstream/_context.py

---

...	...	@@ -109,6 +109,9 @@ class Context():
109	109	# Make sure the XDG vars are set in the environment before loading anything
110	110	self._init_xdg()
111	111
	112	+ # Default to not pulling buildtrees from remote caches
	113	+ self.pullbuildtrees = None
	114	+
112	115	# Private variables
113	116	self._cache_key = None
114	117	self._message_handler = None
...	...	@@ -158,7 +161,7 @@ class Context():
158	161	_yaml.node_validate(defaults, [
159	162	'sourcedir', 'builddir', 'artifactdir', 'logdir',
160	163	'scheduler', 'artifacts', 'logging', 'projects',
161		- 'cache'
	164	+ 'cache', 'pullbuildtrees'
162	165	])
163	166
164	167	for directory in ['sourcedir', 'builddir', 'artifactdir', 'logdir']:

buildstream/_frontend/cli.py

---

...	...	@@ -305,10 +305,12 @@ def init(app, project_name, format_version, element_path, force):
305	305	help="Allow tracking to cross junction boundaries")
306	306	@click.option('--track-save', default=False, is_flag=True,
307	307	help="Deprecated: This is ignored")
	308	+@click.option('--pull-buildtrees', default=False, is_flag=True,
	309	+ help="Pull buildtrees from a remote cache server")
308	310	@click.argument('elements', nargs=-1,
309	311	type=click.Path(readable=False))
310	312	@click.pass_obj
311		-def build(app, elements, all_, track_, track_save, track_all, track_except, track_cross_junctions):
	313	+def build(app, elements, all_, track_, track_save, track_all, track_except, track_cross_junctions, pull_buildtrees):
312	314	"""Build elements in a pipeline"""
313	315
314	316	if (track_except or track_cross_junctions) and not (track_ or track_all):
...	...	@@ -327,7 +329,8 @@ def build(app, elements, all_, track_, track_save, track_all, track_except, trac
327	329	track_targets=track_,
328	330	track_except=track_except,
329	331	track_cross_junctions=track_cross_junctions,
330		- build_all=all_)
	332	+ build_all=all_,
	333	+ pull_buildtrees=pull_buildtrees)
331	334
332	335
333	336	##################################################################
...	...	@@ -429,10 +432,12 @@ def track(app, elements, deps, except_, cross_junctions):
429	432	help='The dependency artifacts to pull (default: none)')
430	433	@click.option('--remote', '-r',
431	434	help="The URL of the remote cache (defaults to the first configured cache)")
	435	+@click.option('--pull-buildtrees', default=False, is_flag=True,
	436	+ help="Pull buildtrees from a remote cache server")
432	437	@click.argument('elements', nargs=-1,
433	438	type=click.Path(readable=False))
434	439	@click.pass_obj
435		-def pull(app, elements, deps, remote):
	440	+def pull(app, elements, deps, remote, pull_buildtrees):
436	441	"""Pull a built artifact from the configured remote artifact cache.
437	442
438	443	By default the artifact will be pulled one of the configured caches
...	...	@@ -446,7 +451,7 @@ def pull(app, elements, deps, remote):
446	451	all: All dependencies
447	452	"""
448	453	with app.initialized(session_name="Pull"):
449		- app.stream.pull(elements, selection=deps, remote=remote)
	454	+ app.stream.pull(elements, selection=deps, remote=remote, pull_buildtrees=pull_buildtrees)
450	455
451	456
452	457	##################################################################

buildstream/_scheduler/queues/pullqueue.py

---

...	...	@@ -31,9 +31,20 @@ class PullQueue(Queue):
31	31	complete_name = "Pulled"
32	32	resources = [ResourceType.DOWNLOAD, ResourceType.CACHE]
33	33
	34	+ def __init__(self, scheduler, buildtrees=False):
	35	+ super().__init__(scheduler)
	36	+
	37	+ # Current default exclusions on pull
	38	+ self._excluded_subdirs = ["buildtree"]
	39	+ self._subdir = None
	40	+ # If buildtrees are to be pulled, remove the value from exclusion list
	41	+ if buildtrees:
	42	+ self._subdir = "buildtree"
	43	+ self._excluded_subdirs.remove(self._subdir)
	44	+
34	45	def process(self, element):
35	46	# returns whether an artifact was downloaded or not
36		- return element._pull()
	47	+ return element._pull(subdir=self._subdir, excluded_subdirs=self._excluded_subdirs)
37	48
38	49	def status(self, element):
39	50	# state of dependencies may have changed, recalculate element state
...	...	@@ -47,7 +58,7 @@ class PullQueue(Queue):
47	58	if not element._can_query_cache():
48	59	return QueueStatus.WAIT
49	60
50		- if element._pull_pending():
	61	+ if element._pull_pending(subdir=self._subdir):
51	62	return QueueStatus.READY
52	63	else:
53	64	return QueueStatus.SKIP

buildstream/_stream.py

---

...	...	@@ -162,12 +162,14 @@ class Stream():
162	162	# track_cross_junctions (bool): Whether tracking should cross junction boundaries
163	163	# build_all (bool): Whether to build all elements, or only those
164	164	# which are required to build the target.
	165	+ # pull_buildtrees (bool): Whether to pull buildtrees from a remote cache server
165	166	#
166	167	def build(self, targets, *,
167	168	track_targets=None,
168	169	track_except=None,
169	170	track_cross_junctions=False,
170		- build_all=False):
	171	+ build_all=False,
	172	+ pull_buildtrees=False):
171	173
172	174	if build_all:
173	175	selection = PipelineSelection.ALL
...	...	@@ -197,7 +199,11 @@ class Stream():
197	199	self._add_queue(track_queue, track=True)
198	200
199	201	if self._artifacts.has_fetch_remotes():
200		- self._add_queue(PullQueue(self._scheduler))
	202	+ # Query if any of the user defined artifact servers have buildtrees set
	203	+ for cache in self._context.artifact_cache_specs:
	204	+ if cache.buildtrees:
	205	+ pull_buildtrees = True
	206	+ self._add_queue(PullQueue(self._scheduler, buildtrees=pull_buildtrees))
201	207
202	208	self._add_queue(FetchQueue(self._scheduler, skip_cached=True))
203	209	self._add_queue(BuildQueue(self._scheduler))
...	...	@@ -297,7 +303,8 @@ class Stream():
297	303	#
298	304	def pull(self, targets, *,
299	305	selection=PipelineSelection.NONE,
300		- remote=None):
	306	+ remote=None,
	307	+ pull_buildtrees=False):
301	308
302	309	use_config = True
303	310	if remote:
...	...	@@ -312,8 +319,13 @@ class Stream():
312	319	if not self._artifacts.has_fetch_remotes():
313	320	raise StreamError("No artifact caches available for pulling artifacts")
314	321
	322	+ # Query if any of the user defined artifact servers have buildtrees set
	323	+ for cache in self._context.artifact_cache_specs:
	324	+ if cache.buildtrees:
	325	+ pull_buildtrees = True
	326	+
315	327	self._pipeline.assert_consistent(elements)
316		- self._add_queue(PullQueue(self._scheduler))
	328	+ self._add_queue(PullQueue(self._scheduler, buildtrees=pull_buildtrees))
317	329	self._enqueue_plan(elements)
318	330	self._run()
319	331

buildstream/element.py

---

...	...	@@ -1131,6 +1131,7 @@ class Element(Plugin):
1131	1131	if not self.__weak_cached:
1132	1132	self.__weak_cached = self.__artifacts.contains(self, self.__weak_cache_key)
1133	1133
	1134	+
1134	1135	if (not self.__assemble_scheduled and not self.__assemble_done and
1135	1136	not self._cached_success() and not self._pull_pending()):
1136	1137	# Workspaced sources are considered unstable if a build is pending
...	...	@@ -1678,18 +1679,26 @@ class Element(Plugin):
1678	1679
1679	1680	# _pull_pending()
1680	1681	#
1681		- # Check whether the artifact will be pulled.
	1682	+ # Check whether the artifact will be pulled. If the pull operation is to
	1683	+ # include a specific subdir of the element artifact (from cli or user conf)
	1684	+ # then the local cache is queried for the subdirs existence.
	1685	+ #
	1686	+ # Args:
	1687	+ # subdir (str): Whether the pull has been invoked with a specific subdir set
1682	1688	#
1683	1689	# Returns:
1684	1690	# (bool): Whether a pull operation is pending
1685	1691	#
1686		- def _pull_pending(self):
	1692	+ def _pull_pending(self, subdir=None):
1687	1693	if self._get_workspace():
1688	1694	# Workspace builds are never pushed to artifact servers
1689	1695	return False
1690	1696
1691		- if self.__strong_cached:
1692		- # Artifact already in local cache
	1697	+ if self.__strong_cached and subdir:
	1698	+ # If we've specified a subdir, check if the subdir is cached locally
	1699	+ if self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, subdir):
	1700	+ return False
	1701	+ elif self.__strong_cached:
1693	1702	return False
1694	1703
1695	1704	# Pull is pending if artifact remote server available
...	...	@@ -1711,11 +1720,10 @@ class Element(Plugin):
1711	1720
1712	1721	self._update_state()
1713	1722
1714		- def _pull_strong(self, *, progress=None):
	1723	+ def _pull_strong(self, *, progress=None, subdir=None, excluded_subdirs=None):
1715	1724	weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1716		-
1717	1725	key = self.__strict_cache_key
1718		- if not self.__artifacts.pull(self, key, progress=progress):
	1726	+ if not self.__artifacts.pull(self, key, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
1719	1727	return False
1720	1728
1721	1729	# update weak ref by pointing it to this newly fetched artifact
...	...	@@ -1723,10 +1731,9 @@ class Element(Plugin):
1723	1731
1724	1732	return True
1725	1733
1726		- def _pull_weak(self, *, progress=None):
	1734	+ def _pull_weak(self, *, progress=None, subdir=None, excluded_subdirs=None):
1727	1735	weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1728		-
1729		- if not self.__artifacts.pull(self, weak_key, progress=progress):
	1736	+ if not self.__artifacts.pull(self, weak_key, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
1730	1737	return False
1731	1738
1732	1739	# extract strong cache key from this newly fetched artifact
...	...	@@ -1744,17 +1751,17 @@ class Element(Plugin):
1744	1751	#
1745	1752	# Returns: True if the artifact has been downloaded, False otherwise
1746	1753	#
1747		- def _pull(self):
	1754	+ def _pull(self, subdir=None, excluded_subdirs=None):
1748	1755	context = self._get_context()
1749	1756
1750	1757	def progress(percent, message):
1751	1758	self.status(message)
1752	1759
1753	1760	# Attempt to pull artifact without knowing whether it's available
1754		- pulled = self._pull_strong(progress=progress)
	1761	+ pulled = self._pull_strong(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1755	1762
1756	1763	if not pulled and not self._cached() and not context.get_strict():
1757		- pulled = self._pull_weak(progress=progress)
	1764	+ pulled = self._pull_weak(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1758	1765
1759	1766	if not pulled:
1760	1767	return False
...	...	@@ -1783,6 +1790,11 @@ class Element(Plugin):
1783	1790	if self.__get_tainted():
1784	1791	return True
1785	1792
	1793	+ # Do not push element's that have a dangling buildtree artifact
	1794	+ subdir = 'buildtree'
	1795	+ if not self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, subdir):
	1796	+ return True
	1797	+
1786	1798	return False
1787	1799
1788	1800	# _push():

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.