Page MenuHomePhabricator
Create Task
Maniphest T264683

doFixIpbAddressUniqueIndex results in an attempt to update a shared table when it shouldn't
Closed, ResolvedPublic
Actions

Description

From a support desk posting (Error 1091: Can't DROP 'ipb_address_unique'), it looks like a migration from 1.31 to 1.35 might run into problems with T250071: Rename ipb_address index on ipb_address to ipb_address_unique:

# php maintenance/update.php --quick --skip-external-dependencies
...
Updating length of lc_lang in `l10n_cache` ...done.
Updating length of ll_lang in `langlinks` ...done.
Updating length of site_language in `sites` ...done.
...skipping update to shared table ipblocks.
Removing ipb_anon_only column from ipb_address_unique index ...

Wikimedia\Rdbms\DBQueryError from line 1699 of /var/www/html/includes/libs/rdbms/database/Database.php:                                                                      Error 1091: Can't DROP 'ipb_address_unique'; check that column/key exists (mysql)

Function: Wikimedia\Rdbms\Database::sourceFile( /var/www/html/maintenance/archives/patch-ipblocks-fix-ipb_address_unique.sql )

Query: ALTER TABLE `zeus_de`.`ipblocks` DROP INDEX ipb_address_unique, ADD UNIQUE INDEX ipb_address_unique (ipb_address(255), ipb_user, ipb_auto )
...

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+4 -8Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields
mediawiki/coreREL1_35+4 -8Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields
mediawiki/coremaster+6 -3Split patch-ipblocks-fix-ipb_address_unique.sql into two files
mediawiki/coremaster+3 -3Remove ipb_anon_only from ipb_address_unique index addition
mediawiki/coreREL1_35+3 -3Remove ipb_anon_only from ipb_address_unique index addition
Customize query in gerrit

Related Objects

Event Timeline

MarkAHershberger created this task.Oct 5 2020, 10:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2020, 10:47 PM
Reedy added a project: MW-1.35-release.Oct 6 2020, 12:14 AM
Reedy added a subscriber: Reedy.Oct 6 2020, 12:23 AM
	protected function doFixIpbAddressUniqueIndex() {
		if ( !$this->indexHasField( 'ipblocks', 'ipb_address_unique', 'ipb_anon_only' ) ) {
			$this->output( "...ipb_address_unique index up-to-date.\n" );
			return;
		}

		$this->applyPatch(
			'patch-ipblocks-fix-ipb_address_unique.sql',
			false,
			'Removing ipb_anon_only column from ipb_address_unique index'
		);
	}

T251188: ipb_address_unique has an extra column in production but not in the code, rMW39448a9cf654: Remove ipb_anon_only from ipb_address_unique UNIQUE INDEX

Original index added in rMWd779d2889af3: Rename ipb_address to ipb_address_unique...

But how is that not being run in 1.35 before the other one

			// 1.35
			[ 'addTable', 'watchlist_expiry', 'patch-watchlist_expiry.sql' ],
			[ 'modifyField', 'page', 'page_restrictions', 'patch-page_restrictions-null.sql' ],
			[ 'renameIndex', 'ipblocks', 'ipb_address', 'ipb_address_unique', false,
				'patch-ipblocks-rename-ipb_address.sql' ],
			[ 'addField', 'revision', 'rev_actor', 'patch-revision-actor-comment-MCR.sql' ],
			[ 'dropField', 'archive', 'ar_text_id', 'patch-archive-MCR.sql' ],
			[ 'doLanguageLinksLengthSync' ],
			[ 'doFixIpbAddressUniqueIndex' ],
			[ 'modifyField', 'actor', 'actor_name', 'patch-actor-actor_name-varbinary.sql' ],
			[ 'modifyField', 'sites', 'site_global_key', 'patch-sites-site_global_key.sql' ],

Specifically [ 'renameIndex', 'ipblocks', 'ipb_address', 'ipb_address_unique', false, 'patch-ipblocks-rename-ipb_address.sql' ], should be run before before [ 'doFixIpbAddressUniqueIndex' ],

In rMW39448a9cf654: Remove ipb_anon_only from ipb_address_unique UNIQUE INDEX should've modified patch-ipblocks-rename-ipb_address.sql to not add that column... No point repeatedly adding and removing indexes

I'll upload a patch to fix that... But doesn't explain how one didn't happen before the other

gerritbot added a comment.Oct 6 2020, 12:24 AM

Change 632352 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Remove ipb_anon_only from ipb_address_unique index

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

gerritbot added a project: Patch-For-Review.Oct 6 2020, 12:24 AM
Reedy added a comment.Oct 6 2020, 12:30 AM

Looking at

	protected function indexHasField( $table, $index, $field ) {
		if ( !$this->doTable( $table ) ) {
			return true;
		}

		$info = $this->db->indexInfo( $table, $index, __METHOD__ );
		if ( $info ) {
			foreach ( $info as $row ) {
				if ( $row->Column_name == $field ) {
					$this->output( "...index $index on table $table includes field $field.\n" );
					return true;
				}
			}
		}
		$this->output( "...index $index on table $table has no field $field; added.\n" );

		return false;
	}

and

	/**
	 * Get information about an index into an object
	 * Returns false if the index does not exist
	 *
	 * @param string $table
	 * @param string $index
	 * @param string $fname
	 * @return bool|array|null False or null on failure
	 */
	public function indexInfo( $table, $index, $fname = __METHOD__ ) {

$this->db->indexInfo() should be returning false, because ipb_address_unique doesn't exist (?)... Based on the SQL error?

Presumably it didn't go missing between the two different actions?

gerritbot added a comment.Oct 6 2020, 5:15 PM

Change 632352 merged by jenkins-bot:
[mediawiki/core@master] Remove ipb_anon_only from ipb_address_unique index addition

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

gerritbot added a comment.Oct 6 2020, 5:16 PM

Change 632341 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] Remove ipb_anon_only from ipb_address_unique index addition

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

gerritbot added a comment.Oct 6 2020, 5:30 PM

Change 632341 merged by jenkins-bot:
[mediawiki/core@REL1_35] Remove ipb_anon_only from ipb_address_unique index addition

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

Reedy removed a project: Patch-For-Review.Oct 6 2020, 5:37 PM

I'm still not sure about this report. Their posted schema shows the index exists... So possibly just no permission to drop?

ReleaseTaggerBot added projects: MW-1.35-notes, MW-1.36-notes (1.36.0-wmf.13; 2020-10-12).Oct 6 2020, 6:00 PM
Reedy changed the task status from Open to Stalled.Oct 6 2020, 10:48 PM

Marking it as stalled as per my above. Further conversation on the support desk topic hasn't yielded anything, other than their schema seemingly regressing and changing when they haven't apparently changed anything

gerritbot added a comment.Oct 7 2020, 3:02 PM

Change 632734 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Split patch-ipblocks-fix-ipb_address_unique.sql statements

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

gerritbot added a project: Patch-For-Review.Oct 7 2020, 3:02 PM
gerritbot added a comment.Oct 7 2020, 3:18 PM

Change 632734 abandoned by Reedy:
[mediawiki/core@master] Split patch-ipblocks-fix-ipb_address_unique.sql into two files

Reason:

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

Reedy renamed this task from schema migration should handle index renaming on ipb_address to doFixIpbAddressUniqueIndex results in a shared table being updated when it shouldn't be.Oct 7 2020, 3:31 PM
Reedy renamed this task from doFixIpbAddressUniqueIndex results in a shared table being updated when it shouldn't be to doFixIpbAddressUniqueIndex results in an attempt to update a shared table when it shouldn't be.Oct 7 2020, 3:54 PM
Reedy changed the task status from Stalled to Open.
gerritbot added a comment.Oct 7 2020, 3:54 PM

Change 632749 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields

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

Reedy renamed this task from doFixIpbAddressUniqueIndex results in an attempt to update a shared table when it shouldn't be to doFixIpbAddressUniqueIndex results in an attempt to update a shared table when it shouldn't.Oct 7 2020, 4:12 PM
gerritbot added a comment.Oct 8 2020, 12:25 AM

Change 632749 merged by jenkins-bot:
[mediawiki/core@master] Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields

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

gerritbot added a comment.Oct 8 2020, 12:25 AM

Change 632811 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields

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

gerritbot added a comment.Oct 8 2020, 12:32 AM

Change 632811 merged by jenkins-bot:
[mediawiki/core@REL1_35] Remove doTable check from (Mysql|Sqlite)Updater::indexHasFields

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

Reedy closed this task as Resolved.Oct 8 2020, 12:32 AM
Reedy claimed this task.
Nintendofan885 removed a project: Patch-For-Review.Oct 8 2020, 10:45 AM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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