Page MenuHomePhabricator

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

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 )
...

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2020, 10:47 PM
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

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

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?

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

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

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

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

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

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

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.

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

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

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

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

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.