[PATCH 7/9] Don't keep sqlite connections open

Sebastian Spaeth Sebastian at SSpaeth.de
Tue Apr 26 11:31:38 BST 2011


Due to non-threadsafeness, we need to close the sqlite connection after
we used it. It would be nicer to keep a cursor open, but apparently the
python bindings to sqlite don't allow this as early versions of sqlite
could not deal with several threads accessing an sqlite object.

Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
---
 offlineimap/folder/LocalStatusSQLite.py |  175 +++++++++++++++++++------------
 1 files changed, 108 insertions(+), 67 deletions(-)

diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py
index 9f7a49d..9329829 100644
--- a/offlineimap/folder/LocalStatusSQLite.py
+++ b/offlineimap/folder/LocalStatusSQLite.py
@@ -24,7 +24,13 @@ except:
     pass #fail only when needed later on
 
 class LocalStatusSQLiteFolder(LocalStatusFolder):
-    """LocalStatus backend implemented with an SQLite database"""
+    """LocalStatus backend implemented with an SQLite database
+
+    As python-sqlite currently does not allow to access the same sqlite
+    objects from various threads, we need to open get and close a db
+    connection and cursor for all operations. This is a big disadvantage
+    and we might want to investigate if we cannot hold an object open
+    for a thread somehow."""
     def __deinit__(self):
         #TODO, need to invoke this when appropriate?
         self.save()
@@ -34,29 +40,37 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
     def __init__(self, root, name, repository, accountname, config):
         super(LocalStatusSQLiteFolder, self).__init__(root, name, repository, accountname, config)
         
-        self.dblock = RLock()
-        """dblock protects against concurrent forbidden access of the db
-        object, e.g trying to test for existence and on-demand creation
-        of the db."""
-
-        #Try to establish connection
+        #Try to establish connection, no need for threadsafety in __init__
         try:
-            self.connection = sqlite.connect(self.filename)
+            connection = sqlite.connect(self.filename)
         except NameError:
             # sqlite import had failed
             raise UserWarning('SQLite backend chosen, but no sqlite python '
                               'bindings available. Please install.')
 
         #Test if the db version is current enough and if the db is
-        #readable.  Lock, so that only one thread at a time can do this,
-        #so we don't create them in parallel.
-        with self.dblock:
-            try:
-                self.cursor = self.connection.cursor()
-                self.cursor.execute('SELECT version from metadata')
-            except sqlite.DatabaseError:
-                #db file missing or corrupt, recreate it.
-                self.create_db()
+        #readable.
+        try:
+            cursor = connection.cursor()
+            cursor.execute("SELECT value from metadata WHERE key='db_version'")
+        except sqlite.DatabaseError as e:
+            #db file missing or corrupt, recreate it.
+            connection.close()
+            self.create_db()
+        connection.close()
+
+    def get_cursor(self):
+        """Return a db (connection, cursor) that we can use
+
+        You are responsible for connection.commit() and
+        connection.close() yourself. Connection close() happens
+        automatically when the connection variable is destroyed
+        though. According to sqlite docs, you need to commit() before
+        the connection is closed or your changes will be lost!"""
+        #get db connection which autocommits
+        connection = sqlite.connect(self.filename, isolation_level=None)        
+        cursor = connection.cursor()
+        return connection, cursor
 
     def isnewfolder(self):
         # testing the existence of the db file won't work. It is created
@@ -68,48 +82,71 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
         """Create a new db file"""
         self.ui.warn('Creating new Local Status db for %s:%s' \
                          % (self.repository.getname(), self.getname()))
-        self.connection = sqlite.connect(self.filename)
-        self.cursor = self.connection.cursor()
-        self.cursor.execute('CREATE TABLE metadata (key VARCHAR(50) PRIMARY KEY, value VARCHAR(128))')
-        self.cursor.execute("INSERT INTO metadata VALUES('db_version', '1')")
-        self.cursor.execute('CREATE TABLE status (id INTEGER PRIMARY KEY, flags VARCHAR(50))')
-        self.save() #commit if needed
+        conn, cursor = self.get_cursor()
+        with conn:
+            cursor.execute('CREATE TABLE metadata (key VARCHAR(50) PRIMARY KEY, value VARCHAR(128))')
+            cursor.execute("INSERT INTO metadata VALUES('db_version', '1')")
+            cursor.execute('CREATE TABLE status (id INTEGER PRIMARY KEY, flags VARCHAR(50))')
+            conn.commit()
 
     def deletemessagelist(self):
         """delete all messages in the db"""
-        self.cursor.execute('DELETE FROM status')
+        conn, cursor = self.get_cursor()
+        with conn:
+            cursor.execute('DELETE FROM status')
+            conn.commit()
 
     def cachemessagelist(self):
         self.messagelist = {}
-        self.cursor.execute('SELECT id,flags from status')
-        for row in self.cursor:
-            flags = [x for x in row[1]]
-            self.messagelist[row[0]] = {'uid': row[0], 'flags': flags}
+        conn, cursor = self.get_cursor()
+        with conn:
+            cursor.execute('SELECT id,flags from status')
+            for row in cursor:
+                flags = [x for x in row[1]]
+                self.messagelist[row[0]] = {'uid': row[0], 'flags': flags}
 
     def save(self):
-        self.connection.commit()
+        """Noop in this backend"""
+        pass
 
     def getmessagelist(self):
         return self.messagelist
 
-    def uidexists(self,uid):
-        self.cursor.execute('SELECT id FROM status WHERE id=:id',{'id': uid})
-        for row in self.cursor:
-            if(row[0]==uid):
-                return 1
-        return 0
-
-    def getmessageuidlist(self):
-        self.cursor.execute('SELECT id from status')
-        r = []
-        for row in self.cursor:
-            r.append(row[0])
-        return r
-
-    def getmessagecount(self):
-        self.cursor.execute('SELECT count(id) from status');
-        row = self.cursor.fetchone()
-        return row[0]
+    # Following some pure SQLite functions, where we chose to use
+    # BaseFolder() methods instead. Doing those on the in-memory list is
+    # quicker anyway. If our db becomes so big that we don't want to
+    # maintain the in-memory list anymore, these might come in handy
+    # though.
+    #
+    #def uidexists(self,uid):
+    #    conn, cursor = self.get_cursor()
+    #    with conn:
+    #        cursor.execute('SELECT id FROM status WHERE id=:id',{'id': uid})
+    #        return cursor.fetchone()
+    # This would be the pure SQLite solution, use BaseFolder() method,
+    # to avoid threading with sqlite...
+    #def getmessageuidlist(self):
+    #    conn, cursor = self.get_cursor()
+    #    with conn:
+    #        cursor.execute('SELECT id from status')
+    #        r = []
+    #        for row in cursor:
+    #            r.append(row[0])
+    #        return r
+    #def getmessagecount(self):
+    #    conn, cursor = self.get_cursor()
+    #    with conn:
+    #        cursor.execute('SELECT count(id) from status');
+    #        return cursor.fetchone()[0]
+    #def getmessageflags(self, uid):
+    #    conn, cursor = self.get_cursor()
+    #    with conn:
+    #        cursor.execute('SELECT flags FROM status WHERE id=:id',
+    #                        {'id': uid})
+    #        for row in cursor:
+    #            flags = [x for x in row[0]]
+    #            return flags
+    #        assert False,"getmessageflags() called on non-existing message"
 
     def savemessage(self, uid, content, flags, rtime):
         if uid < 0:
@@ -123,28 +160,29 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
         self.messagelist[uid] = {'uid': uid, 'flags': flags, 'time': rtime}
         flags.sort()
         flags = ''.join(flags)
-        self.cursor.execute('INSERT INTO status (id,flags) VALUES (?,?)',
-                            (uid,flags))
-        self.save()
+        conn, cursor = self.get_cursor()
+        while True:
+            try:
+                cursor.execute('INSERT INTO status (id,flags) VALUES (?,?)',
+                               (uid,flags))
+            except sqlite.OperationalError as e:
+                if e.args[0] != 'database is locked':
+                    raise
+            else: #success
+                break    
+        conn.commit()
+        cursor.close()
+        conn.close()
         return uid
 
-    def getmessageflags(self, uid):
-        self.cursor.execute('SELECT flags FROM status WHERE id=:id',
-                            {'id': uid})
-        for row in self.cursor:
-            flags = [x for x in row[0]]
-            return flags
-        assert False,"getmessageflags() called on non-existing message"
-
-    def getmessagetime(self, uid):
-        return self.messagelist[uid]['time']
-
     def savemessageflags(self, uid, flags):
         self.messagelist[uid] = {'uid': uid, 'flags': flags}
         flags.sort()
         flags = ''.join(flags)
-        self.cursor.execute('UPDATE status SET flags=? WHERE id=?',(flags,uid))
-        self.save()
+        conn, cursor = self.get_cursor()
+        with conn:
+            cursor.execute('UPDATE status SET flags=? WHERE id=?',(flags,uid))
+            conn.commit()
 
     def deletemessages(self, uidlist):
         # Weed out ones not in self.messagelist
@@ -152,7 +190,10 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
         if not len(uidlist):
             return
 
-        for uid in uidlist:
-            del(self.messagelist[uid])
-            #if self.uidexists(uid):
-            self.cursor.execute('DELETE FROM status WHERE id=:id', {'id': uid})
+        conn, cursor = self.get_cursor()
+        with conn:
+            for uid in uidlist:
+                del(self.messagelist[uid])
+                cursor.execute('DELETE FROM status WHERE id=:id',
+                               {'id': uid})
+            conn.commit()
-- 
1.7.4.1





More information about the OfflineIMAP-project mailing list