Support of journal_mode=WAL and synchronous=NORMAL in SQLite

[Jun Omae]

Currently, repository resync command is very slow with mirror of ​http://svn.edgewall.org/repos/trac on SQLite. Using ​Write-Ahead Logging feature would be faster. The feature is available since SQLite 3.7.0.

repository resync with mirror of ​http://svn.edgewall.org/repos/trac on SQLite:

journal_mode	synchronous	Elapsed
DELETE	FULL	37m44.746s	default
DELETE	NORMAL	30m02.647s
WAL	FULL	10m09.947s
WAL	NORMAL	2m39.104s

functional-test with SQLite:

journal_mode	synchronous	Elapsed
DELETE	FULL	261.468s	default
DELETE	NORMAL	235.012s
WAL	FULL	205.513s
WAL	NORMAL	177.595s

trac/db/sqlite_backend.py

@@ -207 +207 @@
         else:
             cnx = self.get_connection(path, log, params)
         cursor = cnx.cursor()
+
+        journal_mode = params.get('journal_mode', '').upper()
+        if journal_mode == 'WAL' and sqlite_version < (3, 7, 0):
+            journal_mode = None
+        if journal_mode in ('DELETE', 'TRUNCATE', 'PERSIST', 'MEMORY', 'WAL',
+                            'OFF'):
+            cursor.execute('PRAGMA journal_mode = %s' % journal_mode)
+
         if schema is None:
             from trac.db_default import schema
         for table in schema:
@@ -290 +298 @@
                 cnx.load_extension(ext)
             cnx.enable_load_extension(False)
 
+        synchronous = params.get('synchronous', '').upper()
+        if synchronous == 'NORMAL' and sqlite_version < (2, 8, 0):
+            synchronous = None
+        if synchronous in ('OFF', 'NORMAL'):
+            cursor = cnx.cursor()
+            cursor.execute('PRAGMA synchronous = %s' % synchronous)
+
         ConnectionWrapper.__init__(self, cnx, log)
 
     def cursor(self):

[Jun Omae]

Revised patch can be found in [fd2801610/jomae.git].

[Ryan J Ollos]

I see a conditional for sqlite_version < (2, 8, 0) which brings to mind a related question. Since ​Python 2.5 includes sqlite3, do we have a compelling reason to support SQLite < 3.0 on the 1.0-stable branch? We probably shouldn't drop support on 1.0-stable at this point, but maybe we could drop it on the trunk (which would be a topic for another ticket).

Let's discuss in #11969.

[Jun Omae]

Okay. That check has been removed in [66ec8afb/jomae.git].

According to ​https://hg.python.org/cpython/raw-file/v2.5/Misc/NEWS, at least SQLite version is 3.0.8 or later in Python 2.5.

• Added the sqlite3 package. This is based on pysqlite2.1.3, and provides a DB-API interface in the standard library. You'll need sqlite 3.0.8 or later to build this - if you have an earlier version, the C extension module will not be built.

[Ryan J Ollos]

On further review, we have a statement about SQLite 2.x not being supported: TracInstall#ForSQLite.

Do you think a reasonable approach is to support the version of PySqlite that is provided by the minimum supported version of Python?

• Python 2.5 has PySqlite 2.1.3
• Python 2.6 has PySqlite 2.4.1
• Python 2.7 has PySqlite 2.6.0

Later, with a review of supported platforms and the version of SQLite they provide, we could make more definitive statements about the supported version of SQLite. For now, we could just state SQLite ≥ 3.0.8.

[Ryan J Ollos]

Related: we should document supported parameters in DatabaseBackend#SQLite, like is done for MySQL.

[Ryan J Ollos]

Changes look good to me and I see the speedup when running unit tests with journal_mode=wal 70s → 63s.

Reviewing #11592 again, with your changes it would be nice if we could use the following patch:

trac/test.py

@@ -172 +172 @@
                 dburi += "&schema=tractest"
             else:
                 dburi += "?schema=tractest"
+        elif scheme == 'sqlite':
+            # Speed-up tests with SQLite database
+            dburi += ('&' if '?' in dburi else '?') + 'synchronous=off'
         return dburi
     return 'sqlite::memory:'
 
@@ -364 +367 @@
                 self.global_databasemanager.shutdown()
 
         with self.db_transaction as db:
-            if scheme == 'sqlite':
-                # Speed-up tests with SQLite database
-                db("PRAGMA synchronous = OFF")
             if default_data:
                 for table, cols, vals in db_default.get_data(db):
                     db.executemany("INSERT INTO %s (%s) VALUES (%s)"

That change moves us a bit closer to having all the db-specific code out of the EnvironmentStub class.

Except that synchronous=OFF is not allowed, which probably make sense for production instances. I'm not sure there is a way to resolve that, so maybe we just have to keep the db statement in EnvironmentStub.

[Jun Omae]

Updated jomae.git@t11967.

Reviewing #11592 again, with your changes it would be nice if we could use the following patch: […] That change moves us a bit closer to having all the db-specific code out of the EnvironmentStub class.

Sounds good if dburi doesn't have synchronous parameter. See [8e5aef34/jomae.git].

Except that synchronous=OFF is not allowed, which probably make sense for production instances. I'm not sure there is a way to resolve that, so maybe we just have to keep the db statement in EnvironmentStub.

Removed check of synchronous=OFF in [add02924/jomae.git]. I was confused with journal_mode=off which rollback command no longer works.

[Ryan J Ollos]

Replying to jomae:

Sounds good if dburi doesn't have synchronous parameter. See [8e5aef34/jomae.git].

We'll need not db_prop.get('params', {}).get('synchronous') since params is a nested dict.

It would probably be fine to push in milestone:1.1.5. I probably won't get to creating the release package until mid-week.

[Jun Omae]

Later, with a review of supported platforms and the version of SQLite they provide, we could make more definitive statements about the supported version of SQLite. For now, we could just state SQLite ≥ 3.0.8.

Latest PySqlite library also requires SQLite 3.0.8 or later. See ​https://github.com/ghaering/pysqlite/blob/master/doc/install-source.txt#L34.

However, I think we could recommend SQLite ≥ 3.3.8 and PySqlite ≥ 2.5.0 because connection pool requires the versions on SQLiteConnection.

257     poolable = have_pysqlite and sqlite_version >= (3, 3, 8) \
258                              and sqlite.version_info >= (2, 5, 0)

[Jun Omae]

Replying to rjollos:

We'll need not db_prop.get('params', {}).get('synchronous') since params is a nested dict.

Oh, I'm stupid. Thanks.

It would probably be fine to push in milestone:1.1.5. I probably won't get to creating the release package until mid-week.

I'm going to push it. BTW, milestone:1.0.5? If you're saying milestone:1.1.4, please retarget to 1.1.4.

[Ryan J Ollos]

Replying to jomae:

I'm going to push it. BTW, milestone:1.0.5? If you're saying milestone:1.1.4, please retarget to 1.1.4.

The changes look good and it seems to only enhance capabilities, which was my thinking towards milestone:1.0.5.

[Jun Omae]

Committed in [13816,13818] and merged to trunk in [13817,13819]. Also, updated wiki:DatabaseBackend@48 and wiki:DatabaseBackend@49.