client: simplify previous commit since all loads are serialized

neo/client/app.py

@@ -18,7 +18,6 @@ from cPickle import dumps, loads
 from zlib import compress as real_compress, decompress
 from neo.lib.locking import Empty
 from random import shuffle
-from thread import get_ident
 import heapq
 import time
 import os
@@ -98,7 +97,7 @@ class Application(object):
         # no self-assigned UUID, primary master will supply us one
         self.uuid = None
         self._cache = ClientCache()
-        self._loading = {}
+        self._loading_oid = None
         self.new_oid_list = []
         self.last_oid = '\0' * 8
         self.storage_event_handler = storage.StorageEventHandler(self)
@@ -412,6 +411,10 @@ class Application(object):
 
         acquire = self._cache_lock_acquire
         release = self._cache_lock_release
+        # XXX: Is it possible this giant lock ?
+        #      See commit b77c946d67c9d7cc1e9ee9b15437568dee144aa4
+        #      for a way to invalidate cache properly when several loads
+        #      are done simultaneously.
         self._load_lock_acquire()
         try:
             acquire()
@@ -419,16 +422,14 @@ class Application(object):
                 result = self._loadFromCache(oid, tid, before_tid)
                 if result:
                     return result
-                loading_key = oid, get_ident()
-                self._loading[loading_key] = None
+                self._loading_oid = oid
+            finally:
                 release()
-                try:
-                    result = self._loadFromStorage(oid, tid, before_tid)
-                finally:
-                    acquire()
-                    invalidated = self._loading.pop(loading_key)
-                if invalidated and not result[2]:
-                    result = result[0], result[1], invalidated
+            result = self._loadFromStorage(oid, tid, before_tid)
+            acquire()
+            try:
+                if not (self._loading_oid or result[2]):
+                    result = result[0], result[1], self._loading_invalidated
                 self._cache.store(oid, *result)
                 return result
             finally:
@@ -782,7 +783,6 @@ class Application(object):
             self._cache_lock_acquire()
             try:
                 cache = self._cache
-                loading = self._loading
                 for oid, data in cache_dict.iteritems():
                     if data is CHECKED_SERIAL:
                         # this is just a remain of
@@ -793,9 +793,7 @@ class Application(object):
                     try:
                         cache.invalidate(oid, tid)
                     except KeyError:
-                        for k in loading:
-                            if k[0] == oid and not loading[k]:
-                                loading[k] = tid
+                        pass
                     if data is not None:
                         # Store in cache with no next_tid
                         cache.store(oid, data, tid, None)

neo/client/cache.py

@@ -238,6 +238,7 @@ def test(self):
     self.assertRaises(KeyError, cache.invalidate, 1, 10)
     data = 'foo', 5, 10
     # 2 identical stores happens if 2 threads got a cache miss at the same time
+    # (which currently never happens in NEO, due to a lock)
     cache.store(1, *data)
     cache.store(1, *data)
     self.assertEqual(cache.load(1, 10), data)

neo/client/handlers/master.py

@@ -113,14 +113,14 @@ class PrimaryNotificationsHandler(BaseHandler):
         app._cache_lock_acquire()
         try:
             invalidate = app._cache.invalidate
-            loading = app._loading
+            loading = app._loading_oid
             for oid in oid_list:
                 try:
                     invalidate(oid, tid)
                 except KeyError:
-                    for k in loading:
-                        if k[0] == oid and not loading[k]:
-                            loading[k] = tid
+                    if oid == loading:
+                        app._loading_oid = None
+                        app._loading_invalidated = tid
             db = app.getDB()
             if db is not None:
                 db.invalidate(tid, oid_list)

neo/tests/threaded/test.py

@@ -511,12 +511,6 @@ class Test(NEOThreadedTest):
                 l1.release()
                 l2.acquire()
             orig(conn, packet, kw, handler)
-        def _loadFromStorage(orig, *args):
-            try:
-                return orig(*args)
-            finally:
-                l1.release()
-                l2.acquire()
         cluster = NEOCluster()
         try:
             cluster.start()
@@ -537,26 +531,6 @@ class Test(NEOThreadedTest):
                 l2.release()
             t.join()
             self.assertEqual(x2.value, 1)
-
-            return # Following is disabled due to deadlock
-                   # caused by client load lock
-            t1.begin()
-            x1.value = 0
-            x2._p_deactivate()
-            cluster.client._cache.clear()
-            p = Patch(cluster.client, _loadFromStorage=_loadFromStorage)
-            try:
-                t = self.newThread(x2._p_activate)
-                l1.acquire()
-                t1.commit()
-                t1.begin()
-            finally:
-                del p
-                l2.release()
-            t.join()
-            x1._p_deactivate()
-            self.assertEqual(x2.value, 1)
-            self.assertEqual(x1.value, 0)
         finally:
             cluster.stop()