Merge lp:~mkanat/loggerhead/codebrowse-hangs into lp:loggerhead

[Max Kanat-Alexander]

[Max Kanat-Alexander (mkanat)]

This most likely fixes Bug 118625, though I wanted to get at least a code review before having the LOSAs deploy it.

[lp:~mkanat/loggerhead/codebrowse-hangs]

401. By Max Kanat-Alexander on 2010-01-14

Address code review comments from mwhudson and Peng.

[Michael Hudson-Doyle (mwhudson)]

Looks good to me.

review: Approve

[lp:~mkanat/loggerhead/codebrowse-hangs]

402. By Max Kanat-Alexander on 2010-01-14

Be super-paranoid about releasing the revision_graph_check_lock.

[Matt Nordhoff (mnordhoff)]

I'll land it.

review: Approve

=== modified file 'loggerhead/history.py'
--- loggerhead/history.py	2010-01-11 03:26:44 +0000
+++ loggerhead/history.py	2010-01-14 00:17:10 +0000
@@ -33,6 +33,7 @@
 import logging
 import re
 import textwrap
+import threading
 
 import bzrlib.branch
 import bzrlib.delta
@@ -188,6 +189,12 @@
         """
         self._cache[key] = (revid, data)
 
+# Used to store locks that prevent multiple threads from building a 
+# revision graph for the same branch at the same time, because that can
+# cause severe performance issues that are so bad that the system seems
+# to hang.
+revision_graph_locks = {}
+revision_graph_check_lock = threading.Lock()
 
 class History(object):
     """Decorate a branch to provide information for rendering.
@@ -227,18 +234,33 @@
         def update_missed_caches():
             for cache in missed_caches:
                 cache.set(cache_key, self.last_revid, self._rev_info)
-        for cache in caches:
-            data = cache.get(cache_key, self.last_revid)
-            if data is not None:
-                self._rev_info = data
+
+        # Theoretically, it's possible for two threads to race in creating
+        # the Lock() object for their branch, so we put a lock around
+        # creating the per-branch Lock().
+        revision_graph_check_lock.acquire()
+        try:
+            if cache_key not in revision_graph_locks:
+                revision_graph_locks[cache_key] = threading.Lock()
+        finally:
+            revision_graph_check_lock.release()
+
+        revision_graph_locks[cache_key].acquire()
+        try:
+            for cache in caches:
+                data = cache.get(cache_key, self.last_revid)
+                if data is not None:
+                    self._rev_info = data
+                    update_missed_caches()
+                    break
+                else:
+                    missed_caches.append(cache)
+            else:
+                whole_history_data = compute_whole_history_data(self._branch)
+                self._rev_info, self._rev_indices = whole_history_data
                 update_missed_caches()
-                break
-            else:
-                missed_caches.append(cache)
-        else:
-            whole_history_data = compute_whole_history_data(self._branch)
-            self._rev_info, self._rev_indices = whole_history_data
-            update_missed_caches()
+        finally:
+            revision_graph_locks[cache_key].release()
 
         if self._rev_indices is not None:
             self._revno_revid = {}