openstack
/
oslo.db
Code Issues Proposed changes

Rename idle_timeout to connection_recycle_time 

The "idle_timeout" name and documentation are misleading to users.
Rename the option to connection_recycle_time to more closely
match that of the underlying SQLAlchemy argument name
"pool_recycle", which refers to a time period for which
a connection present in the pool for longer than that number
of seconds will be closed and replaced with a new one *upon checkout*,
to prevent usage of stale connections that may have been closed by
server-side reapers.

Change-Id: I5e39d5ac81eaa8d85b1dfb18862fe27695ab8b96
This commit is contained in:
Mike Bayer 2016-06-25 11:22:02 -04:00
parent 92c5091d4c
commit 6634218415
7 changed files with 59 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
oslo_db/options.py
View File

@ -48,15 +48,25 @@ database_opts = [
default=False,
help='If True, transparently enables support for handling '
'MySQL Cluster (NDB).'),
cfg.IntOpt('idle_timeout',
default=3600,
deprecated_opts=[cfg.DeprecatedOpt('sql_idle_timeout',
group='DEFAULT'),
cfg.DeprecatedOpt('sql_idle_timeout',
group='DATABASE'),
cfg.DeprecatedOpt('idle_timeout',
group='sql')],
help='Timeout before idle SQL connections are reaped.'),
cfg.IntOpt(
'connection_recycle_time',
default=3600,
deprecated_opts=[
cfg.DeprecatedOpt('idle_timeout',
group="DATABASE"),
cfg.DeprecatedOpt('idle_timeout',
group="database"),
cfg.DeprecatedOpt('sql_idle_timeout',
group='DEFAULT'),
cfg.DeprecatedOpt('sql_idle_timeout',
group='DATABASE'),
cfg.DeprecatedOpt('idle_timeout',
group='sql')
],
help='Connections which have been present in the connection '
'pool longer than this number of seconds will be replaced '
'with a new one the next time they are checked out from '
'the pool.'),
cfg.IntOpt('min_pool_size',
default=1,
deprecated_opts=[cfg.DeprecatedOpt('sql_min_pool_size',

18
oslo_db/sqlalchemy/enginefacade.py
View File

@ -18,7 +18,8 @@ import operator
import threading
import warnings
import debtcollector.removals as removals
import debtcollector.removals
import debtcollector.renames
from oslo_config import cfg
from oslo_utils import excutils
@ -136,7 +137,7 @@ class _TransactionFactory(object):
'sqlite_fk': _Default(False),
'mysql_sql_mode': _Default('TRADITIONAL'),
'mysql_enable_ndb': _Default(False),
'idle_timeout': _Default(3600),
'connection_recycle_time': _Default(3600),
'connection_debug': _Default(0),
'max_pool_size': _Default(),
'max_overflow': _Default(),
@ -177,6 +178,8 @@ class _TransactionFactory(object):
self._legacy_facade = None
self._start_lock = threading.Lock()
@debtcollector.renames.renamed_kwarg(
"idle_timeout", "connection_recycle_time", replace=True)
def configure_defaults(self, **kw):
"""Apply default configurational options.
@ -209,7 +212,7 @@ class _TransactionFactory(object):
:param mysql_enable_ndb: enable MySQL Cluster (NDB) support
:param idle_timeout: connection pool recycle time,
:param connection_recycle_time: connection pool recycle time,
defaults to 3600. Note the connection does not actually have to
be "idle" to be recycled.
@ -282,6 +285,8 @@ class _TransactionFactory(object):
"""
self._configure(True, kw)
@debtcollector.renames.renamed_kwarg(
"idle_timeout", "connection_recycle_time", replace=True)
def configure(self, **kw):
"""Apply configurational options.
@ -529,7 +534,7 @@ class _TestTransactionFactory(_TransactionFactory):
"""
@removals.removed_kwarg(
@debtcollector.removals.removed_kwarg(
'synchronous_reader',
'argument value is propagated from the parent _TransactionFactory')
def __init__(self, engine, maker, apply_global, from_factory=None, **kw):
@ -1196,8 +1201,8 @@ class LegacyEngineFacade(object):
:keyword mysql_enable_ndb: If True, transparently enables support for
handling MySQL Cluster (NDB).
(defaults to False)
:keyword idle_timeout: timeout before idle sql connections are reaped
(defaults to 3600)
:keyword connection_recycle_time: Time period for connections to be
recycled upon checkout (defaults to 3600)
:keyword connection_debug: verbosity of SQL debugging information.
-1=Off, 0=None, 100=Everything (defaults
to 0)
@ -1230,7 +1235,6 @@ class LegacyEngineFacade(object):
"oslo_db.sqlalchemy.enginefacade",
exception.OsloDBDeprecationWarning,
stacklevel=2)
if _factory:
self._factory = _factory
else:

7
oslo_db/sqlalchemy/engines.py
View File

@ -22,6 +22,7 @@ import os
import re
import time
import debtcollector.renames
import six
import sqlalchemy
from sqlalchemy import event
@ -121,9 +122,11 @@ def _vet_url(url):
)
@debtcollector.renames.renamed_kwarg(
"idle_timeout", "connection_recycle_time", replace=True)
def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None,
mysql_enable_ndb=False,
idle_timeout=3600,
connection_recycle_time=3600,
connection_debug=0, max_pool_size=None, max_overflow=None,
pool_timeout=None, sqlite_synchronous=True,
connection_trace=False, max_retries=10, retry_interval=10,
@ -137,7 +140,7 @@ def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None,
_vet_url(url)
engine_args = {
"pool_recycle": idle_timeout,
"pool_recycle": connection_recycle_time,
'convert_unicode': True,
'connect_args': {},
'logging_name': logging_name

3
oslo_db/tests/sqlalchemy/test_enginefacade.py
View File

@ -2156,7 +2156,8 @@ class ConfigOptionsTest(oslo_test_base.BaseTestCase):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
factory.configure(fake1='x', idle_timeout=200, wrong2='y')
factory.configure(
fake1='x', connection_recycle_time=200, wrong2='y')
self.assertEqual(1, len(w))
self.assertTrue(

2
oslo_db/tests/sqlalchemy/test_options.py
View File

@ -84,6 +84,7 @@ pool_timeout=7
]])[0]
self.conf(['--config-file', path])
self.assertEqual('fake_connection', self.conf.database.connection)
self.assertEqual(100, self.conf.database.connection_recycle_time)
self.assertEqual(100, self.conf.database.idle_timeout)
self.assertEqual(99, self.conf.database.min_pool_size)
self.assertEqual(199, self.conf.database.max_pool_size)
@ -99,6 +100,7 @@ pool_timeout=7
]])[0]
self.conf(['--config-file', path])
self.assertEqual('test_sql_connection', self.conf.database.connection)
self.assertEqual(99, self.conf.database.connection_recycle_time)
self.assertEqual(99, self.conf.database.idle_timeout)
def test_deprecated_dbapi_parameters(self):

6
oslo_db/tests/sqlalchemy/test_sqlalchemy.py
View File

@ -316,6 +316,10 @@ class EngineFacadeTestCase(oslo_test.BaseTestCase):
self.assertFalse(ses.autocommit)
self.assertTrue(ses.expire_on_commit)
def test_direct_invocation_deprecated_args(self):
facade = session.EngineFacade("sqlite://", idle_timeout=59)
self.assertEqual(59, facade.get_engine().pool._recycle)
@mock.patch('oslo_db.sqlalchemy.orm.get_maker')
@mock.patch('oslo_db.sqlalchemy.engines.create_engine')
def test_creation_from_config(self, create_engine, get_maker):
@ -343,7 +347,7 @@ class EngineFacadeTestCase(oslo_test.BaseTestCase):
mysql_sql_mode='TRADITIONAL',
mysql_enable_ndb=False,
sqlite_fk=False,
idle_timeout=mock.ANY,
connection_recycle_time=mock.ANY,
retry_interval=mock.ANY,
max_retries=mock.ANY,
max_overflow=mock.ANY,

15
releasenotes/notes/deprecate_idle_timeout-029d9f2cb7184b28.yaml Normal file
View File

@ -0,0 +1,15 @@
---
deprecations:
- |
The configuration option ``idle_timeout`` is now deprecated and has been
renamed to ``connection_recycle_time``, including within the main oslo.db
options, as well as in the keyword arguments to
``engines.create_engine()``, ``enginefacade.configure()`` and
``enginefacade.configure_defaults()``. The new name more accurately
describes what this option does, in that it is not directly related to the
"idle" time of the connection itself, nor is the connection disconnected at
any specific time. It refers to a rule stating that any connection which
has been present more than N seconds as a member of the connection pool
will be automatically discarded and replaced the next time it is checked
out from the pool.