Open Stack

Hi Matt, interesting questions/points. Some comments inline.

On Sun, 2014-03-09 at 15:20 -0500, Matt Riedemann wrote:
> <snip>
> I've been working on this more and am running up against some issues, 
> part of this has to do with my lack of sqlalchemy know-how and 
> inexperience with writing DB migrations, so dumping some info/problems 
> here to see where people would like me to take this.
> 
> My original thinking for doing a migration was to delete the instances 
> records where uuid == None and then move those to shadow_instances, then 
> make instances.uuid.nullable=False.  Some of the problems with this 
> approach are:
> 
> 1. There are at least 5 other tables related to instances that need to 
> be handled for a delete: InstanceFault, InstanceMetadata, 
> InstanceSystemMetadata, InstanceInfoCache and 
> SecurityGroupInstanceAssociation. Also, these tables don't define their 
> instance_uuid column the same way, some have it nullable=False and 
> others don't.

All tables should define the instance uuid column in the same way, and
making that consistent should best be done as part of the same
migration. The reasoning, of course, is that these columns represent
identical data (primary to child key relations), and therefore the
schema of all the columns should be identical.

> 2. I'm not sure if I can use a session in the migration to make it a 
> transaction.

You can use a transaction around all statements, including DDL, in
PostgreSQL. In MySQL, you can use a transaction only around DML
statements, but any DDL statement will cause a flush of all transactions
before the DDL statement begins.

I'd recommend doing all of the necessary "moves" of records (between the
shadow tables and regular tables) within a single transaction so that at
least there is some level of data consistency ensured for those
operations. But, unfortunately, the DDL schema changes will not occur
within a transaction unless you are using PostgreSQL and Alembic
migrations.

> 3. This would make the instances and shadow_instances tables have 
> different schemas, i.e. instances.uuid would be nullable=False in 
> instances but nullable=True in shadow_instances.  Maybe this doesn't matter.

No, I don't think this matters much, to be honest. I'm not entirely sure
what the long-term purpose of the shadow tables are in Nova -- perhaps
someone could clue me in to whether the plan is to keep them around?

> The whole reason behind using shadow_instances (or any backup table I 
> guess) was so I could restore the records on DB downgrade.
> 
> So the more I think about this, I'm getting to the point of asking:
> 
> 1. Do we even care about instances where uuid is None?  I'd have to 
> think those wouldn't be working well in the current code with how 
> everything relies on uuid for foreign keys and tracking relationships to 
> volumes, images and networks across services.  

Agreed. I don't personally think instance records with no UUID are
useful in Nova any more.

> If the answer is 'no' 
> then the migration is pretty simple, just delete the records where uuid 
> is None and be done with it.  You couldn't downgrade to get them back, 
> but in this case we're asserting that we don't want them back.

This is exactly what I would recommend.

> 2. Have an alternative schema in the DB2 case. This would be handled in 
> the 216_havana migration when the instances table is defined and 
> created, we'd just make the uuid column non-nullable in the DB2 case and 
> leave it nullable for all other engines.  Anyone moving to DB2 would 
> have to install from scratch anyway since there is no tooling to migrate 
> a MySQL DB to DB2, for example.  As it stands, the 216_havana migration 
> in my patch [1] already has a different schema for DB2 because of the 
> foreign keys it can't create due to this problem.

Nah, I don't think we want to keep a special schema just for DB2. That's
technical debt that I don't think it worthwhile to keep around...

Best,
-jay

> Anyway, looking for some thoughts on how to best handle this, or if 
> anyone has other ideas or good reasons why either approach couldn't be used.
> 
> [1] https://review.openstack.org/#/c/69047/
>