Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004715opensim[GRID] Asset Servicepublic2010-05-19 05:502011-08-26 21:19
Assigned To 
PlatformOSOS Version
Product Versionmaster (dev code) 
Target VersionFixed in Version 
Summary0004715: Inconsistent use of transactions in migration scripts may cause unexpected problems
DescriptionThe code in Migrations.cs has the following catch block after executing a command:

catch (Exception e)
    m_log.DebugFormat("[MIGRATIONS] Cmd was {0}", e.Message.Replace("\n", " "));
    m_log.Debug("[MIGRATIONS]: An error has occurred...");

The block is supposed to log the error and continue with the migration.

However, the ROLLBACK at the end might itself throw an exception when there is
no active transaction. This means that absolutely any portion of the migration script must be surrounded by BEGIN .. COMMIT. However, the rules of what kinds of SQL can or can not be executed within a transaction vary for various databases. For example, in MySQL schema changes (add column, etc) are not rolled back. Also a single SQL statement is rolled back automatically and doesn't require an explicit transaction.

It seems unwise to have a hardcoded "ROLLBACK" for scripts that are written at different times, by different people and may or may not run within a transaction.

As it is, there are migration sections for various stores/DBs that are not surrounded by a transaction. If any of that fails, the migration will be aborted with an exception rather than continued.

I'm not sure what a perfect solution would be, but one possibility may be this:

* make the Migration class recognize some kind of ":TRANS" marker in the script,
starting an explicit transaction from that point on to the next ":TRANS" or to the end of the script section. Fully manage these transactions within the Migration code.

* remove all explicit BEGINs/COMMITs from all migrations for all DBs. Instead, use the ":TRANS" marker and only when it is necessary (multiple commands, actually transactional).

This looks like something for me to fix, but I'd like this report to be
reviewed and confirmed first.

(Just in case: this issue is NOT a result of my rewriting of the Migration
class in the latest commits. The ROLLBACK has been there for a long time!)
TagsNo tags attached.
Git Revision or version number12808
Run ModeStandalone (1 Region)
Physics EngineBasicPhysics
Script Engine
Environment.NET / Windows32
Mono VersionNone
Attached Files

- Relationships

-  Notes
melanie (administrator)
2010-05-19 11:24

The old code didn't check for errors thrown by that rollback, so if it wasn't proper, it would be ignored. Also, there is a contract:

Any migration MUST be enclosed in an EXPLICIT transaction.

No migration can include both schema changes and data manipulation

So, the rollback will roll back the data manipulation (insert into xxx select fff from yyy) types of migration. it will not do anything for the schema alterations. However, since the data manipulation migrations, which are expected to fail in certain cases, can never be combined with schema manipulation, this is acceptable.

Schema manipulation is not expected to fail. If it does, it means the user has manually modified their database, which means they're on their own.

That is how it was designed. How to change it, is up for discussion.
makopoppo (manager)
2011-08-26 21:19

Probably it is still in open discussion for architecture proposal.

- Issue History
Date Modified Username Field Change
2010-05-19 05:50 AlexRa New Issue
2010-05-19 05:50 AlexRa Git Revision => 12808
2010-05-19 05:50 AlexRa SVN Revision => 0
2010-05-19 05:50 AlexRa Run Mode => Standalone (1 Region)
2010-05-19 05:50 AlexRa Physics Engine => BasicPhysics
2010-05-19 05:50 AlexRa Environment => .NET / Windows32
2010-05-19 05:50 AlexRa Mono Version => None
2010-05-19 11:24 melanie Note Added: 0015478
2011-08-26 21:19 makopoppo Note Added: 0019745
2011-08-26 21:19 makopoppo Severity minor => feature
2011-08-26 21:19 makopoppo Reproducibility sometimes => N/A
2011-08-26 21:19 makopoppo Status new => acknowledged

Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker