Hello,
With the upcoming 0.12b1 release, we will probably have a lot more
people upgrading their 0.11 database to 0.12, i.e. going from
database_version 21 to 26. This will likely trigger errors we're not yet
aware of. Actually I'm writing this as a follow-up to
http://trac.edgewall.org/ticket/9268, which reported such upgrade
problem. I'm wondering if we shouldn't make this upgrade a bit more
robust than it currently is, and more convenient to debug. In
particular, we could consider each upgrade step (i.e. each call to an
IEnvironmentSetupParticipant's upgrade_environment method) as being
self-contained. They already are in practice, as schema modifications
can't be rollbacked for the SQLite and MySQL database backends
This reminded me of a discussion we had in Bitten
(http://bitten.edgewall.org/ticket/462) and indeed hodgestar later added
an explicit db.commit(), despite the Trac documentation advising to not
do it.
Well, I think we should now simply invalidate this advice and say that
Trac not only takes care of doing a commit after each step, but that in
addition it should also be possible to perform finer grained commits
inside an upgrade_environment method if appropriate. It's certainly
appropriate for the trac.env.EnvironmentSetup.upgrade_environment method
(i.e. the place where we go through the trac/upgrades/db<version>.py steps).
I tested the attached patch with sqlite, postgresql and mysql, for an
upgrade of a repository-less 0.11-stable environment upgraded to trunk.
Tell me what you think about it, but I think it's a worthy change to do
before the beta1.
This was also the occasion to output something in the log about which
step is currently being executed.
An interesting exercise for 0.13 would be to add an unit-test which
would create an environment with the database_version==1 schema, and
make it go through all the updates ;-)
-- Christian
--
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en.
# HG changeset patch
# Parent 826e2cb931b56a6795ca37f6505d06171a5e7128
Make environment upgrades more robust by committing after each upgrade step.
This was anyway the de facto way to make upgrades more robust (see e.g. [Bitten
786/trunk/bitten/main.py]).
diff --git a/trac/env.py b/trac/env.py
--- a/trac/env.py
+++ b/trac/env.py
@@ -65,9 +65,13 @@ class IEnvironmentSetupParticipant(Inter
def upgrade_environment(db):
"""Actually perform an environment upgrade.
- Implementations of this method should not commit any database
- transactions. This is done implicitly after all participants have
- performed the upgrades they need without an error being raised.
+ Implementations of this method don't need to commit any database
+ transactions. This is done implicitly for each participant
+ if the upgrade succeeds without an error being raised.
+
+ However, if the `upgrade_environment` consists of small, restartable,
+ steps of upgrade, it can decide to commit on its own after each
+ successful step.
"""
@@ -509,35 +513,27 @@ class Environment(Component, ComponentMa
def upgrade(self, backup=False, backup_dest=None):
"""Upgrade database.
- Each db version should have its own upgrade module, named
- upgrades/dbN.py, where 'N' is the version number (int).
-
@param backup: whether or not to backup before upgrading
@param backup_dest: name of the backup file
@return: whether the upgrade was performed
"""
upgraders = []
-
- @with_transaction(self)
- def do_upgrade(db):
- for participant in self.setup_participants:
- if participant.environment_needs_upgrade(db):
- upgraders.append(participant)
- if not upgraders:
- return
-
- if backup:
- self.backup(backup_dest)
+ db = self.get_read_db()
+ for participant in self.setup_participants:
+ if participant.environment_needs_upgrade(db):
+ upgraders.append(participant)
+ if not upgraders:
+ return
- for participant in upgraders:
- participant.upgrade_environment(db)
+ if backup:
+ self.backup(backup_dest)
- if not upgraders:
- return False
-
- # Database schema may have changed, so close all connections
- self.shutdown(except_logging=True)
-
+ for participant in upgraders:
+ self.log.info("%s.%s upgrading...", participant.__module__,
+ participant.__class__.__name__)
+ with_transaction(self)(participant.upgrade_environment)
+ # Database schema may have changed, so close all connections
+ self.shutdown(except_logging=True)
return True
def _get_href(self):
@@ -590,6 +586,9 @@ class EnvironmentSetup(Component):
return True
def upgrade_environment(self, db):
+ """Each db version should have its own upgrade module, named
+ upgrades/dbN.py, where 'N' is the version number (int).
+ """
cursor = db.cursor()
dbver = self.env.get_version()
for i in range(dbver + 1, db_default.db_version + 1):
@@ -601,10 +600,11 @@ class EnvironmentSetup(Component):
raise TracError(_('No upgrade module for version %(num)i '
'(%(version)s.py)', num=i, version=name))
script.do_upgrade(self.env, i, cursor)
- cursor.execute("UPDATE system SET value=%s WHERE "
- "name='database_version'", (db_default.db_version,))
- self.log.info('Upgraded database version from %d to %d',
- dbver, db_default.db_version)
+ cursor.execute("""
+ UPDATE system SET value=%s WHERE name='database_version'
+ """, (i,))
+ self.log.info('Upgraded database version from %d to %d', i - 1, i)
+ db.commit()
self._update_sample_config()
# Internal methods
