1277993 - Improve mechanism for excluding jobs and setting tiers that is less brittle

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[Cameron Dawson [:camd]]

Currently, excluding jobs consists of creating a set of values to exclude by.  Then we get all the reference_data_signatures that match those params, and persist that list at the time of saving the exclusion profile.

However, when new jobs that SHOULD call within those exclusion parameters are added, they are not excluded.  A sheriff must go in to the exclusion editor and just re-save it to update the list of signatures.

This was inspired by bug 1258720.

We COULD create a process that re-saves all exclusion profiles every time a new job type or platform (or some criteria) is added.  Or have a timer that re-saves them all every X minutes/hours/etc.

Or perhaps there's a better approach we could take that is less brittle.

I don't know what the approach might be at this time, but this bug is here to re-think how this is working.  Do we shore it up?  Or come up with something better?

Comment 1

[Cameron Dawson [:camd]]

Note that this also effects the Tier-2 and Tier-3 exclusion profiles which don't actually exclude anything, but set their tier during ingestion.  Missing signatures in that list mean that jobs are ingested and STAY in the wrong tier.

Comment 2

[William Lachance (:wlach)]

I would personally just hook into the post-save method to update the exclusion profiles when a new reference data signature is added:

https://docs.djangoproject.com/en/1.8/ref/signals/#django.db.models.signals.post_save

This shouldn't happen *that* often, so it doesn't matter if it's a little slow (I doubt it would even be that slow)

Comment 3

[GitHub Autolander Bot]

Attachment: [treeherder] martiansideofthemoon:hello > mozilla:master

Comment 4

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8768930 [details] [review]
[treeherder] martiansideofthemoon:hello > mozilla:master

Does this look any good?

Comment 5

[William Lachance (:wlach)]

Comment on attachment 8768930 [details] [review]
[treeherder] martiansideofthemoon:hello > mozilla:master

Cam, if you have a moment, could you take a look and let :martianwars if he's on the right track?

Comment 6

[Cameron Dawson [:camd]]

I left a comment in the PR, but I'm only really concerned about performance.  The exclusion profiles are huge on production, so saving them will have a cost.  I think the approach is good as long as that cost isn't too much.  

An alternative would be to set a value in memcached that the profiles are "dirty" and have a celery beat process that checks that every 5 or 10 minutes to do the update.

Comment 7

[Cameron Dawson [:camd]]

Here's a unit test that works with exclusion_profiles: https://github.com/mozilla/treeherder/blob/b040bb40455e2baac403813c882d143711262b49/tests/webapp/api/test_jobs_api.py#L160-L160

Comment 8

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8768930 [details] [review]
[treeherder] martiansideofthemoon:hello > mozilla:master

The unit-test fails when I remove the added code, so I guess it works :) I'm not sure how much this will harm the performance though.

Comment 9

[William Lachance (:wlach)]

Comment on attachment 8768930 [details] [review]
[treeherder] martiansideofthemoon:hello > mozilla:master

We need to update the tier info inside jobs model (see PR for details), but this is looking good!

Comment 10

[William Lachance (:wlach)]

Comment on attachment 8768930 [details] [review]
[treeherder] martiansideofthemoon:hello > mozilla:master

Looks good. We should be somewhat cautious about merging this (i.e. we should wait for the issues in bug 1287501 to be resolved), but I am pretty sure it will be fine. There isn't actually that much data in the job exclusion profiles themselves, so I think resaving them should be quite fast.

Comment 11

[Kalpesh Krishna [:martianwars]]

wlach is this good to go?

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/2dac5add3b2aae87a8e865a7c713ead81c70bbcc
Bug 1277993 - Improve mechanism for excluding jobs and setting tiers (#1661)

Comment 13

[William Lachance (:wlach)]

I merged the patch and will deploy to stage shortly. Hopefully it won't cause any explosions. :)

Comment 14

[Kalpesh Krishna [:martianwars]]

Can this be marked as resolved?