Page MenuHomePhabricator
Create Task
Maniphest T122111

ref_target should be NOT NULL
Closed, ResolvedPublic
Actions

Description

Link to ALTER fixing this:

https://gerrit.wikimedia.org/r/#/c/270755/1/db_patches/patch-ref_target_not_null.sql

Needs to be run on flowdb and officewiki. Can be done now.

Old background:

I think https://gerrit.wikimedia.org/r/#/c/196660/1/db_patches/patch-remove_unique_ref_indices.sql accidentally changed ref_target to be null-able. My understanding is that we just wanted to make it a blob so there would be no length limitation, but it defaulted to nullable.

Without fully noticing this, I changed flow.sql to blob not null in https://git.wikimedia.org/blobdiff/mediawiki%2Fextensions%2FFlow.git/f07c6ba71699cfaa36b63d53d7c633d4c773d215/flow.sql , but that doesn't solve anything for either installations that upgraded (production or otherwise).

See also T110446: Type of flow_ext_ref.ref_target incorrect on some tables on Beta Cluster.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
ref_target should be NOT NULLmediawiki/extensions/Flowmaster+3 -1
Customize query in gerrit

Related Objects

Event Timeline

• Mattflaschen-WMF created this task.Dec 22 2015, 2:50 AM
• Mattflaschen-WMF raised the priority of this task from to High.
• Mattflaschen-WMF updated the task description. (Show Details)
• Mattflaschen-WMF added projects: StructuredDiscussions, Collaboration-Team-Archive-2015-2016.
• Mattflaschen-WMF added subscribers: • Mattflaschen-WMF, matthiasmullie.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 22 2015, 2:50 AM
• Mattflaschen-WMF updated the task description. (Show Details)Dec 22 2015, 2:50 AM
• Mattflaschen-WMF set Security to None.

This needs a simple patch from our side.

• Mattflaschen-WMF mentioned this in T117787: Make ref_id of flow_wiki_ref and flow_ext_ref not null and PK (DBA phase 2).Dec 22 2015, 2:56 AM
• Mattflaschen-WMF added a comment.Dec 22 2015, 2:58 AM

I checked, and there are no violations of this in production:

mysql:research@s2-analytics-slave [officewiki]> SELECT * FROM flow_ext_ref WHERE ref_target IS NULL;
Empty set (0.06 sec)
mysql:research@x1-analytics-slave [flowdb]> SELECT * FROM flow_ext_ref WHERE ref_target IS NULL;
Empty set (1.09 sec)
• Mattflaschen-WMF renamed this task from ref_target should be not null to ref_target should be NOT NULL.Dec 22 2015, 2:59 AM
Catrope moved this task from Untriaged to Ready for pickup on the Collaboration-Team-Archive-2015-2016 board.Jan 19 2016, 3:07 AM
matthiasmullie claimed this task.Feb 15 2016, 2:49 PM
matthiasmullie moved this task from Ready for pickup to Needs Review on the Collaboration-Team-Archive-2015-2016 board.
gerritbot subscribed.Feb 15 2016, 3:56 PM

Change 270755 had a related patch set uploaded (by Matthias Mullie):
ref_target should be NOT NULL

https://gerrit.wikimedia.org/r/270755

gerritbot added a project: Patch-For-Review.Feb 15 2016, 3:56 PM
gerritbot added a comment.Feb 16 2016, 5:03 AM

Change 270755 merged by jenkins-bot:
ref_target should be NOT NULL

https://gerrit.wikimedia.org/r/270755

hashar mentioned this in T127040: Beta cluster update database is broken because of Flow db patch db_patches/patch-ref_target_not_null.sql .Feb 16 2016, 9:42 AM
hashar subscribed.

https://gerrit.wikimedia.org/r/270755 causes the beta cluster update.php script to fail with:

DatabaseBase::sourceFile( /mnt/srv/mediawiki-staging/php-master/extensions/Flow/db_patches/patch-ref_target_not_null.sql )
10.68.16.193	1170	BLOB/TEXT column 'ref_target' used in key specification without a key length (10.68.16.193)
                        ALTER TABLE `flow_ext_ref` CHANGE ref_target ref_target BLOB NOT NULL

T127040: Beta cluster update database is broken because of Flow db patch db_patches/patch-ref_target_not_null.sql

ReleaseTaggerBot added a project: MW-1.27-release (WMF-deploy-2016-03-01_(1.27.0-wmf.15)).Feb 17 2016, 10:02 PM
Catrope subscribed.Feb 19 2016, 11:18 PM

How does this now make it onto the WMF cluster? Do we need to request a schema change?

matthiasmullie added a comment.EditedFeb 22 2016, 12:29 PM

I misread @Mattflaschen's earlier comment to mean that "production is fine", when in fact he was only saying that even though schema currently allows null, there are no such occurences in prod (and there still aren't any - just checked)

So yeah, we should get patch-ref_target_not_null.sql applied on production. This has to be done on flowdb on extension1, and officewiki on the standard cluster.

matthiasmullie added projects: DBA, Schema-change-in-production.Feb 22 2016, 12:31 PM
matthiasmullie moved this task from Needs Review to Blocked on the Collaboration-Team-Archive-2015-2016 board.Feb 22 2016, 3:17 PM
• Mattflaschen-WMF updated the task description. (Show Details)Feb 24 2016, 10:00 PM
• Mattflaschen-WMF updated the task description. (Show Details)Feb 24 2016, 10:02 PM
• Mattflaschen-WMF added a subscriber: jcrespo.

@jcrespo Should I create a separate task for this, or does this one work? It's ready to run.

jcrespo claimed this task.Feb 25 2016, 7:38 AM

It is ok like this, I will apply it to:

x1-master.flowdb
s3-master.officewiki
jcrespo moved this task from Triage to In progress on the DBA board.Feb 25 2016, 10:10 AM
jcrespo moved this task from In progress to Done on the DBA board.Feb 25 2016, 5:09 PM
jcrespo moved this task from Backlog to Done on the Schema-change-in-production board.
jcrespo added a comment.Feb 25 2016, 5:13 PM
$ mysql --host=x1-master.eqiad.wmnet flowdb -e "SHOW CREATE TABLE flow_ext_ref\G"
*************************** 1. row ***************************
       Table: flow_ext_ref
Create Table: CREATE TABLE `flow_ext_ref` (
  `ref_src_object_id` binary(11) NOT NULL,
  `ref_src_object_type` varbinary(32) NOT NULL,
  `ref_src_workflow_id` binary(11) NOT NULL,
  `ref_src_namespace` int(11) NOT NULL,
  `ref_src_title` varbinary(255) NOT NULL,
  `ref_target` blob NOT NULL,
  `ref_type` varbinary(16) NOT NULL,
  `ref_src_wiki` varbinary(16) NOT NULL,
  `ref_id` binary(11) NOT NULL,
  PRIMARY KEY (`ref_id`),
  KEY `flow_ext_ref_idx_v2` (`ref_src_wiki`,`ref_src_namespace`,`ref_src_title`,`ref_type`,`ref_target`(255),`ref_src_object_type`,`ref_src_object_id`),
  KEY `flow_ext_ref_revision_v2` (`ref_src_wiki`,`ref_src_namespace`,`ref_src_title`,`ref_src_object_type`,`ref_src_object_id`,`ref_type`,`ref_target`(255))
) ENGINE=InnoDB DEFAULT CHARSET=binary

$ mysql --host=s3-master.eqiad.wmnet officewiki -e "SHOW CREATE TABLE flow_ext_ref\G"
*************************** 1. row ***************************
       Table: flow_ext_ref
Create Table: CREATE TABLE `flow_ext_ref` (
  `ref_src_object_id` binary(11) NOT NULL,
  `ref_src_object_type` varbinary(32) NOT NULL,
  `ref_src_workflow_id` binary(11) NOT NULL,
  `ref_src_namespace` int(11) NOT NULL,
  `ref_src_title` varbinary(255) NOT NULL,
  `ref_target` blob NOT NULL,
  `ref_type` varbinary(16) NOT NULL,
  `ref_src_wiki` varbinary(16) NOT NULL,
  `ref_id` binary(11) NOT NULL,
  PRIMARY KEY (`ref_id`),
  KEY `flow_ext_ref_idx_v2` (`ref_src_wiki`,`ref_src_namespace`,`ref_src_title`,`ref_type`,`ref_target`(255),`ref_src_object_type`,`ref_src_object_id`),
  KEY `flow_ext_ref_revision_v2` (`ref_src_wiki`,`ref_src_namespace`,`ref_src_title`,`ref_src_object_type`,`ref_src_object_id`,`ref_type`,`ref_target`(255))
) ENGINE=InnoDB DEFAULT CHARSET=binary

The reason why this can be done now so quickly is because there is a primary key there (also the change was trivial).

jcrespo removed jcrespo as the assignee of this task.Feb 25 2016, 5:13 PM
Catrope closed this task as Resolved.Feb 25 2016, 11:59 PM

Yay! Thanks @jcrespo

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits