Merge lp:~rlucio/nova/lp677475 into lp:~hudson-openstack/nova/trunk

[Ryan Lucio]

Addresses bug 677475 by changing the DB column for internal_id in the instances table to be unsigned.

[Jesse Andrews (anotherjesse)]

How does this affect postgresql and sqlite users?

[Soren Hansen (soren)]

Perhaps we'd be better off with a smaller key space? I.e. change
nova.db.api instance_create to choose a random id that will fit in an
int32.

--
Soren Hansen
Ubuntu Developer    http://www.ubuntu.com/
OpenStack Developer http://www.openstack.org/

[Ryan Lucio (rlucio)]

- From my brief experience with the sqlalchemy libraries, it appears that all the mysql specific calls do is support its 'enhanced' column types/options. So if the db is not mysql the mysql.MSInteger call will simply ignore the mysql options as if you had called the base class sqlalchemy.types.Integer. I can do some testing for sqllite to see.

- Yes the easy alternative is simply to change the calculations to range from 0 to (2^31 - 1). The reason I didn't go that route is that it isn't documented why the 0 to (2^32 - 1) range was chosen in the first place, and I wasn't sure if it was arbitrary or chosen for a specific reason.

[lp:~rlucio/nova/lp677475]

440. By Ryan Lucio on 2010-12-03

Decreased the maximum value for instance-id generation from uint32 to int32 to avoid truncation when being entered into the instance table. Reverted fix to make internal_id column a uint

[Ryan Lucio (rlucio)]

This fix was tested/verified on a multi-box deployment configured to use a mysql db.

[Michael Gundlach (gundlach)]

2^31 should work. This was my code (and my bug -- thanks for addressing it.) 2^32 isn't special -- we just wanted a large enough space so that we wouldn't run out of instance IDs. 2^31 should be fine.

When we split into one sqlite DB per compute worker instead of a centralized DB, we'll need to revisit this anyway, since creating a new instance currently relies on manually checking the DB for uniqueness.

[Michael Gundlach (gundlach)]

lgtm.

review: Approve

[Eric Day (eday)]

This needs to be refactored soon. The key space is too small and this is a very inefficient way to generate unique IDs (especially once the data can be distributed). Approving for now since it fixes the issue at hand.

review: Approve

[David Pravec (alekibango)]

lgtm for now. but i would also say refactor later...

review: Approve

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2010-12-01 21:53:27 +0000
+++ nova/db/sqlalchemy/api.py	2010-12-03 21:58:09 +0000
@@ -543,7 +543,7 @@
     with session.begin():
         while instance_ref.internal_id == None:
             # Instances have integer internal ids.
-            internal_id = random.randint(0, 2 ** 32 - 1)
+            internal_id = random.randint(0, 2 ** 31 - 1)
             if not instance_internal_id_exists(context, internal_id,
                                                session=session):
                 instance_ref.internal_id = internal_id

=== modified file 'nova/image/local.py'
--- nova/image/local.py	2010-11-18 21:27:52 +0000
+++ nova/image/local.py	2010-12-03 21:58:09 +0000
@@ -59,7 +59,7 @@
         """
         Store the image data and return the new image id.
         """
-        id = random.randint(0, 2 ** 32 - 1)
+        id = random.randint(0, 2 ** 31 - 1)
         data['id'] = id
         self.update(context, id, data)
         return id