Page MenuHomePhabricator

update.php for Postgres creates sequences, then fails when it to rename sequences to those same names
Closed, ResolvedPublic

Description

Author: andy

Description:
In includes/installer/PostgresUpdater.php, there's a function getCoreUpdateList() that returns a list of actions for the updater to take.

There are two actions of adding new sequences:

array( 'addSequence', 'logging_log_id_seq' ),
array( 'addSequence', 'page_restrictions_pr_id_seq' ),

However, after that, it's specified to rename existing sequences to those very names:

array( 'renameSequence', 'log_log_id_seq', 'logging_log_id_seq' ),
array( 'renameSequence', 'pr_id_val', 'page_restrictions_pr_id_seq' ),

Since the updater just created logging_log_id_seq, trying to rename log_log_id_seq to logging_log_id_seq fails.

I commented out the top two lines, dropped the sequence logging_log_id_seq that had been created, and now my update runs correctly, and the old sequence gets a new name.


Version: 1.19.2
Severity: normal

Details

Reference
bz29635

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:37 PM
bzimport set Reference to bz29635.
bzimport added a subscriber: Unknown Object (MLST).

l.corsaro wrote:

Patch to skip existent sequence and throw a warning

I've resolved and tested, it seems to work

attachment PostgresUpdater.php.patch ignored as obsolete

Your patch removes some non-existent code. Perhaps you meant it should be added?

  • /home/www-data/wiki/tf2m/includes/installer/PostgresUpdater.php 2011-08-16 12:34:08.000000000 +0200

+++ /home/www-data/wiki/mediawiki-1.17.0/includes/installer/PostgresUpdater.php 2011-06-10 13:55:23.000000000 +0200

Looks like it might be diffing the wrong way...

r103766

sumanah wrote:

Thanks for the patch, Luigi.

nedelcumax wrote:

But is this really fixing the problem? Sure, the patch will prevent the updater to crash. But shouldn't also the real problem be addressed? I'm new to this hole Postgres thing so I can't say how this impacts but in my opinion:

  • either the new sequences get created and the the old ones are no longer renamed
  • the new sequences are not created but instead the old ones are renamed.

I'm suffering to the sequences mention by Andy in the bug description.

nedelcumax wrote:

Except for the fact that my previous comment looks like it was written by a complete moron, here's my 2 cent on this: I was having problems with uploading files, kept getting an error:

Database error
A database query syntax error has occurred. This may indicate a bug in the software. The last attempted database query was:
(SQL query hidden)
from within function "LogPage::saveContent". Database returned error "1: ERROR: duplicate key value violates unique constraint "logging_pkey"".

After removing the two actions of adding new sequences and just letting the script rename the old ones uploading files was working again.

t.glaser wrote:

Thanks, your analysis of the problem is correct.
However, the issue is *not* fixed, especially not for people who upgraded within the timeframe where it occurred, but also because due to your change, the sequence is *not* renamed, but a new one is created and the old one kept.

I’ve prepared a patch that can be used to addOrRenameSequence() in the future and fixes up this very specific situation. (I’ve got 238 MediaWiki instances on our farms and am so not going to do that manually.)

I’ve tested this on a development system instance so far, and it looks working, but would love to have it discussed before I upload it to Debian and try to get the Release Team to do another freeze exception.

Thanks in advance!

t.glaser wrote:

Fixup databases where the problem already occurred

Attached:

Are you using 1.17?

This should have been fixed in March 2012 in master (git commit f752cf80423615b380cf5612a3f1f68a6b9d0173).

Can you try that version of PostgresUpdater.php?

I was testing those changes by upgrading MediaWiki 1.7 to master using PostgresUpdater and it worked fine for me.

t.glaser wrote:

I’m using 1.19.2.

The commit f752cf80423615b380cf5612a3f1f68a6b9d0173 will *not* fix the issue, because there’s still an

array( 'addSequence', 'logging_log_id_seq'          ),
array( 'addSequence', 'page_restrictions_pr_id_seq' ),

*before* the

array( 'renameSequence', 'log_log_id_seq',      'logging_log_id_seq'          ),

calls. Furthermore, once this code has been run, i.e. anyone who upgraded an 1.19 release from e.g. 1.15, *two* sequences exist, so the fixup patch I submitted (except the last [5~ line, that was editor junk) is still required.

wikimedia-bugs wrote:

Attachment 11168 confirmed to fix things when upgrading from mediawiki 1.15 to 1.19.2, postgresql 9.1.3, after update.php first introduced the problem.

with PostgresUpdater.php patched, and maintenance/update.php invoked for a second time, things seem to be back in order, with the mediawiki schema containing a total of 13 named sequences rather than 15 with two pairs of similarly named sequences.

sumanah wrote:

Great, Amir! Can you go ahead and put that patch in Git? https://www.mediawiki.org/wiki/How_to_contribute will give you the information you need. Thanks!

Fixed by gerrit 3365/gerrit 4672 where the addSequences was moved after the renameSequences.

Jdforrester-WMF subscribed.

Migrating from the old tracking task to a tag for PostgreSQL-related tasks.