Make memoize behave better in concurrent environments
Currently there is a chance for System view to raise sqlite3 exceptions because of table lock in PersistentDict. 1) Stop re-loading content on PersistentDict creation. 2) Retry OperationalError on all database accesses. 3) Rule out a race condition in __delitem__ Also refactor tests to remove excessive mocking. Change-Id: I82726c41578700835f3b15d316aed562d68fbf67
This commit is contained in:
Dmitry Tantsur
parent
e7dbba93f1
commit
8d52482c79
@ -55,6 +55,7 @@ Sphinx==2.0.0
|
||||
sphinxcontrib-websupport==1.0.1
|
||||
stestr==1.0.0
|
||||
stevedore==1.20.0
|
||||
tenacity==6.2.0
|
||||
testrepository==0.0.20
|
||||
testscenarios==0.4
|
||||
testtools==2.2.0
|
||||
|
||||
@ -5,3 +5,4 @@
|
||||
pbr!=2.1.0,>=2.0.0 # Apache-2.0
|
||||
Flask>=1.0.2 # BSD
|
||||
requests>=2.14.2 # Apache-2.0
|
||||
tenacity>=6.2.0 # Apache-2.0
|
||||
|
||||
@ -26,6 +26,8 @@ import pickle
|
||||
import sqlite3
|
||||
import tempfile
|
||||
|
||||
import tenacity
|
||||
|
||||
# Python 3.8
|
||||
MutableMapping = getattr(collections, 'abc', collections).MutableMapping
|
||||
|
||||
@ -71,6 +73,13 @@ def memoize(permanent_cache=None):
|
||||
return decorator
|
||||
|
||||
|
||||
_retry = tenacity.retry(
|
||||
retry=tenacity.retry_if_exception_type(sqlite3.OperationalError),
|
||||
wait=tenacity.wait_exponential(min=0.1, max=2, multiplier=1),
|
||||
stop=tenacity.stop_after_delay(5),
|
||||
reraise=True)
|
||||
|
||||
|
||||
class PersistentDict(MutableMapping):
|
||||
DBPATH = os.path.join(tempfile.gettempdir(), 'sushy-emulator')
|
||||
|
||||
@ -88,8 +97,6 @@ class PersistentDict(MutableMapping):
|
||||
'(key blob primary key not null, value blob not null)'
|
||||
)
|
||||
|
||||
self.update(dict(self))
|
||||
|
||||
@staticmethod
|
||||
def encode(obj):
|
||||
return pickle.dumps(obj)
|
||||
@ -103,16 +110,10 @@ class PersistentDict(MutableMapping):
|
||||
if not self._dbpath:
|
||||
raise TypeError('Dict is not yet persistent')
|
||||
|
||||
connection = sqlite3.connect(self._dbpath)
|
||||
|
||||
try:
|
||||
with sqlite3.connect(self._dbpath) as connection:
|
||||
yield connection.cursor()
|
||||
|
||||
connection.commit()
|
||||
|
||||
finally:
|
||||
connection.close()
|
||||
|
||||
@_retry
|
||||
def __getitem__(self, key):
|
||||
key = self.encode(key)
|
||||
|
||||
@ -128,6 +129,7 @@ class PersistentDict(MutableMapping):
|
||||
|
||||
return self.decode(value[0])
|
||||
|
||||
@_retry
|
||||
def __setitem__(self, key, value):
|
||||
key = self.encode(key)
|
||||
value = self.encode(value)
|
||||
@ -138,23 +140,19 @@ class PersistentDict(MutableMapping):
|
||||
(key, value)
|
||||
)
|
||||
|
||||
@_retry
|
||||
def __delitem__(self, key):
|
||||
key = self.encode(key)
|
||||
|
||||
with self.connection() as cursor:
|
||||
cursor.execute(
|
||||
'select count(*) from cache where key=?',
|
||||
(key,)
|
||||
)
|
||||
|
||||
if cursor.fetchone()[0] == 0:
|
||||
raise KeyError(key)
|
||||
|
||||
cursor.execute(
|
||||
'delete from cache where key=?',
|
||||
(key,)
|
||||
)
|
||||
if not cursor.rowcount:
|
||||
raise KeyError(key)
|
||||
|
||||
@_retry
|
||||
def __iter__(self):
|
||||
with self.connection() as cursor:
|
||||
cursor.execute(
|
||||
@ -165,6 +163,7 @@ class PersistentDict(MutableMapping):
|
||||
for r in records:
|
||||
yield self.decode(r[0])
|
||||
|
||||
@_retry
|
||||
def __len__(self):
|
||||
with self.connection() as cursor:
|
||||
cursor.execute(
|
||||
|
||||
@ -12,6 +12,9 @@
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import pickle
|
||||
import sqlite3
|
||||
from unittest import mock
|
||||
|
||||
from oslotest import base
|
||||
@ -82,112 +85,145 @@ class MemoizeTestCase(base.BaseTestCase):
|
||||
self.assertEqual(0, driver.call_count)
|
||||
|
||||
|
||||
@mock.patch.object(sqlite3, 'connect', autospec=True)
|
||||
class PersistentDictTestCase(base.BaseTestCase):
|
||||
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test_make_permanent(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
|
||||
pd.make_permanent('/', 'file')
|
||||
mock_sqlite3.assert_called_once_with('/file.sqlite')
|
||||
|
||||
mock_sqlite3.connect.assert_called_with('/file.sqlite')
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
def test_encode(self, mock_pickle):
|
||||
def test_encode(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
value = pd.encode({1: '2'})
|
||||
self.assertEqual(pickle.dumps({1: '2'}), value)
|
||||
|
||||
pd.encode({1: '2'})
|
||||
|
||||
mock_pickle.dumps.assert_called_once_with({1: '2'})
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
def test_decode(self, mock_pickle):
|
||||
def test_decode(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
value = pd.decode(pickle.dumps({1: '2'}))
|
||||
self.assertEqual({1: '2'}, value)
|
||||
|
||||
pd.decode('blob')
|
||||
|
||||
mock_pickle.loads.assert_called_once_with('blob')
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test___getitem__(self, mock_sqlite3, mock_pickle):
|
||||
def test___getitem__(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_pickle.dumps.return_value = 'pickled-key'
|
||||
mock_connection = mock_sqlite3.connect.return_value
|
||||
mock_cursor = mock_connection.cursor.return_value
|
||||
mock_cursor.fetchone.return_value = ['pickled-value']
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.fetchone.return_value = [pickle.dumps('pickled-value')]
|
||||
|
||||
pd[1]
|
||||
result = pd[1]
|
||||
self.assertEqual('pickled-value', result)
|
||||
|
||||
mock_cursor.execute.assert_called_with(
|
||||
'select value from cache where key=?', ('pickled-key',))
|
||||
mock_pickle.loads.assert_called_once_with('pickled-value')
|
||||
'select value from cache where key=?', (pickle.dumps(1),))
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test___setitem__(self, mock_sqlite3, mock_pickle):
|
||||
def test___getitem__retries(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_pickle.dumps.side_effect = [
|
||||
'pickled-key', 'pickled-value']
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.execute.side_effect = [
|
||||
sqlite3.OperationalError,
|
||||
None
|
||||
]
|
||||
mock_cursor.fetchone.return_value = [pickle.dumps('pickled-value')]
|
||||
|
||||
mock_connection = mock_sqlite3.connect.return_value
|
||||
mock_cursor = mock_connection.cursor.return_value
|
||||
result = pd[1]
|
||||
self.assertEqual('pickled-value', result)
|
||||
|
||||
mock_cursor.execute.assert_called_with(
|
||||
'select value from cache where key=?', (pickle.dumps(1),))
|
||||
self.assertEqual(2, mock_cursor.execute.call_count)
|
||||
|
||||
def test___setitem__(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
|
||||
pd[1] = 2
|
||||
|
||||
mock_cursor.execute.assert_called_once_with(
|
||||
'insert or replace into cache values (?, ?)',
|
||||
(pickle.dumps(1), pickle.dumps(2)))
|
||||
|
||||
def test___setitem__retries(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.execute.side_effect = [
|
||||
sqlite3.OperationalError,
|
||||
None
|
||||
]
|
||||
|
||||
pd[1] = 2
|
||||
|
||||
mock_cursor.execute.assert_called_with(
|
||||
'insert or replace into cache values (?, ?)',
|
||||
('pickled-key', 'pickled-value'))
|
||||
(pickle.dumps(1), pickle.dumps(2)))
|
||||
self.assertEqual(2, mock_cursor.execute.call_count)
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test___delitem__(self, mock_sqlite3, mock_pickle):
|
||||
def test___delitem__(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_pickle.dumps.return_value = 'pickled-key'
|
||||
mock_connection = mock_sqlite3.connect.return_value
|
||||
mock_cursor = mock_connection.cursor.return_value
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
|
||||
del pd[1]
|
||||
|
||||
mock_cursor.execute.assert_called_with(
|
||||
'delete from cache where key=?', ('pickled-key',))
|
||||
mock_cursor.execute.assert_called_once_with(
|
||||
'delete from cache where key=?', (pickle.dumps(1),))
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test___iter__(self, mock_sqlite3, mock_pickle):
|
||||
def test___delitem__fails(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_pickle.dumps.return_value = 'pickled-key'
|
||||
mock_connection = mock_sqlite3.connect.return_value
|
||||
mock_cursor = mock_connection.cursor.return_value
|
||||
mock_cursor.fetchall.return_value = [['pickled-key']]
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.rowcount = 0
|
||||
|
||||
for x in pd:
|
||||
x += x
|
||||
def _del():
|
||||
del pd[1]
|
||||
|
||||
mock_cursor.execute.assert_called_with('select key from cache')
|
||||
mock_pickle.loads.assert_called_once_with('pickled-key')
|
||||
self.assertRaises(KeyError, _del)
|
||||
|
||||
@mock.patch.object(memoize, 'pickle', autospec=True)
|
||||
@mock.patch.object(memoize, 'sqlite3', autospec=True)
|
||||
def test___len__(self, mock_sqlite3, mock_pickle):
|
||||
mock_cursor.execute.assert_called_once_with(
|
||||
'delete from cache where key=?', (pickle.dumps(1),))
|
||||
|
||||
def test___iter__(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_connection = mock_sqlite3.connect.return_value
|
||||
mock_cursor = mock_connection.cursor.return_value
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.fetchall.return_value = [[pickle.dumps('pickled-key')]]
|
||||
|
||||
expected = 1
|
||||
self.assertEqual(['pickled-key'], list(iter(pd)))
|
||||
|
||||
mock_cursor.fetchone.return_value = [expected]
|
||||
mock_cursor.execute.assert_called_once_with('select key from cache')
|
||||
|
||||
self.assertEqual(expected, len(pd))
|
||||
def test___len__(self, mock_sqlite3):
|
||||
pd = memoize.PersistentDict()
|
||||
pd.make_permanent('/', 'file')
|
||||
|
||||
mock_cursor.execute.assert_called_with('select count(*) from cache')
|
||||
mock_conn = mock_sqlite3.return_value.__enter__.return_value
|
||||
mock_cursor = mock_conn.cursor.return_value
|
||||
mock_cursor.execute.reset_mock()
|
||||
mock_cursor.fetchone.return_value = [42]
|
||||
|
||||
self.assertEqual(42, len(pd))
|
||||
|
||||
mock_cursor.execute.assert_called_once_with(
|
||||
'select count(*) from cache')
|
||||
|
||||
Loading…
Reference in New Issue
Block a user