614946 - Unimportant l10n changes still get run eventually

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Chris AtLee [:catlee]]

We have a fileIsImportant function for l10n changes (isImportantL10nFile) that restricts triggering of dependent builds to changes that touch certain modules.

The problem is that the buildbot schedule doesn't throw away unimportant changes, it just keeps them for later.  The first important change triggers builds for all accumulated unimportant changes since we have treeStableTimer=None.

This could be considered an upstream bug, but a quick fix would be to write a custom scheduler that completely ignores unimportant changes.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: 8cd63b53b93749fc487c.patch

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

I think we'll need to upgrade the scheduler and release masters and restart them, right?

Comment 3

[Chris AtLee [:catlee]]

Comment on attachment 494308 [details] [diff] [review]
8cd63b53b93749fc487c.patch

It might be worth going through buildbotcustom/scheduler.py to see if this will break anything there, or if we need to make similar changes to custom schedulers.

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Yes, it looks like scheduler.py needs similar changes.  I'll put a patch up when I get a chance.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: 614946-buildbotcustom-r1.patch

ben is staging both of these patches as well as the patch from bug 615559

Comment 6

[[e-mail] (:bhearsum)]

I tested this in staging by applying both of the patches and doing the following;
- resetting the db
- pushing two changes to my user repository, which the poller picked up
- sendchanging to start the staging release

I ended up with a dummy repo_setup and tag run that only included the sendchange'd Change, which is what we want.

When I tried the same experiment without the patches from here I got 3 pending builds when i sendchange'd, which were merged together randomly.

One additional thing I noticed is that I got mail from the ChangeNotifier...which is not quite what we want. It seems to happen regardless of whether or not the patches from here are applied though, so I'll file it separately.

This is ready to land as far as I'm concerned.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 494756 [details] [diff] [review]
614946-buildbotcustom-r1.patch

changeset:   1251:a603fd12bf7c

Landed on the default branch, will be merged into production and on the Buildbot masters tomorrow morning.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 494308 [details] [diff] [review]
8cd63b53b93749fc487c.patch

changeset:   111:e7d90277a2a9

Landed on the default branch, will be merged into production and on the Buildbot masters tomorrow morning.

Comment 9

[[e-mail] (:bhearsum)]

Will be landing this in a rolling downtime tomorrow -- aka no tree closure needed....needs-reconfig+ is the best analogue.

Comment 10

[[e-mail] (:bhearsum)]

Should be all done here now.