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

Sebastian Spaeth Sebastian at SSpaeth.de
Thu May 5 14:59:27 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 |  199 ++++++++++++++++++++-----------
 1 files changed, 127 insertions(+), 72 deletions(-)

diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py
index c8c179f..dbfad54 100644
--- a/offlineimap/folder/LocalStatusSQLite.py
+++ b/offlineimap/folder/LocalStatusSQLite.py
@@ -23,7 +23,13 @@ except:
     pass #fail only if needed later on, not on import
 
 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."""
     #current version of the db format
     cur_version = 1
 
@@ -32,9 +38,9 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
                                                       repository, 
                                                       accountname,
                                                       config)       
-        #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 '
@@ -43,19 +49,31 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
         #Test if the db version is current enough and if the db is
         #readable.
         try:
-            self.cursor = self.connection.cursor()
-            self.cursor.execute("SELECT value from metadata WHERE key='db_version'")
+            cursor = connection.cursor()
+            cursor.execute("SELECT value from metadata WHERE key='db_version'")
         except sqlite.DatabaseError:
             #db file missing or corrupt, recreate it.
-            self.connection.close()
+            connection.close()
             self.upgrade_db(0)
         else:
             # fetch db version and upgrade if needed
-            version = int(self.cursor.fetchone()[0])
-            self.cursor.close()
+            version = int(cursor.fetchone()[0])
+            connection.close()
             if version < LocalStatusSQLiteFolder.cur_version:
                 self.upgrade_db(version)
-            self.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 upgrade_db(self, from_ver):
         """Upgrade the sqlite format from version 'from_ver' to current"""
@@ -75,21 +93,20 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
                 file = open(plaintextfilename, "rt")
                 line = file.readline().strip()
                 assert(line == magicline)
-                connection = sqlite.connect(self.filename)
-                cursor = connection.cursor()
-                for line in file.xreadlines():
-                    line = line.strip()
-                    uid, flags = line.split(':')
-                    uid = long(uid)
-                    flags = [x for x in flags]
-                    flags.sort()
-                    flags = ''.join(flags)
-                    self.cursor.execute('INSERT INTO status (id,flags) VALUES (?,?)',
+                conn, cursor = self.get_cursor()
+                with conn:
+                    for line in file.xreadlines():
+                        line = line.strip()
+                        uid, flags = line.split(':')
+                        uid = long(uid)
+                        flags = [x for x in flags]
+                        flags.sort()
+                        flags = ''.join(flags)
+                        cursor.execute('INSERT INTO status (id,flags) VALUES (?,?)',
                                         (uid,flags))
+                    conn.commit()
                 file.close()
-                self.connection.commit()
                 os.rename(plaintextfilename, plaintextfilename + ".old")
-                self.connection.close()
         # Future version upgrades come here...
         # if from_ver <= 1: ... #upgrade from 1 to 2
         # if from_ver <= 2: ... #upgrade from 2 to 3
@@ -98,12 +115,12 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
         """Create a new db file"""
         self.ui._msg('Creating new Local Status db for %s:%s' \
                          % (self.repository, self))
-        connection = sqlite.connect(self.filename)
-        cursor = connection.cursor()
-        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))')
-        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 isnewfolder(self):
         # testing the existence of the db file won't work. It is created
@@ -113,37 +130,59 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
 
     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):
         #Noop in this backend
         pass
 
-    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
+    # in the future 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:
@@ -155,30 +194,36 @@ class LocalStatusSQLiteFolder(LocalStatusFolder):
             return uid
 
         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()
+        flags = ''.join(sorted(flags))
+        conn, cursor = self.get_cursor()
+        with conn:
+            while True:
+                try:
+                    cursor.execute('INSERT INTO status (id,flags) VALUES (?,?)',
+                               (uid,flags))
+                    conn.commit()
+                except sqlite.OperationalError as e:
+                    if e.args[0] != 'database is locked':
+                        raise
+                else: #success
+                    break
         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:
+            while True:
+                try:
+                    cursor.execute('UPDATE status SET flags=? WHERE id=?',(flags,uid))
+                    conn.commit()
+                except sqlite.OperationalError as e:
+                    if e.args[0] != 'database is locked':
+                        raise
+                else: #success
+                    break
 
     def deletemessages(self, uidlist):
         # Weed out ones not in self.messagelist
@@ -186,7 +231,17 @@ 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])
+                while True:
+                    try:
+                        cursor.execute('DELETE FROM status WHERE id=?',
+                               uid)
+                        conn.commit()
+                    except sqlite.OperationalError as e:
+                        if e.args[0] != 'database is locked':
+                            raise
+                    else: #success
+                        break
-- 
1.7.4.1





More information about the OfflineIMAP-project mailing list