Closed
Bug 1304062
Opened 8 years ago
Closed 8 years ago
Differences in the failure_line table schema between environments
Categories
(Tree Management :: Treeherder, defect, P1)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(1 file)
| Comment hidden (obsolete) |
|
Assignee | |
Comment 1•8 years ago
|
||
Differences:
* SCL3 prod is missing an index on `signature`.
* SCL3 stage / Heroku prototype are missing the composite unique key on (`job_log_id`,`line`).
* SCL3 stage / Heroku prototype's `job_log_id` is DEFAULT NULL, whereas the others have no default.
* SCL3 prod / Heroku stage have a duplicate foreign key constraint on `best_classification_id`.
* Heroku stage / Heroku prototype's `best_classification_id` is DEFAULT NULL, whereas the others have no default.
* Heroku prototype is missing:
KEY `<REMOVED>` (`repository_id`),
KEY `<REMOVED>` (`job_log_id`), (though note this will be a redundant index anyway, bug 1302869)
CONSTRAINT `<REMOVED>` FOREIGN KEY (`job_log_id`) REFERENCES `job_log` (`id`),
CONSTRAINT `<REMOVED>` FOREIGN KEY (`repository_id`) REFERENCES `repository` (`id`)
...but has the following that other environments don't:
KEY `<REMOVED>` (`action`),
KEY `<REMOVED>` (`created`,`test`(100),`subtest`(25),`status`,`expected`),
KEY `<REMOVED>` (`created`,`signature`(50),`test`(100)),
* Vagrant's `created` and `modified` use datetime(6), whereas everything else uses datetime.
* Charset/collation (but that's bug 1303767)
|
||
Comment 2•8 years ago
|
||
Wow, what a mess,
> Differences:
> * SCL3 prod is missing an index on `signature`.
That seems reasonable since the index with signature(25) at the start will be used instead (but see below).
> * SCL3 stage / Heroku prototype are missing the composite unique key on (`job_log_id`,`line`).
I think this one should be present.
> * SCL3 stage / Heroku prototype's `job_log_id` is DEFAULT NULL, whereas the others have no default.
I think this must be a leftover from the migration from datasource. I think it should be fine as non-null now.
> * SCL3 prod / Heroku stage have a duplicate foreign key constraint on `best_classification_id`.
That seems obviously wrong
> * Heroku stage / Heroku prototype's `best_classification_id` is DEFAULT NULL, whereas the others have no default.
I think this should be DEFAULT NULL.
> * Heroku prototype is missing:
> KEY `<REMOVED>` (`repository_id`),
I think this should be present
> KEY `<REMOVED>` (`job_log_id`), (though note this will be a redundant index anyway, bug 1302869)
Should be missing.
> CONSTRAINT `<REMOVED>` FOREIGN KEY (`job_log_id`) REFERENCES `job_log` (`id`),
Should be present,
> CONSTRAINT `<REMOVED>` FOREIGN KEY (`repository_id`) REFERENCES `repository` (`id`)
Should be present.
> ...but has the following that other environments don't:
> KEY `<REMOVED>` (`action`),
Should be missing.
> KEY `<REMOVED>` (`created`,`test`(100),`subtest`(25),`status`,`expected`),
> KEY `<REMOVED>` (`created`,`signature`(50),`test`(100)),
These were intended as replacements for the indices without "created" now that the classified failure lookups are bucketed by date. I guess they should replace the other indices on non-prototype systems, but there could be cases where this would prevent us using an index at all.
> * Vagrant's `created` and `modified` use datetime(6), whereas everything else uses datetime.
No idea. I guess !vagrant is right.
* Charset/collation (but that's bug 1303767)
Flags: needinfo?(james)
|
|
Assignee | |
Comment 3•8 years ago
|
||
Updated diff:
--- schema-vagrant.sql
+++ schema-prod.sql
...
CREATE TABLE `failure_line` (
`id` bigint(20) NOT NULL AUTO_INCREMENT,
`job_guid` varchar(50) COLLATE utf8_bin NOT NULL,
`action` varchar(11) COLLATE utf8_bin NOT NULL,
`line` int(10) unsigned NOT NULL,
`test` longtext COLLATE utf8_bin,
`subtest` longtext COLLATE utf8_bin,
`status` varchar(7) COLLATE utf8_bin NOT NULL,
`expected` varchar(7) COLLATE utf8_bin DEFAULT NULL,
`message` longtext COLLATE utf8_bin,
`signature` longtext COLLATE utf8_bin,
`level` varchar(8) COLLATE utf8_bin DEFAULT NULL,
`stack` longtext COLLATE utf8_bin,
`stackwalk_stdout` longtext COLLATE utf8_bin,
`stackwalk_stderr` longtext COLLATE utf8_bin,
`best_is_verified` tinyint(1) NOT NULL,
- `created` datetime(6) NOT NULL,
- `modified` datetime(6) NOT NULL,
+ `created` datetime NOT NULL,
+ `modified` datetime NOT NULL,
`best_classification_id` bigint(20) DEFAULT NULL,
- `job_log_id` int(11),
+ `job_log_id` int(11) DEFAULT NULL,
`repository_id` int(11) NOT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `<NAME>` (`job_log_id`,`line`),
KEY `<NAME>` (`job_guid`,`repository_id`),
- KEY `<NAME>` (`signature`(50)),
KEY `<NAME>` (`test`(50),`subtest`(25),`status`,`expected`,`created`),
KEY `<NAME>` (`signature`(25),`test`(50),`created`),
KEY `<NAME>` (`best_classification_id`),
KEY `<NAME>` (`repository_id`),
CONSTRAINT `<NAME>` FOREIGN KEY (`best_classification_id`) REFERENCES `classified_failure` (`id`),
+ CONSTRAINT `<NAME>` FOREIGN KEY (`best_classification_id`) REFERENCES `classified_failure` (`id`),
CONSTRAINT `<NAME>` FOREIGN KEY (`job_log_id`) REFERENCES `job_log` (`id`),
CONSTRAINT `<NAME>` FOREIGN KEY (`repository_id`) REFERENCES `repository` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
Summary: The failure_line table schema is not consistent between any environment → Differences in the failure_line table schema between environments
|
|
Assignee | |
Comment 4•8 years ago
|
||
On prod, the duplicate constraints are:
CONSTRAINT `D57246a07aeab6616e05069de98472d9` FOREIGN KEY (`best_classification_id`) REFERENCES `classified_failure` (`id`),
CONSTRAINT `failure_best_classification_id_2355e01f_fk_classified_failure_id` FOREIGN KEY (`best_classification_id`) REFERENCES `classified_failure` (`id`),
So we can drop the duplicate using:
ALTER TABLE treeherder.failure_line DROP CONSTRAINT `D57246a07aeab6616e05069de98472d9`;
The `DEFAULT NULL` difference on `job_log_id` (whilst a no-op, it would be good to keep the schema diffs as clean as possible) can be resolved using:
ALTER TABLE treeherder.failure_line ALTER COLUMN `job_log_id` DROP DEFAULT;
Both of those can be done in-place without table copies (https://dev.mysql.com/doc/refman/5.6/en/innodb-create-index-overview.html#innodb-online-ddl-summary-grid).
James, r+ on those?
For the missing prefix index on prod (`signature`(50), discussed partially in comment 2), it overlaps with the `signature`(25) of the composite index, albeit isn't as long - so presumably not as performant?
Would you prefer to:
a) Stop adding the `signature`(50) in the migrations file (so remove it from Vagrant/Travis/...)
b) Add the index on prod?
I don't really mind which, I'd just like the schemas to be consistent across environments.
For the datetime difference, the data type changes require a table-copy (so perhaps we should do at the weekend, given this table is 89GB on prod), and would be performed using:
ALTER TABLE treeherder.failure_line
MODIFY `created` datetime(6) NOT NULL,
MODIFY `modified` datetime(6) NOT NULL;
r+ on the SQL there? Thoughts about time to run/being ok at weekend?
Flags: needinfo?(james)
|
|
Assignee | |
Updated•8 years ago
|
Assignee: nobody → emorley
|
|
||
Comment 5•8 years ago
|
||
r+ on the first two and option b) (stop adding signature(50)) for the second one. For the datetime stuff I think the fractional seconds are not really useful and so we can save the 6 bytes per row by aligning on what prod currently has.
Flags: needinfo?(james)
|
|
Assignee | |
Comment 6•8 years ago
|
||
Yeah agreed they are not useful, ideally Django would allow controlling whether to enable fractional seconds, but currently does not (https://github.com/django/django/blob/924af638e4d4fb8eb46a19ac0cafcb2e83480cf3/django/db/backends/mysql/base.py#L140). Let's just wontfix that part and accept that prod won't match for now.
There was a typo in the first statement, for foreign keys the constraints have to be dropped like so:
ALTER TABLE treeherder.failure_line DROP FOREIGN KEY `D57246a07aeab6616e05069de98472d9`;
I've run that and the DROP DEFAULT statement against dev/stage/prod.
I'll open a PR now to remove the RunSQL migration operation that adds the `signature(50)` to Vagrant/...
|
||
Comment 7•8 years ago
|
||
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8832941 -
Flags: review?(james)
|
|
||
Updated•8 years ago
|
Attachment #8832941 -
Flags: review?(james) → review+
|
||
Comment 8•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/90ad380c1664526b32e8ea801fad8611fd3690c4
Bug 1304062 - Remove prefix index on failure_line.signature
Since it overlaps with the composite index, and the additional length
isn't required (50 characters vs 25).
The RunSQL operation is being removed from the existing migrations file
(rather than adding a new RunSQL operation that reverts it), since the
index is missing on prod, so any attempts to remove it would error. This
means that existing Vagrant environments will have an extra index until
the environment is recreated, however it's unlike to have an impact on
reproducibility of prod issues.
|
|
Assignee | |
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.