1304335 - Differences in the performance_alert table schema between environments

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P2)

Description

[Ed Morley [:emorley]]

Differences:
* The `performance_alert` table field `related_summary_id` has a default of NULL on Heroku stage, but no default on all other environments.
* Charset/collation varies (bug 1303767)

# SCL3 prod / SCL3 stage:

DROP TABLE IF EXISTS `performance_alert`;
/*!40101 SET @saved_cs_client     = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `performance_alert` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `is_regression` tinyint(1) NOT NULL,
  `status` int(11) NOT NULL,
  `amount_pct` double NOT NULL,
  `amount_abs` double NOT NULL,
  `prev_value` double NOT NULL,
  `new_value` double NOT NULL,
  `t_value` double DEFAULT NULL,
  `related_summary_id` int(11),
  `series_signature_id` int(11) NOT NULL,
  `summary_id` int(11) NOT NULL,
  `manually_created` tinyint(1) NOT NULL,
  `classifier_id` int(11),
  PRIMARY KEY (`id`),
  UNIQUE KEY `<REMOVED>` (`summary_id`,`series_signature_id`),
  KEY `<REMOVED>` (`related_summary_id`),
  KEY `<REMOVED>` (`series_signature_id`),
  KEY `<REMOVED>` (`summary_id`),
  KEY `<REMOVED>` (`classifier_id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`related_summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`series_signature_id`) REFERENCES `performance_signature` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`classifier_id`) REFERENCES `auth_user` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;


# Vagrant / Heroku prototype:

CREATE TABLE `performance_alert` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `is_regression` tinyint(1) NOT NULL,
  `status` int(11) NOT NULL,
  `amount_pct` double NOT NULL,
  `amount_abs` double NOT NULL,
  `prev_value` double NOT NULL,
  `new_value` double NOT NULL,
  `t_value` double DEFAULT NULL,
  `related_summary_id` int(11),
  `series_signature_id` int(11) NOT NULL,
  `summary_id` int(11) NOT NULL,
  `manually_created` tinyint(1) NOT NULL,
  `classifier_id` int(11),
  PRIMARY KEY (`id`),
  UNIQUE KEY `<REMOVED>` (`summary_id`,`series_signature_id`),
  KEY `<REMOVED>` (`related_summary_id`),
  KEY `<REMOVED>` (`series_signature_id`),
  KEY `<REMOVED>` (`summary_id`),
  KEY `<REMOVED>` (`classifier_id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`related_summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`series_signature_id`) REFERENCES `performance_signature` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`classifier_id`) REFERENCES `auth_user` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1;


# Heroku stage:

CREATE TABLE `performance_alert` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `is_regression` tinyint(1) NOT NULL,
  `status` int(11) NOT NULL,
  `amount_pct` double NOT NULL,
  `amount_abs` double NOT NULL,
  `prev_value` double NOT NULL,
  `new_value` double NOT NULL,
  `t_value` double DEFAULT NULL,
  `related_summary_id` int(11) DEFAULT NULL,
  `series_signature_id` int(11) NOT NULL,
  `summary_id` int(11) NOT NULL,
  `manually_created` tinyint(1) NOT NULL,
  `classifier_id` int(11),
  PRIMARY KEY (`id`),
  UNIQUE KEY `<REMOVED>` (`summary_id`,`series_signature_id`),
  KEY `<REMOVED>` (`related_summary_id`),
  KEY `<REMOVED>` (`series_signature_id`),
  KEY `<REMOVED>` (`summary_id`),
  KEY `<REMOVED>` (`classifier_id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`related_summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`series_signature_id`) REFERENCES `performance_signature` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`summary_id`) REFERENCES `performance_alert_summary` (`id`),
  CONSTRAINT `<REMOVED>` FOREIGN KEY (`classifier_id`) REFERENCES `auth_user` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;


Looks like the field was added in:
https://github.com/mozilla/treeherder/commit/b6908b2140dac534de45da47cb3e5d9db7c87e17

Perhaps the initial version that landed on master didn't have the `null=True`? Or else were manual changes made?

Comment 2

[William Lachance (:wlach)]

I suspect this is just due to an idiosyncracy of how Django ran the initial migration. I don't think the default of null actually does anything here, since afaict it's the default behaviour to initialize this type of field to null (certainly that's been the case with this table). We should be able to sync heroku's db with what we have on scl3 with no ill effect.

Comment 3

[William Lachance (:wlach)]

See also: http://stackoverflow.com/a/12253609

Comment 4

[Ed Morley [:emorley]]

(In reply to William Lachance (:wlach) from comment #2)
> I suspect this is just due to an idiosyncracy of how Django ran the initial
> migration. I don't think the default of null actually does anything here,
> since afaict it's the default behaviour to initialize this type of field to
> null (certainly that's been the case with this table). We should be able to
> sync heroku's db with what we have on scl3 with no ill effect.

Yeah I agree the difference in this bug is not significant in terms of behaviour.

It's more that:
* I would say it's best practice to have zero differences between dev and prod schema, if only so we can have a clean diff and not have to lookup MySQL language specifics to reason about whether a specific difference is important
* perhaps not in this case, but several of these differences have occurred because we've gotten into the habbit of making manual changes with no in-bug papertrail/review, when I think we should stop doing that (I'm planning on emailing something to the newsgroup about ideas for improving the situation :-))

Comment 5

[Ed Morley [:emorley]]

Updated diff:

--- schema-vagrant.sql
+++ schema-prod.sql
...
 CREATE TABLE `performance_alert` (
   `id` int(11) NOT NULL AUTO_INCREMENT,
   `is_regression` tinyint(1) NOT NULL,
   `status` int(11) NOT NULL,
   `amount_pct` double NOT NULL,
   `amount_abs` double NOT NULL,
   `prev_value` double NOT NULL,
   `new_value` double NOT NULL,
   `t_value` double DEFAULT NULL,
   `manually_created` tinyint(1) NOT NULL,
   `classifier_id` int(11) DEFAULT NULL,
-  `related_summary_id` int(11),
+  `related_summary_id` int(11) DEFAULT NULL,
   `series_signature_id` int(11) NOT NULL,
   `summary_id` int(11) NOT NULL,
   PRIMARY KEY (`id`),
   UNIQUE KEY `<NAME>` (`summary_id`,`series_signature_id`),
   KEY `<NAME>` (`classifier_id`),
   KEY `<NAME>` (`related_summary_id`),
   KEY `<NAME>` (`series_signature_id`),
   CONSTRAINT `<NAME>` FOREIGN KEY (`related_summary_id`) REFERENCES `performance_alert_summary` (`id`),
   CONSTRAINT `<NAME>` FOREIGN KEY (`series_signature_id`) REFERENCES `performance_signature` (`id`),
   CONSTRAINT `<NAME>` FOREIGN KEY (`summary_id`) REFERENCES `performance_alert_summary` (`id`),
   CONSTRAINT `<NAME>` FOREIGN KEY (`classifier_id`) REFERENCES `auth_user` (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;


Updated on dev/stage/prod using:

ALTER TABLE treeherder.performance_alert MODIFY `related_summary_id` int(11) DEFAULT NULL;