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

[Ryan Lucio]

Adds the pool_recycle option to the sql engine startup call. This enables connection auto-timeout so that connection pooling will work properly. The recommended setting (per sqlalchemy FAQ page) has been provided as a default for a new configuration flag. What this means is that if a db connection sits idle for the configured # of seconds, the engine will automatically close the connection and return it to the available thread pool. See Bug #690314 for info.

The fix was tested and verified on multi-node deployments of Austin and Bexar with MySQL, and it was also verified that the change does not affect sqlite users (dev environment testing only).

[Thierry Carrez (ttx)]

3600 sounds like a sane default given most databases idle connection timeout... but I don't think we should hardcode it. Adding a flag to change that default value would allow us to cover all corner cases ?

[Ryan Lucio (rlucio)]

If you think it is necessary that is fine. How about a new flag, sql_idle_timeout that defaults to 3600?

[Devin Carlen (devcamcar)]

sql_idle_timeout flag is a reasonable addition

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Eric Day (eday)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~rlucio/nova/lp690314 into lp:nova failed. Below is the output from the failed tests.

nova/db/sqlalchemy/session.py:39:58: W291 trailing whitespace
            _ENGINE = create_engine(FLAGS.sql_connection,
                                                         ^
    JCR: Trailing whitespace is superfluous.
    FBM: Except when it occurs as part of a blank line (i.e. the line is
         nothing but whitespace). According to Python docs[1] a line with only
         whitespace is considered a blank line, and is to be ignored. However,
         matching a blank line to its indentation level avoids mistakenly
         terminating a multi-line statement (e.g. class declaration) when
         pasting code into the standard Python interpreter.

         [1] http://docs.python.org/reference/lexical_analysis.html#blank-lines

    The warning returned varies on whether the line itself is blank, for easier
    filtering for those who want to indent their blank lines.

    Okay: spam(1)
    W291: spam(1)\s
    W293: class Foo(object):\n \n bang = 12
nova/db/sqlalchemy/session.py:40:73: W291 trailing whitespace
                                    pool_recycle=FLAGS.sql_idle_timeout,
                                                                        ^
    JCR: Trailing whitespace is superfluous.
    FBM: Except when it occurs as part of a blank line (i.e. the line is
         nothing but whitespace). According to Python docs[1] a line with only
         whitespace is considered a blank line, and is to be ignored. However,
         matching a blank line to its indentation level avoids mistakenly
         terminating a multi-line statement (e.g. class declaration) when
         pasting code into the standard Python interpreter.

         [1] http://docs.python.org/reference/lexical_analysis.html#blank-lines

    The warning returned varies on whether the line itself is blank, for easier
    filtering for those who want to indent their blank lines.

    Okay: spam(1)
    W291: spam(1)\s
    W293: class Foo(object):\n \n bang = 12

=== modified file 'nova/db/sqlalchemy/session.py'
--- nova/db/sqlalchemy/session.py	2010-10-21 23:15:26 +0000
+++ nova/db/sqlalchemy/session.py	2011-01-03 19:12:01 +0000
@@ -36,7 +36,9 @@
     global _MAKER
     if not _MAKER:
         if not _ENGINE:
-            _ENGINE = create_engine(FLAGS.sql_connection, echo=False)
+            _ENGINE = create_engine(FLAGS.sql_connection,
+                                    pool_recycle=FLAGS.sql_idle_timeout,
+                                    echo=False)
         _MAKER = (sessionmaker(bind=_ENGINE,
                                 autocommit=autocommit,
                                 expire_on_commit=expire_on_commit))
 
=== modified file 'nova/flags.py'
--- nova/flags.py	2010-12-27 19:45:57 +0000
+++ nova/flags.py	2011-01-03 19:12:01 +0000
@@ -263,6 +263,9 @@
 DEFINE_string('sql_connection',
               'sqlite:///$state_path/nova.sqlite',
               'connection string for sql database')
+DEFINE_string('sql_idle_timeout',
+              '3600',
+              'timeout for idle sql database connections')
 
 DEFINE_string('compute_manager', 'nova.compute.manager.ComputeManager',
               'Manager for compute')