[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:

• ea32dacb
  by Tom Pollard at 2018-09-27T12:31:29Z

  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, 'url')
55	57	raise LoadError(LoadErrorReason.INVALID_DATA,
...	...	@@ -77,7 +79,7 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl
77	79	raise LoadError(LoadErrorReason.INVALID_DATA,
78	80	"{}: 'client-cert' was specified without 'client-key'".format(provenance))
79	81
80		- return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert)
	82	+ return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert, buildtrees)
81	83
82	84
83	85	ArtifactCacheSpec.__new__.__defaults__ = (None, None, None)
...	...	@@ -419,6 +421,22 @@ class ArtifactCache():
419	421	raise ImplError("Cache '{kind}' does not implement contains()"
420	422	.format(kind=type(self).__name__))
421	423
	424	+ # contains_subdir_artifact():
	425	+ #
	426	+ # Check whether an artifact element contains a digest for a subdir
	427	+ # which is populated in the cache, i.e non dangling.
	428	+ #
	429	+ # Args:
	430	+ # element (Element): The Element to check
	431	+ # key (str): The cache key to use
	432	+ # subdir (str): The subdir to check
	433	+ #
	434	+ # Returns: True if the subdir exists & is populated in the cache, False otherwise
	435	+ #
	436	+ def contains_subdir_artifact(self, element, key, subdir):
	437	+ raise ImplError("Cache '{kind}' does not implement contains_subdir_artifact()"
	438	+ .format(kind=type(self).__name__))
	439	+
422	440	# list_artifacts():
423	441	#
424	442	# List artifacts in this cache in LRU order.
...	...	@@ -544,11 +562,12 @@ class ArtifactCache():
544	562	# element (Element): The Element whose artifact is to be fetched
545	563	# key (str): The cache key to use
546	564	# progress (callable): The progress callback, if any
	565	+ # buildtree (bool): If buildtrees are to be pulled from the remote cache
547	566	#
548	567	# Returns:
549	568	# (bool): True if pull was successful, False if artifact was not available
550	569	#
551		- def pull(self, element, key, *, progress=None):
	570	+ def pull(self, element, key, *, progress=None, subdir=None, excluded_subdirs=None):
552	571	raise ImplError("Cache '{kind}' does not implement pull()"
553	572	.format(kind=type(self).__name__))
554	573

buildstream/_artifactcache/cascache.py

---

...	...	@@ -68,7 +68,6 @@ class CASCache(ArtifactCache):
68	68	self.casdir = os.path.join(context.artifactdir, 'cas')
69	69	os.makedirs(os.path.join(self.casdir, 'refs', 'heads'), exist_ok=True)
70	70	os.makedirs(os.path.join(self.casdir, 'objects'), exist_ok=True)
71		-
72	71	self._calculate_cache_quota()
73	72
74	73	self._enable_push = enable_push
...	...	@@ -89,6 +88,16 @@ class CASCache(ArtifactCache):
89	88	# This assumes that the repository doesn't have any dangling pointers
90	89	return os.path.exists(refpath)
91	90
	91	+ def contains_subdir_artifact(self, element, key, subdir):
	92	+ tree = self.resolve_ref(self.get_artifact_fullname(element, key))
	93	+
	94	+ # This assumes that the subdir digest is present in the element tree
	95	+ subdirdigest = self._get_subdir(tree, subdir)
	96	+ objpath = self.objpath(subdirdigest)
	97	+
	98	+ # True if subdir content is cached or if empty as expected
	99	+ return os.path.exists(objpath)
	100	+
92	101	def extract(self, element, key):
93	102	ref = self.get_artifact_fullname(element, key)
94	103
...	...	@@ -225,7 +234,7 @@ class CASCache(ArtifactCache):
225	234	remotes_for_project = self._remotes[element._get_project()]
226	235	return any(remote.spec.push for remote in remotes_for_project)
227	236
228		- def pull(self, element, key, *, progress=None):
	237	+ def pull(self, element, key, *, progress=None, subdir=None, excluded_subdirs=None):
229	238	ref = self.get_artifact_fullname(element, key)
230	239
231	240	project = element._get_project()
...	...	@@ -244,8 +253,14 @@ class CASCache(ArtifactCache):
244	253	tree.hash = response.digest.hash
245	254	tree.size_bytes = response.digest.size_bytes
246	255
247		- self._fetch_directory(remote, tree)
	256	+ # Check if the element artifact is present, if so just fetch subdir
	257	+ if subdir and os.path.exists(self.objpath(tree)):
	258	+ self._fetch_subdir(remote, tree, subdir)
	259	+ else:
	260	+ # Fetch artifact, excluded_subdirs determined in pullqueue
	261	+ self._fetch_directory(remote, tree, excluded_subdirs=excluded_subdirs)
248	262
	263	+ # tree is the remote value, so is the same without or without dangling ref locally
249	264	self.set_ref(ref, tree)
250	265
251	266	element.info("Pulled artifact {} <- {}".format(display_key, remote.spec.url))
...	...	@@ -646,7 +661,6 @@ class CASCache(ArtifactCache):
646	661	################################################
647	662	# Local Private Methods #
648	663	################################################
649		-
650	664	def _checkout(self, dest, tree):
651	665	os.makedirs(dest, exist_ok=True)
652	666
...	...	@@ -665,8 +679,10 @@ class CASCache(ArtifactCache):
665	679	stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
666	680
667	681	for dirnode in directory.directories:
668		- fullpath = os.path.join(dest, dirnode.name)
669		- self._checkout(fullpath, dirnode.digest)
	682	+ # Don't try to checkout a dangling ref
	683	+ if os.path.exists(self.objpath(dirnode.digest)):
	684	+ fullpath = os.path.join(dest, dirnode.name)
	685	+ self._checkout(fullpath, dirnode.digest)
670	686
671	687	for symlinknode in directory.symlinks:
672	688	# symlink
...	...	@@ -945,7 +961,7 @@ class CASCache(ArtifactCache):
945	961	# remote (Remote): The remote to use.
946	962	# dir_digest (Digest): Digest object for the directory to fetch.
947	963	#
948		- def _fetch_directory(self, remote, dir_digest):
	964	+ def _fetch_directory(self, remote, dir_digest, excluded_subdirs=None):
949	965	fetch_queue = [dir_digest]
950	966	fetch_next_queue = []
951	967	batch = _CASBatchRead(remote)
...	...	@@ -963,8 +979,13 @@ class CASCache(ArtifactCache):
963	979	directory.ParseFromString(f.read())
964	980
965	981	for dirnode in directory.directories:
966		- batch = self._fetch_directory_node(remote, dirnode.digest, batch,
967		- fetch_queue, fetch_next_queue, recursive=True)
	982	+ if excluded_subdirs:
	983	+ if dirnode.name not in excluded_subdirs:
	984	+ batch = self._fetch_directory_node(remote, dirnode.digest, batch,
	985	+ fetch_queue, fetch_next_queue, recursive=True)
	986	+ else:
	987	+ batch = self._fetch_directory_node(remote, dirnode.digest, batch,
	988	+ fetch_queue, fetch_next_queue, recursive=True)
968	989
969	990	for filenode in directory.files:
970	991	batch = self._fetch_directory_node(remote, filenode.digest, batch,
...	...	@@ -973,6 +994,12 @@ class CASCache(ArtifactCache):
973	994	# Fetch final batch
974	995	self._fetch_directory_batch(remote, batch, fetch_queue, fetch_next_queue)
975	996
	997	+
	998	+ def _fetch_subdir(self, remote, tree, subdir):
	999	+ subdirdigest = self._get_subdir(tree, subdir)
	1000	+ self._fetch_directory(remote, subdirdigest)
	1001	+
	1002	+
976	1003	def _fetch_tree(self, remote, digest):
977	1004	# download but do not store the Tree object
978	1005	with tempfile.NamedTemporaryFile(dir=self.tmpdir) as out:

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

---

...	...	@@ -32,9 +32,20 @@ class PullQueue(Queue):
32	32	complete_name = "Pulled"
33	33	resources = [ResourceType.DOWNLOAD, ResourceType.CACHE]
34	34
	35	+ def __init__(self, scheduler, buildtrees=False):
	36	+ super().__init__(scheduler)
	37	+
	38	+ # Current default exclusions on pull
	39	+ self._excluded_subdirs = ["buildtree"]
	40	+ self._subdir = None
	41	+ # If buildtrees are to be pulled, remove the value from exclusion list
	42	+ if buildtrees:
	43	+ self._subdir = "buildtree"
	44	+ self._excluded_subdirs.remove(self._subdir)
	45	+
35	46	def process(self, element):
36	47	# returns whether an artifact was downloaded or not
37		- if not element._pull():
	48	+ if not element._pull(subdir=self._subdir, excluded_subdirs=self._excluded_subdirs):
38	49	raise SkipJob(self.action_name)
39	50
40	51	def status(self, element):
...	...	@@ -49,7 +60,7 @@ class PullQueue(Queue):
49	60	if not element._can_query_cache():
50	61	return QueueStatus.WAIT
51	62
52		- if element._pull_pending():
	63	+ if element._pull_pending(subdir=self._subdir):
53	64	return QueueStatus.READY
54	65	else:
55	66	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
...	...	@@ -1676,18 +1677,26 @@ class Element(Plugin):
1676	1677
1677	1678	# _pull_pending()
1678	1679	#
1679		- # Check whether the artifact will be pulled.
	1680	+ # Check whether the artifact will be pulled. If the pull operation is to
	1681	+ # include a specific subdir of the element artifact (from cli or user conf)
	1682	+ # then the local cache is queried for the subdirs existence.
	1683	+ #
	1684	+ # Args:
	1685	+ # subdir (str): Whether the pull has been invoked with a specific subdir set
1680	1686	#
1681	1687	# Returns:
1682	1688	# (bool): Whether a pull operation is pending
1683	1689	#
1684		- def _pull_pending(self):
	1690	+ def _pull_pending(self, subdir=None):
1685	1691	if self._get_workspace():
1686	1692	# Workspace builds are never pushed to artifact servers
1687	1693	return False
1688	1694
1689		- if self.__strong_cached:
1690		- # Artifact already in local cache
	1695	+ if self.__strong_cached and subdir:
	1696	+ # If we've specified a subdir, check if the subdir is cached locally
	1697	+ if self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, subdir):
	1698	+ return False
	1699	+ elif self.__strong_cached:
1691	1700	return False
1692	1701
1693	1702	# Pull is pending if artifact remote server available
...	...	@@ -1709,11 +1718,10 @@ class Element(Plugin):
1709	1718
1710	1719	self._update_state()
1711	1720
1712		- def _pull_strong(self, *, progress=None):
	1721	+ def _pull_strong(self, *, progress=None, subdir=None, excluded_subdirs=None):
1713	1722	weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1714		-
1715	1723	key = self.__strict_cache_key
1716		- if not self.__artifacts.pull(self, key, progress=progress):
	1724	+ if not self.__artifacts.pull(self, key, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
1717	1725	return False
1718	1726
1719	1727	# update weak ref by pointing it to this newly fetched artifact
...	...	@@ -1721,10 +1729,9 @@ class Element(Plugin):
1721	1729
1722	1730	return True
1723	1731
1724		- def _pull_weak(self, *, progress=None):
	1732	+ def _pull_weak(self, *, progress=None, subdir=None, excluded_subdirs=None):
1725	1733	weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1726		-
1727		- if not self.__artifacts.pull(self, weak_key, progress=progress):
	1734	+ if not self.__artifacts.pull(self, weak_key, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
1728	1735	return False
1729	1736
1730	1737	# extract strong cache key from this newly fetched artifact
...	...	@@ -1742,17 +1749,17 @@ class Element(Plugin):
1742	1749	#
1743	1750	# Returns: True if the artifact has been downloaded, False otherwise
1744	1751	#
1745		- def _pull(self):
	1752	+ def _pull(self, subdir=None, excluded_subdirs=None):
1746	1753	context = self._get_context()
1747	1754
1748	1755	def progress(percent, message):
1749	1756	self.status(message)
1750	1757
1751	1758	# Attempt to pull artifact without knowing whether it's available
1752		- pulled = self._pull_strong(progress=progress)
	1759	+ pulled = self._pull_strong(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1753	1760
1754	1761	if not pulled and not self._cached() and not context.get_strict():
1755		- pulled = self._pull_weak(progress=progress)
	1762	+ pulled = self._pull_weak(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1756	1763
1757	1764	if not pulled:
1758	1765	return False
...	...	@@ -1775,10 +1782,14 @@ class Element(Plugin):
1775	1782	if not self._cached():
1776	1783	return True
1777	1784
1778		- # Do not push tained artifact
	1785	+ # Do not push tainted artifact
1779	1786	if self.__get_tainted():
1780	1787	return True
1781	1788
	1789	+ # Do not push elements that have a dangling buildtree artifact unless element type is
	1790	+ # expected to have an empty buildtree directory
	1791	+ if not self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, 'buildtree'):
	1792	+ return True
1782	1793	return False
1783	1794
1784	1795	# _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.