Merge lp:~mvo/software-center/per-thread-xapiandb into lp:software-center

[Michael Vogt]

Hopefully kills off race condition behind #736116 (and maybe even #638706)

[lp:~mvo/software-center/per-thread-xapiandb]

2330. By Michael Vogt on 2011-09-14

softwarecenter/db/enquire.py: make sure we use uniq names

[Gary Lasker (gary-lasker)]

Hey mvo! This is a nice and very clever approach, and it certainly does appear to have have excellent potential for taking care of those bugs you mention (and as you know the second of the two, the DocNotFound master bug, is our current most evil one of all).

I did a bunch of testing and this does appear to maintain a unique database instance per each thread, exactly as intended. I could not detect a case where a given thread_name was changed to use a different database.

So, this one seems good to me! If this takes care of the DocNotFound errors, man, that will call for a serious celebration!

review: Approve

=== modified file 'softwarecenter/db/database.py'
--- softwarecenter/db/database.py	2011-08-17 12:24:39 +0000
+++ softwarecenter/db/database.py	2011-09-14 16:21:29 +0000
@@ -140,18 +140,63 @@
         # the xapian values as read from /var/lib/apt-xapian-index/values
         self._axi_values = {}
         self._logger = logging.getLogger("softwarecenter.db")
-
-    def acquire_search_lock(self):
-        self._search_lock.acquire()
-
-    def release_search_lock(self):
-        self._search_lock.release()
-
+        # we open one db per thread, thread names are reused eventually
+        # so no memory leak
+        self._db_per_thread = {} 
+        self._parser_per_thread = {} 
+
+    @property
+    def xapiandb(self):
+        """ returns a per thread db """
+        thread_name = threading.current_thread().name
+        if not thread_name in self._db_per_thread:
+            self._db_per_thread[thread_name] = self._get_new_xapiandb()
+        return self._db_per_thread[thread_name]
+
+    @property
+    def xapian_parser(self):
+        """ returns a per thread query parser """
+        thread_name = threading.current_thread().name
+        if not thread_name in self._parser_per_thread:
+            self._parser_per_thread[thread_name] = self._get_new_xapian_parser()
+        return self._parser_per_thread[thread_name]
+
+    def _get_new_xapiandb(self):
+        xapiandb = xapian.Database(self._db_pathname)
+        if self._use_axi:
+            try:
+                axi = xapian.Database("/var/lib/apt-xapian-index/index")
+                xapiandb.add_database(axi)
+            except:
+                self._logger.exception("failed to add apt-xapian-index")
+        if (self._use_agent and 
+            os.path.exists(XAPIAN_BASE_PATH_SOFTWARE_CENTER_AGENT)):
+            try:
+                sca = xapian.Database(XAPIAN_BASE_PATH_SOFTWARE_CENTER_AGENT)
+                xapiandb.add_database(sca)
+            except Exception as e:
+                logging.warn("failed to add sca db %s" % e)
+        for db in self._additional_databases:
+            xapiandb.add_database(db)
+        return xapiandb
+
+    def _get_new_xapian_parser(self):
+        xapian_parser = xapian.QueryParser()
+        xapian_parser.set_database(self.xapiandb)
+        xapian_parser.add_boolean_prefix("pkg", "XP")
+        xapian_parser.add_boolean_prefix("pkg", "AP")
+        xapian_parser.add_boolean_prefix("mime", "AM")
+        xapian_parser.add_boolean_prefix("section", "XS")
+        xapian_parser.add_boolean_prefix("origin", "XOC")
+        xapian_parser.add_prefix("pkg_wildcard", "XP")
+        xapian_parser.add_prefix("pkg_wildcard", "AP")
+        xapian_parser.set_default_op(xapian.Query.OP_AND)
+        return xapian_parser
+        
     def open(self, pathname=None, use_axi=True, use_agent=True):
         """ open the database """
         if pathname:
             self._db_pathname = pathname
-        self.xapiandb = xapian.Database(self._db_pathname)
         # add the apt-xapian-database for here (we don't do this
         # for now as we do not have a good way to integrate non-apps
         # with the UI)
@@ -159,35 +204,13 @@
         self._use_axi = use_axi
         self._use_agent = use_agent
         if use_axi:
-            try:
-                axi = xapian.Database("/var/lib/apt-xapian-index/index")
-                self.xapiandb.add_database(axi)
-                self._axi_values = parse_axi_values_file()
-                self.nr_databases += 1
-            except:
-                self._logger.exception("failed to add apt-xapian-index")
+            self._axi_values = parse_axi_values_file()
+            self.nr_databases += 1
         if use_agent:
-            try:
-                sca = xapian.Database(XAPIAN_BASE_PATH_SOFTWARE_CENTER_AGENT)
-                self.xapiandb.add_database(sca)
-                self.nr_databases += 1
-            except Exception as e:
-                logging.warn("failed to add sca db %s" % e)
+            self.nr_databases += 1
         # additional dbs
         for db in self._additional_databases:
-            self.xapiandb.add_database(db)
             self.nr_databases += 1
-        # parser etc
-        self.xapian_parser = xapian.QueryParser()
-        self.xapian_parser.set_database(self.xapiandb)
-        self.xapian_parser.add_boolean_prefix("pkg", "XP")
-        self.xapian_parser.add_boolean_prefix("pkg", "AP")
-        self.xapian_parser.add_boolean_prefix("mime", "AM")
-        self.xapian_parser.add_boolean_prefix("section", "XS")
-        self.xapian_parser.add_boolean_prefix("origin", "XOC")
-        self.xapian_parser.add_prefix("pkg_wildcard", "XP")
-        self.xapian_parser.add_prefix("pkg_wildcard", "AP")
-        self.xapian_parser.set_default_op(xapian.Query.OP_AND)
         self.emit("open", self._db_pathname)
 
     def add_database(self, database):

=== modified file 'softwarecenter/db/enquire.py'
--- softwarecenter/db/enquire.py	2011-08-25 18:51:39 +0000
+++ softwarecenter/db/enquire.py	2011-09-14 16:21:29 +0000
@@ -85,7 +85,14 @@
 
     def _threaded_perform_search(self):
         self._perform_search_complete = False
-        thread_name = 'ThreadedQuery-%s' % (threading.active_count()+1)
+        # generate a name and ensure we never have two threads
+        # with the same name
+        names = [thread.name for thread in threading.enumerate()]
+        for i in range(threading.active_count()+1, 0, -1):
+            thread_name = 'ThreadedQuery-%s' % i
+            if not thread_name in names:
+                break
+        # create and start it
         t = threading.Thread(
             target=self._blocking_perform_search, name=thread_name)
         t.start()
@@ -128,9 +135,6 @@
         # use a unique instance of both enquire and xapian database
         # so concurrent queries dont result in an inconsistent database
 
-        # get lock
-        self.db.acquire_search_lock()
-
         # an alternative would be to serialise queries
         enquire = xapian.Enquire(self.db.xapiandb)
 
@@ -223,10 +227,6 @@
                         _matches.append(match)
                         match_docids.add(match.docid)
 
-        # release the lock here because the following check may trigger
-        # calling this function again (and we would deadlock otherwise)
-        self.db.release_search_lock()
-
         # if we have no results, try forcing pkgs to be displayed
         # if not NonAppVisibility.NEVER_VISIBLE is set
         if (not _matches and