Unbalanced prioritization in the images buildbot? (main branch deprioritized too much)

Thibaut hacks at slashdirt.org
Tue Nov 14 01:24:28 PST 2023



> Le 14 nov. 2023 à 09:59, Petr Štetiar <ynezz at true.cz> a écrit :
> 
> Thibaut <hacks at slashdirt.org> [2023-11-13 22:20:28]:
> 
> Hi,
> 
>> GitPoller accepts a single poll interval. What you’re suggesting would
>> require separating each branch, i.e. returning to the previous situation.
> 
> IIUC then you can have multiple GitPoller instances, so wouldn't something
> along this lines work?
> 
> 	diff --git a/phase1/master.cfg b/phase1/master.cfg
> 	index 6f6c650cf54c..6cbd0c8d9609 100644
> 	--- a/phase1/master.cfg
> 	+++ b/phase1/master.cfg
> 	@@ -341,15 +341,16 @@ populateTargets()
> 	 # about source code changes.
> 
> 	 c["change_source"] = []
> 	-c["change_source"].append(
> 	-    GitPoller(
> 	-        repo_url,
> 	-        workdir=work_dir + "/work.git",
> 	-        branches=branchNames,
> 	-        pollAtLaunch=True,
> 	-        pollinterval=300,
> 	+for branch in branchNames:
> 	+    c["change_source"].append(
> 	+        GitPoller(
> 	+            repo_url,
> 	+            branch=branch,
> 	+            workdir=work_dir + "/work.git",
> 	+            pollAtLaunch=True,
> 	+            pollinterval=pollinterval_for_branch(branch),
> 	+        )
> 	     )
> 	-)
> 

I’m pretty sure this would explode should two or more gitpoller instances happen to update at the same time.

It also defeats the branch priority purpose and solves nothing: say you have a 24h interval on some high priority branch. It only takes two separate commits at 24h interval to that branch to still starve master.

>> Also note that even with a 24h poll interval you could still starve the master builds.
> 
> Sure, but meanwhile it would still allow to build quite a bunch of the master
> targets, so improving current situation.

There’s a much easier way to do this: we just stop prioritizing (release) branches.

Treat all builders [there’s one builder per branch per target] equally and rank them by time last run (regardless of branch) and voila, problem solved, no branch *and* no target can be starved.

Of course we would still keep tag builds at maximum priority in that scenario.

This should be a 2-3-line change, let me know if I should submit this.

Of course for low traffic branches, the first new commit will get priority over everything else (if the branch hasn’t been built in a long time), but that I believe is quite « logical » and acceptable.

>> or we switch to a periodic build schedule
> 
> What about handling of security fixes in this once a week periodic build
> proposal?

I don’t follow, what do security fixes have to do with snapshot builds? Surely this is not suggesting that we address security issues via the snapshot builds, especially when, as I pointed out earlier, there is no guarantee that any given commit will be built unequivocally on *all* targets?

My understanding is that security fixes are handled via service releases; I don’t expect users (that includes myself) to keep constantly looking at the git history to find if/when a CVE has been addressed in the snapshot builds.

> Going forward with this approach, we would still probably need some custom
> scheduler which would be able to skim through the commit content and trigger
> build when certain patterns are found, like a word 'security' or CVE number?

See above, I’m afraid this doesn’t make sense to me.

>> This would have the added benefit of ensuring *consistency*
>> across targets, i.e. ensuring that whichever commit is built is built on
>> *all* targets.
> 
> I'm probably missing something, but what is this consistency good for?

Even testing field.

> I would actually prefer to select say top X targets (ideally have them in
> testbeds for automatic runtime testing) and increase build priorities for
> those in phase1/phase2 builds, making sure, that we provide builds to actual
> users/testers first and build the rest afterwards.
> 
> Those targets would be clearly marked as such in the source tree, for example:
> 
> FEATURES+=ci-tier1

I didn’t know we were going to second-class some targets now?

Cheers
T


More information about the openwrt-devel mailing list