[Notes] [Git][BuildStream/buildstream][tristan/cache-quota-max-only] 5 commits: _profile.py: Added a new profiling topic, scheduler

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch tristan/cache-quota-max-only at BuildStream / buildstream

Commits:

• d5847462
  by James Ennis at 2019-01-25T16:35:21Z

  _profile.py: Added a new profiling topic, scheduler
  
  profile_start() and profile_end() calls have been incorporated into
  Scheduler.run()
  

• 56a3954c
  by James Ennis at 2019-01-25T16:35:21Z

  _profile.py: Added a new profiling topic, load-selection
  
  profile_start() and profile_end() calls have been added to
  Stream.load_selection()
  

• 116da6d7
  by James Ennis at 2019-01-25T16:35:21Z

  _profile.py: Update copyright statement
  

• 137d31cd
  by James Ennis at 2019-01-25T17:59:22Z

  Merge branch 'jennis/add_new_profile_topic' into 'master'
  
  Add new 'scheduler' and 'load-selection' profiling topics
  
  See merge request BuildStream/buildstream!1088

• 22b3a0c1
  by Tristan Van Berkom at 2019-01-25T18:35:36Z

  _artifactcache.py: Don't require the quota to be available on disk.
  
  Instead only rely on the headroom to be enough to protect against
  out of space conditions. The headroom can become configurable as
  a separate step is required.
  
  The changes to achieve this are:
  
    * Rename ArtifactCache.has_quota_exceeded() to ArtifactCache.full().
  
    * ArtifactCache.full() now also reports True if the available
      space on the artifact cache volume is smaller than the headroom.
  
      This ensures jobs get triggered to cleanup the cache when
      reaching the end of the disk.
  
    * When loading the artifact quota, it is now only an error if
      the quota exceeds the overall disk space, not if it does not
      fit in the available space.
  
      It is still a warning if the quota does not fit in the
      available space on the artifact cache volume.
  
    * Updated scheduler.py and buildqueue.py for the API rename
  
    * tests: Updated the artifactcache/expiry.py test for its
      expectations in this regard.
  
      Added a new test to test an error when quota was specified to
      exceed total disk space, and adjusted the existing tests to
      expect a warning when the quota does not fit in the available
      space.
  
  This fixes issue #733 and #869.
  

6 changed files:

• buildstream/_artifactcache.py
• buildstream/_profile.py
• buildstream/_scheduler/queues/buildqueue.py
• buildstream/_scheduler/scheduler.py
• buildstream/_stream.py
• tests/artifactcache/expiry.py

Changes:

buildstream/_artifactcache.py

---

...	...	@@ -98,6 +98,7 @@ class ArtifactCache():
98	98	self._cache_size = None # The current cache size, sometimes it's an estimate
99	99	self._cache_quota = None # The cache quota
100	100	self._cache_quota_original = None # The cache quota as specified by the user, in bytes
	101	+ self._cache_quota_headroom = None # The headroom in bytes before reaching the quota or full disk
101	102	self._cache_lower_threshold = None # The target cache size for a cleanup
102	103	self._remotes_setup = False # Check to prevent double-setup of remotes
103	104
...	...	@@ -314,7 +315,7 @@ class ArtifactCache():
314	315	len(self._required_elements),
315	316	(context.config_origin or default_conf)))
316	317
317		- if self.has_quota_exceeded():
	318	+ if self.full():
318	319	raise ArtifactError("Cache too full. Aborting.",
319	320	detail=detail,
320	321	reason="cache-too-full")
...	...	@@ -431,15 +432,25 @@ class ArtifactCache():
431	432	self._cache_size = cache_size
432	433	self._write_cache_size(self._cache_size)
433	434
434		- # has_quota_exceeded()
	435	+ # full()
435	436	#
436		- # Checks if the current artifact cache size exceeds the quota.
	437	+ # Checks if the artifact cache is full, either
	438	+ # because the user configured quota has been exceeded
	439	+ # or because the underlying disk is almost full.
437	440	#
438	441	# Returns:
439		- # (bool): True of the quota is exceeded
	442	+ # (bool): True if the artifact cache is full
440	443	#
441		- def has_quota_exceeded(self):
442		- return self.get_cache_size() > self._cache_quota
	444	+ def full(self):
	445	+
	446	+ if self.get_cache_size() > self._cache_quota:
	447	+ return True
	448	+
	449	+ _, volume_avail = self._get_cache_volume_size()
	450	+ if volume_avail < self._cache_quota_headroom:
	451	+ return True
	452	+
	453	+ return False
443	454
444	455	# preflight():
445	456	#
...	...	@@ -936,9 +947,9 @@ class ArtifactCache():
936	947	# is taken from the user requested cache_quota.
937	948	#
938	949	if 'BST_TEST_SUITE' in os.environ:
939		- headroom = 0
	950	+ self._cache_quota_headroom = 0
940	951	else:
941		- headroom = 2e9
	952	+ self._cache_quota_headroom = 2e9
942	953
943	954	try:
944	955	cache_quota = utils._parse_size(self.context.config_cache_quota,
...	...	@@ -960,27 +971,39 @@ class ArtifactCache():
960	971	#
961	972	if cache_quota is None: # Infinity, set to max system storage
962	973	cache_quota = cache_size + available_space
963		- if cache_quota < headroom: # Check minimum
	974	+ if cache_quota < self._cache_quota_headroom: # Check minimum
964	975	raise LoadError(LoadErrorReason.INVALID_DATA,
965	976	"Invalid cache quota ({}): ".format(utils._pretty_size(cache_quota)) +
966	977	"BuildStream requires a minimum cache quota of 2G.")
967		- elif cache_quota > cache_size + available_space: # Check maximum
968		- if '%' in self.context.config_cache_quota:
969		- available = (available_space / total_size) * 100
970		- available = '{}% of total disk space'.format(round(available, 1))
971		- else:
972		- available = utils._pretty_size(available_space)
973		-
	978	+ elif cache_quota > total_size:
	979	+ # A quota greater than the total disk size is certianly an error
974	980	raise ArtifactError("Your system does not have enough available " +
975	981	"space to support the cache quota specified.",
976	982	detail=("You have specified a quota of {quota} total disk space.\n" +
977	983	"The filesystem containing {local_cache_path} only " +
978		- "has {available_size} available.")
	984	+ "has {total_size} total disk space.")
979	985	.format(
980	986	quota=self.context.config_cache_quota,
981	987	local_cache_path=self.context.artifactdir,
982		- available_size=available),
	988	+ total_size=utils._pretty_size(total_size)),
983	989	reason='insufficient-storage-for-quota')
	990	+ elif cache_quota > cache_size + available_space:
	991	+ # The quota does not fit in the available space, this is a warning
	992	+ if '%' in self.context.config_cache_quota:
	993	+ available = (available_space / total_size) * 100
	994	+ available = '{}% of total disk space'.format(round(available, 1))
	995	+ else:
	996	+ available = utils._pretty_size(available_space)
	997	+
	998	+ self._message(MessageType.WARN,
	999	+ "Your system does not have enough available " +
	1000	+ "space to support the cache quota specified.",
	1001	+ detail=("You have specified a quota of {quota} total disk space.\n" +
	1002	+ "The filesystem containing {local_cache_path} only " +
	1003	+ "has {available_size} available.")
	1004	+ .format(quota=self.context.config_cache_quota,
	1005	+ local_cache_path=self.context.artifactdir,
	1006	+ available_size=available))
984	1007
985	1008	# Place a slight headroom (2e9 (2GB) on the cache_quota) into
986	1009	# cache_quota to try and avoid exceptions.
...	...	@@ -990,7 +1013,7 @@ class ArtifactCache():
990	1013	# already really fuzzy.
991	1014	#
992	1015	self._cache_quota_original = cache_quota
993		- self._cache_quota = cache_quota - headroom
	1016	+ self._cache_quota = cache_quota - self._cache_quota_headroom
994	1017	self._cache_lower_threshold = self._cache_quota / 2
995	1018
996	1019	# _get_cache_volume_size()

buildstream/_profile.py

---

1	1	#
2	2	# Copyright (C) 2017 Codethink Limited
	3	+# Copyright (C) 2019 Bloomberg Finance LP
3	4	#
4	5	# This program is free software; you can redistribute it and/or
5	6	# modify it under the terms of the GNU Lesser General Public
...	...	@@ -16,6 +17,7 @@
16	17	#
17	18	# Authors:
18	19	# Tristan Van Berkom <tristan vanberkom codethink co uk>
	20	+# James Ennis <james ennis codethink co uk>
19	21
20	22	import cProfile
21	23	import pstats
...	...	@@ -46,6 +48,8 @@ class Topics():
46	48	LOAD_CONTEXT = 'load-context'
47	49	LOAD_PROJECT = 'load-project'
48	50	LOAD_PIPELINE = 'load-pipeline'
	51	+ LOAD_SELECTION = 'load-selection'
	52	+ SCHEDULER = 'scheduler'
49	53	SHOW = 'show'
50	54	ARTIFACT_RECEIVE = 'artifact-receive'
51	55	ALL = 'all'

buildstream/_scheduler/queues/buildqueue.py

---

...	...	@@ -100,7 +100,7 @@ class BuildQueue(Queue):
100	100	# If the estimated size outgrows the quota, ask the scheduler
101	101	# to queue a job to actually check the real cache size.
102	102	#
103		- if artifacts.has_quota_exceeded():
	103	+ if artifacts.full():
104	104	self._scheduler.check_cache_size()
105	105
106	106	def done(self, job, element, result, status):

buildstream/_scheduler/scheduler.py

---

...	...	@@ -29,6 +29,7 @@ from contextlib import contextmanager
29	29	# Local imports
30	30	from .resources import Resources, ResourceType
31	31	from .jobs import JobStatus, CacheSizeJob, CleanupJob
	32	+from .._profile import Topics, profile_start, profile_end
32	33
33	34
34	35	# A decent return code for Scheduler.run()
...	...	@@ -154,11 +155,16 @@ class Scheduler():
154	155	# Check if we need to start with some cache maintenance
155	156	self._check_cache_management()
156	157
	158	+ # Start the profiler
	159	+ profile_start(Topics.SCHEDULER, "_".join(queue.action_name for queue in self.queues))
	160	+
157	161	# Run the queues
158	162	self._sched()
159	163	self.loop.run_forever()
160	164	self.loop.close()
161	165
	166	+ profile_end(Topics.SCHEDULER, "_".join(queue.action_name for queue in self.queues))
	167	+
162	168	# Stop handling unix signals
163	169	self._disconnect_signals()
164	170
...	...	@@ -297,7 +303,7 @@ class Scheduler():
297	303	# starts while we are checking the cache.
298	304	#
299	305	artifacts = self.context.artifactcache
300		- if artifacts.has_quota_exceeded():
	306	+ if artifacts.full():
301	307	self._sched_cache_size_job(exclusive=True)
302	308
303	309	# _spawn_job()
...	...	@@ -332,7 +338,7 @@ class Scheduler():
332	338	context = self.context
333	339	artifacts = context.artifactcache
334	340
335		- if artifacts.has_quota_exceeded():
	341	+ if artifacts.full():
336	342	self._cleanup_scheduled = True
337	343
338	344	# Callback for the cleanup job

buildstream/_stream.py

---

...	...	@@ -32,6 +32,7 @@ from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
32	32	from ._message import Message, MessageType
33	33	from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
34	34	from ._pipeline import Pipeline, PipelineSelection
	35	+from ._profile import Topics, profile_start, profile_end
35	36	from . import utils, _yaml, _site
36	37	from . import Scope, Consistency
37	38
...	...	@@ -106,10 +107,16 @@ class Stream():
106	107	def load_selection(self, targets, *,
107	108	selection=PipelineSelection.NONE,
108	109	except_targets=()):
	110	+
	111	+ profile_start(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
	112	+
109	113	elements, _ = self._load(targets, (),
110	114	selection=selection,
111	115	except_targets=except_targets,
112	116	fetch_subprojects=False)
	117	+
	118	+ profile_end(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
	119	+
113	120	return elements
114	121
115	122	# shell()

tests/artifactcache/expiry.py

---

...	...	@@ -317,6 +317,16 @@ def test_never_delete_required_track(cli, datafiles, tmpdir):
317	317	# has 10K total disk space, and 6K of it is already in use (not
318	318	# including any space used by the artifact cache).
319	319	#
	320	+# Parameters:
	321	+# quota (str): A quota size configuration for the config file
	322	+# err_domain (str): An ErrorDomain, or 'success' or 'warning'
	323	+# err_reason (str): A reson to compare with an error domain
	324	+#
	325	+# If err_domain is 'success', then err_reason is unused.
	326	+#
	327	+# If err_domain is 'warning', then err_reason is asserted to
	328	+# be in the stderr.
	329	+#
320	330	@pytest.mark.parametrize("quota,err_domain,err_reason", [
321	331	# Valid configurations
322	332	("1", 'success', None),
...	...	@@ -328,9 +338,13 @@ def test_never_delete_required_track(cli, datafiles, tmpdir):
328	338	("-1", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
329	339	("pony", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
330	340	("200%", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
	341	+
	342	+ # Not enough space on disk even if you cleaned up
	343	+ ("11K", ErrorDomain.ARTIFACT, 'insufficient-storage-for-quota'),
	344	+
331	345	# Not enough space for these caches
332		- ("7K", ErrorDomain.ARTIFACT, 'insufficient-storage-for-quota'),
333		- ("70%", ErrorDomain.ARTIFACT, 'insufficient-storage-for-quota')
	346	+ ("7K", 'warning', 'Your system does not have enough available'),
	347	+ ("70%", 'warning', 'Your system does not have enough available')
334	348	])
335	349	@pytest.mark.datafiles(DATA_DIR)
336	350	def test_invalid_cache_quota(cli, datafiles, tmpdir, quota, err_domain, err_reason):
...	...	@@ -374,6 +388,8 @@ def test_invalid_cache_quota(cli, datafiles, tmpdir, quota, err_domain, err_reas
374	388
375	389	if err_domain == 'success':
376	390	res.assert_success()
	391	+ elif err_domain == 'warning':
	392	+ assert err_reason in res.stderr
377	393	else:
378	394	res.assert_main_error(err_domain, err_reason)
379	395

—
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.