Edgewall Software
Modify

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#8525 closed defect (fixed)

Multirepos Databse Upgrade from 0.11 Fails w/ PostgreSQL

Reported by: John Hampton Owned by: Christian Boos
Priority: normal Milestone: 0.12-multirepos
Component: version control Version: none
Severity: blocker Keywords: multirepos
Cc:
Release Notes:
API Changes:

Description

Attempting to upgrade from 0.11-stable (r8161) to MultiRepos trunk (r8380) fails during trac-admin env upgrade

Issue is located at upgrades/db23.py#L6. If the repository table doesn't exist (which it doesn't due to upgrading from 0.11), then the select fails. However, on PostgreSQL, this leaves the cursor in a state requiring a rollback. Since there is no rollback in the execept block, then it fails with:

InternalError: current transaction is aborted, commands ignored until end of transaction block

The real kicker here, however, is that if you simply put a rollback in the except block, then you knock out the upgrades done in upgrades/db22.py which means the cache table never gets created. Thus leaving trac in a broken state.

It seems as though the purpose of the try/except block in do_upgrade is simply meant to replay upgrades/db22.py. This doesn't make sense. Why would the database base upgrade need to be re-run? Anyway, I don't have a good idea on how to fix this, other than eliminating the try/except block such that it's unnecessary.

Attachments (0)

Change History (7)

comment:1 Changed 8 years ago by Christian Boos

The reason why the code is like it is was clearly explained by the comments.

But you're right, I didn't expect the failed "SELECT" to leave the connection in an unusable state, so it's broken for PostgreSQL at least.

When moving on trunk, all this "compatibility" code due to the db22.py to db23.py rename that happened at one point during the development will be removed and this issue will go away.

Btw. Remy, what about removing this code now?

comment:2 in reply to:  1 Changed 8 years ago by Remy Blank

Replying to cboos:

Btw. Remy, what about removing this code now?

Sure, go ahead.

comment:3 in reply to:  1 ; Changed 8 years ago by John Hampton

Replying to cboos:

The reason why the code is like it is was clearly explained by the comments.

I did read the comment. What I meant to say is that I don't understand why one would ever replay a db upgrade. If something was missed in the first db upgrade, then simply create another one, etc.

But you're right, I didn't expect the failed "SELECT" to leave the connection in an unusable state, so it's broken for PostgreSQL at least.

With PostgreSQL, if ever a query fails, ie, you have to use a try/except then the cursor is in a bad state and must be rolled back. (Un?)fortunately, SQLite does not have this issue. I also don't know about MySQL.

Btw. Remy, what about removing this code now?

FYI, I have tested the removing of the code and it does work. So removal is a +1 from me.

comment:4 in reply to:  3 Changed 8 years ago by Remy Blank

Replying to jhampton:

I did read the comment. What I meant to say is that I don't understand why one would ever replay a db upgrade. If something was missed in the first db upgrade, then simply create another one, etc.

This was due to having two DB upgrades in parallel, one on multirepos and one on trunk. At some point, db23 was called db22 on multirepos, and it had to be renamed to db23 after db22 was created on trunk. So there were some multirepos installations that already had db23 applied (as db22), but needed the new db22. The check was supposed to catch that case.

comment:5 Changed 8 years ago by Christian Boos

Owner: set to Christian Boos

"Buggy" compatibility code removed in r8395.

comment:6 Changed 8 years ago by Christian Boos

Resolution: fixed
Status: newclosed

I think we can close the completed MultiRepos tickets. As soon as we have the separate 0.12-multirepos milestone, as discussed yesterday in #1507, we'll be able to distinguish what's closed on trunk and what's closed on the branch.

comment:7 Changed 8 years ago by Christian Boos

Milestone: 0.120.12-multirepos

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .

 
Note: See TracTickets for help on using tickets.