Merge "Make memoize behave better in concurrent environments"

lower-constraints.txt

@@ -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

requirements.txt

@@ -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

sushy_tools/emulator/memoize.py

@@ -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)
+        with sqlite3.connect(self._dbpath) as connection:
-
-        try:
             yield connection.cursor()
 
-            connection.commit()
+    @_retry
-
-        finally:
-            connection.close()
-
     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(

sushy_tools/tests/unit/emulator/test_memoize.py

@@ -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')
+    def test_encode(self, mock_sqlite3):
-
-    @mock.patch.object(memoize, 'pickle', autospec=True)
-    def test_encode(self, mock_pickle):
         pd = memoize.PersistentDict()
+        value = pd.encode({1: '2'})
+        self.assertEqual(pickle.dumps({1: '2'}), value)
 
-        pd.encode({1: '2'})
+    def test_decode(self, mock_sqlite3):
-
-        mock_pickle.dumps.assert_called_once_with({1: '2'})
-
-    @mock.patch.object(memoize, 'pickle', autospec=True)
-    def test_decode(self, mock_pickle):
         pd = memoize.PersistentDict()
+        value = pd.decode(pickle.dumps({1: '2'}))
+        self.assertEqual({1: '2'}, value)
 
-        pd.decode('blob')
+    def test___getitem__(self, mock_sqlite3):
-
-        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):
         pd = memoize.PersistentDict()
         pd.make_permanent('/', 'file')
 
-        mock_pickle.dumps.return_value = 'pickled-key'
+        mock_conn = mock_sqlite3.return_value.__enter__.return_value
-        mock_connection = mock_sqlite3.connect.return_value
+        mock_cursor = mock_conn.cursor.return_value
-        mock_cursor = mock_connection.cursor.return_value
+        mock_cursor.execute.reset_mock()
-        mock_cursor.fetchone.return_value = ['pickled-value']
+        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',))
+            'select value from cache where key=?', (pickle.dumps(1),))
-        mock_pickle.loads.assert_called_once_with('pickled-value')
 
-    @mock.patch.object(memoize, 'pickle', autospec=True)
+    def test___getitem__retries(self, mock_sqlite3):
-    @mock.patch.object(memoize, 'sqlite3', autospec=True)
-    def test___setitem__(self, mock_sqlite3, mock_pickle):
         pd = memoize.PersistentDict()
         pd.make_permanent('/', 'file')
 
-        mock_pickle.dumps.side_effect = [
+        mock_conn = mock_sqlite3.return_value.__enter__.return_value
-            'pickled-key', 'pickled-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
+        result = pd[1]
-        mock_cursor = mock_connection.cursor.return_value
+        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)
+    def test___delitem__(self, mock_sqlite3):
-    @mock.patch.object(memoize, 'sqlite3', autospec=True)
-    def test___delitem__(self, mock_sqlite3, mock_pickle):
         pd = memoize.PersistentDict()
         pd.make_permanent('/', 'file')
 
-        mock_pickle.dumps.return_value = 'pickled-key'
+        mock_conn = mock_sqlite3.return_value.__enter__.return_value
-        mock_connection = mock_sqlite3.connect.return_value
+        mock_cursor = mock_conn.cursor.return_value
-        mock_cursor = mock_connection.cursor.return_value
+        mock_cursor.execute.reset_mock()
 
         del pd[1]
 
-        mock_cursor.execute.assert_called_with(
+        mock_cursor.execute.assert_called_once_with(
-            'delete from cache where key=?', ('pickled-key',))
+            'delete from cache where key=?', (pickle.dumps(1),))
 
-    @mock.patch.object(memoize, 'pickle', autospec=True)
+    def test___delitem__fails(self, mock_sqlite3):
-    @mock.patch.object(memoize, 'sqlite3', autospec=True)
-    def test___iter__(self, mock_sqlite3, mock_pickle):
         pd = memoize.PersistentDict()
         pd.make_permanent('/', 'file')
 
-        mock_pickle.dumps.return_value = 'pickled-key'
+        mock_conn = mock_sqlite3.return_value.__enter__.return_value
-        mock_connection = mock_sqlite3.connect.return_value
+        mock_cursor = mock_conn.cursor.return_value
-        mock_cursor = mock_connection.cursor.return_value
+        mock_cursor.execute.reset_mock()
-        mock_cursor.fetchall.return_value = [['pickled-key']]
+        mock_cursor.rowcount = 0
 
-        for x in pd:
+        def _del():
-            x += x
+            del pd[1]
 
-        mock_cursor.execute.assert_called_with('select key from cache')
+        self.assertRaises(KeyError, _del)
-        mock_pickle.loads.assert_called_once_with('pickled-key')
 
-    @mock.patch.object(memoize, 'pickle', autospec=True)
+        mock_cursor.execute.assert_called_once_with(
-    @mock.patch.object(memoize, 'sqlite3', autospec=True)
+            'delete from cache where key=?', (pickle.dumps(1),))
-    def test___len__(self, mock_sqlite3, mock_pickle):
+
+    def test___iter__(self, mock_sqlite3):
         pd = memoize.PersistentDict()
         pd.make_permanent('/', 'file')
 
-        mock_connection = mock_sqlite3.connect.return_value
+        mock_conn = mock_sqlite3.return_value.__enter__.return_value
-        mock_cursor = mock_connection.cursor.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')