1174973 - Using the same job symbol in multiple groups can break things

Status: RESOLVED DUPLICATE of bug 1215587

(Tree Management :: Treeherder: Data Ingestion, defect, P3)

Description

[Cameron Dawson [:camd]]

It's possible to get your data a bit tangled if you reuse job symbols and names.  Need to clarify in the docs that if you're going to reuse a symbol in different groups, that you need to have unique job names for each group.

Comment 1

[Cameron Dawson [:camd]]

Here's an excerpt from the email I sent to mauro and ed:

Henrik is submitting tests that will be in separate groups (in this case “Ff” and “Fu” but will have shared job symbols (“fr”, “de”).  However, if he submit a job named “de” with symbol “de” then it always goes into group “Fu” even if he specifies group “Ff” in the submission from the client.

https://www.dropbox.com/s/a1n1h5o2vjh6qrv/Screenshot%202015-06-15%2017.43.27.png?dl=0

https://treeherder.allizom.org/#/jobs?repo=mozilla-central&revision=3c26bef95d54&filter-searchStr=linux%20x64%20opt

The issue is that the job ``symbol`` and ``name`` are both “de” when submitting to group of “Ff”.  But he also uses the same symbol and name combo for group “Fu”.  Each ``job_type`` has a ``job_group_id`` field in the db.  So jobs types are linked to groups.  If the job was submitted where the symbol was the same, but the “name” field of the job_type was unique, then we’d find (or create) the right group.  Then we’d generate a new record in the job_type table for each combination.  Currently, we ONLY have the record that points to the “Fu” group, which is why the jobs both get in that group, even if he specifies a different group when he posts to TH with the client.

In buildbot, I think we always have symbols that are unique.  But some, like Mochitest, have just numbers.  And a TC set of jobs for group “Gij” are also just numbers.  But that’s ok, because the “name” fields are unique.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

That means there is no strong relationship between job symbol/name and a given job group? Adding more to the job name will only duplicate text in the treeherder ui, which might become ugly. Why can't we have a primary key for jobs which is decoupled from the name or symbol? Both are not unique identifiers, and even totally different job groups could collide with each other. IMHO this is a hard limitation right now.

Comment 3

[Ed Morley [:emorley]]

I agree with Henrik, we should fix this in the data model if possible (said without any understanding of the schema/implementation for this).

Comment 4

[Cameron Dawson [:camd]]

tl;dr: I agree too... :)

Though (if I'm understanding you right) there *is* a strong relationship between the job symbol/name and the group.  It's hierarchical.  An entry in the ``job_type`` table that has the symbol/name combo has a foreign key to the ``job_group`` table.  So a given job symbol/name combo can have only one group it could belong to.

Fwiw: the job name is only displayed on hover.  It's the symbol you see for the most part.  Except when a job is selected and you are seeing the job details, then you see the name.  So it's not all that bad having the name be more descriptive.

But I still agree with you on this.  :)  The ``job_type`` table shouldn't have a foreign key to the ``job_group`` table.  They should be able to exist in any combination.  

I *think* this might not be that hard to fix in that I don't know that we leverage that foreign key.  (I'd have to study the code a bit more).  But we may just be able to remove the foreign key and update the code to be a little smarter when we generate the new entries in the ``job_type`` and ``job_group`` tables as well as the ``reference_data_signature`` table.

Comment 5

[Ed Morley [:emorley]]

Lets try to fix this in the data model/API. Even if we decide we can't support it, we should use unique keys to protect against having the same symbol in multiple groups.

Comment 6

[Ed Morley [:emorley]]

Cameron, is this fixed now that we no longer foreign key between the jobs and groups tables?

Comment 7

[Cameron Dawson [:camd]]

Yes, this is fixed by that commit.