Closed
Bug 1318474
Opened 8 years ago
Closed 8 years ago
Store the job metadata in the central database instead of in datasource
Categories
(Tree Management :: Treeherder, defect)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wlach, Assigned: wlach)
References
Details
Attachments
(10 files, 1 obsolete file)
(deleted),
text/x-github-pull-request
|
camd
:
review+
|
Details |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-github-pull-request
|
camd
:
review+
|
Details |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/x-github-pull-request
|
camd
:
review+
|
Details |
(deleted),
text/x-github-pull-request
|
camd
:
review+
|
Details |
(deleted),
text/plain
|
Details |
This is the last bit of information in the per-project databases, once this is done we should be able to fairly rip out the rest of datasource!
Assignee | ||
Comment 1•8 years ago
|
||
Based on my experiences with the result set / push migration, I'm going to go for a phased approach here:
1. Start ingesting job metadata in ORM alongside datasource.
2. Make endpoints *use* the data in the ORM, but still use the id numbering from the per-project datasource tables.
3. Make endpoints use/return the id numbering from the ORM.
4. Stop ingesting data into the per-project datasource tables.
By doing this phased approach, I think we can get some of the benefits of the migration finished sooner (e.g. making the treeherder database useable/accessible from redash/activedata, facilitate new projects like SETA) and reduce the risk of everything going wrong by switching things over all at once.
I may actually postpone (3) and (4) until December or later, but hoping to get (1) and (2) landed by next week.
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8811959 [details]
[treeherder] wlach:1318474 > mozilla:master
Hey Cam, could you have a look at this? I think it'll be easier to review this work in chunks, instead of me generating a huge big bang patch which does everything at once.
This PR adds all the job data we care about to the job table inside the main database, and starts using it for all API endpoints. When/if this PR lands, we can pretty much start ignoring datasource for all practical purposes (we basically only use it to number the jobs that we store in the main database).
Attachment #8811959 -
Flags: review?(cdawson)
Comment 4•8 years ago
|
||
Sorry I've taken so long to get started on this. I've been trying to finish up the Persona migration work for Pulse Guardian before end of month. Starting to review this now.
Comment 5•8 years ago
|
||
Comment on attachment 8811959 [details]
[treeherder] wlach:1318474 > mozilla:master
As I said in the PR, I flagged a couple things for you to double-check. But overall no major concerns. Thanks again for doing this! :)
Attachment #8811959 -
Flags: review?(cdawson) → review+
Comment 6•8 years ago
|
||
Comment 7•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/9d557622f7c8cc049f95673173d9af7998750de3
Bug 1318474 - Add job metadata to ORM schema and start ingesting it (#1998)
Comment 8•8 years ago
|
||
Comment 9•8 years ago
|
||
Commits pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/9f604fa323db1d1d2e859c85b2c01810f7f781b7
Bug 1318474 - Switch API's and some internal consumers to use Django ORM for job data
Still *ingesting* into datasource for now
https://github.com/mozilla/treeherder/commit/1abbe58211c2933aad30757163b48565542bf679
Bug 1318474 - Remove obsolete e10s-special case handling in perf etl
Comment 10•8 years ago
|
||
Comment 11•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/3090bffc6a338a88e60683717355c99410aaef19
Bug 1318474 - Fix getting individual jobs (#2003)
Comment 12•8 years ago
|
||
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8815854 [details]
[treeherder] wlach:1318474-jobnote-deletion-fix > mozilla:master
Could you have a look at this Cam? It's a fairly simple fix, but we need it to be applied before we can promote what's on master to production.
Attachment #8815854 -
Flags: review?(cdawson)
Updated•8 years ago
|
Attachment #8815854 -
Flags: review?(cdawson) → review+
Comment 14•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/3721af655041ea09795081307add6fbaa7a12a19
Bug 1318474 - Fix job note deletion (#2008)
We were always resetting the failure classification to an invalid value
when the last job note was deleted for a job, but we didn't notice it
before because we didn't have foreign key validation.
Assignee | ||
Comment 15•8 years ago
|
||
Still need to add "not null" constraints to the metadata columns, now that we've transferred over the data. After that, I think we can resolve this particular bug.
Comment 16•8 years ago
|
||
Comment 17•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/53e4a9573238869c282e6f0ef8fa4151dc19f79f
Bug 1318474 - Disallow null job metadata (#2034)
Assignee | ||
Comment 18•8 years ago
|
||
The last part of this (disabling null keys) I am applying manually, as Django can't apply this migration without generating errors. Here's the SQL that is being applied.
Comment 19•8 years ago
|
||
(In reply to William Lachance (:wlach) from comment #18)
> The last part of this (disabling null keys) I am applying manually, as
> Django can't apply this migration without generating errors. Here's the SQL
> that is being applied.
Could you elaborate? :-)
Looking at the SQL I'm presuming it's due to the need to skip the expensive foreign key checks? Or if it's something else I'm happy to fix upstream Django bugs.
Assignee | ||
Comment 20•8 years ago
|
||
(In reply to Ed Morley [:emorley] from comment #19)
> (In reply to William Lachance (:wlach) from comment #18)
> > The last part of this (disabling null keys) I am applying manually, as
> > Django can't apply this migration without generating errors. Here's the SQL
> > that is being applied.
>
> Could you elaborate? :-)
>
> Looking at the SQL I'm presuming it's due to the need to skip the expensive
> foreign key checks? Or if it's something else I'm happy to fix upstream
> Django bugs.
It's a limitation of innodb, that it apparently can't handle inserts during an alter table operation (which I think we might be seeing because of Django's get_or_create logic):
" When running an online DDL operation, the thread that runs the ALTER TABLE statement will apply an “online log” of DML operations that were run concurrently on the same table from other connection threads. When the DML operations are applied, it is possible to encounter a duplicate key entry error (ERROR 1062 (23000): Duplicate entry), even if the duplicate entry is only temporary and would be reverted by a later entry in the “online log”. This is similar to the idea of a foreign key constraint check in InnoDB in which constraints must hold during a transaction."
https://dev.mysql.com/doc/refman/5.6/en/innodb-create-index-limitations.html
I've been working on documenting these gotchas, will file a bug about it tomorrow.
PostgreSQL keeps on looking better.
Comment 21•8 years ago
|
||
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8821279 [details]
[treeherder] mozilla:1318474-use-global-id > mozilla:master
Hey Cam, could you take a look at this one? It's the last externally facing change as part of the datasource removal. After this is applied, all API's should return and use ORM id's instead of datasource ones (there's a backward compatibility shim for old logviewer urls that we might have in orangefactor, etc.) and we can remove the rest of datasource at our leisure.
Attachment #8821279 -
Flags: review?(cdawson)
Comment 23•8 years ago
|
||
Comment on attachment 8821279 [details]
[treeherder] mozilla:1318474-use-global-id > mozilla:master
This looks good to me. :)
Attachment #8821279 -
Flags: review?(cdawson) → review+
Comment 24•8 years ago
|
||
Commits pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/82b3ea713fc96592489bc5b1eb03047ecea0a80b
Bug 1318474 - Make jobs model take job ORM objects as input
https://github.com/mozilla/treeherder/commit/32cfd2b6599860c6a9015d3db994b29e60c223cc
Bug 1318474 - Make web api's use ORM ids as input and output
https://github.com/mozilla/treeherder/commit/041f481a2cedd9c07c0cfc4dc9374c2eae0d0c94
Bug 1318474 - Make sure old logviewer links still work
Unfortunately there are many older links to the logviewer with datasource
ids floating around. Support them by looking for jobs with the old datasource
id and the project inside the logviewer. If there's a match, then look up the log
information for that job instead.
Comment 25•8 years ago
|
||
Assignee | ||
Comment 26•8 years ago
|
||
The PR referenced in comment 25 pretty much wraps up this bug, but I'll wait until after the holidays to ask for review.
Assignee | ||
Comment 29•8 years ago
|
||
Comment on attachment 8822275 [details]
[treeherder] wlach:1318474-remove-jobmodel-read-methods > mozilla:master
Hey Cam, we're almost done with these, I promise. :)
This stops us from writing to datasource at all, paving the way for its outright removal.
Attachment #8822275 -
Flags: review?(cdawson)
Updated•8 years ago
|
Attachment #8822275 -
Flags: review?(cdawson) → review+
Assignee | ||
Comment 30•8 years ago
|
||
Here's the raw sql that should be applied to avoid django migrations issues with the last PR (note only the alter table statement for `job` requires pausing job ingestion)
Comment 31•8 years ago
|
||
Commits pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/a4a19007c0dff001c0c77d73f60c2da2abb0a2f8
Bug 1318474 - Fix SETA tests to not rely on datasource
https://github.com/mozilla/treeherder/commit/901f6288a57947459f4d0c61c9dfbb3c85474712
Bug 1318474 - Rewrite unit tests to only look for ORM representation
https://github.com/mozilla/treeherder/commit/34ac762fd60cb10547d7c343425f9a96d4ab633c
Bug 1318474 - Remove a call to datasource jobs from autoclassify code
https://github.com/mozilla/treeherder/commit/f4a319b2cf4f69e7a0eca85b4742e4ee128f43ae
Bug 1318474 - Actually remove datasource calls from jobs model
https://github.com/mozilla/treeherder/commit/f6407b74935bfebba234ea752c891334d756c956
Bug 1318474 - Stop writing to datasource altogether for jobs data
The last thing we were using it for were cycling data and giving an
id numbering for jobs, and we can easily replace those with calls to
the ORM.
Assignee | ||
Comment 32•8 years ago
|
||
Applied all SQL changes except for the jobs table one to stage, will do rest tomorrow morning (requires a tree closure).
Assignee | ||
Comment 33•8 years ago
|
||
(In reply to William Lachance (:wlach) from comment #32)
> Applied all SQL changes except for the jobs table one to stage, will do rest
> tomorrow morning (requires a tree closure).
s/stage/production/ :)
Comment 34•8 years ago
|
||
(In reply to William Lachance (:wlach) from comment #32)
> Applied all SQL changes except for the jobs table one to stage, will do rest
> tomorrow morning (requires a tree closure).
trees are closed now
Assignee | ||
Comment 35•8 years ago
|
||
Schema change deployed on production, as was code. I think we're done here.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 37•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8935894 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•