Edgewall Software
Modify

Opened 4 years ago

Closed 4 years ago

Last modified 7 months ago

#11967 closed defect (fixed)

Support of journal_mode=WAL and synchronous=NORMAL in SQLite

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.0.5
Component: database backend Version:
Severity: normal Keywords: sqlite performance
Cc:
Release Notes:

Add support journal_mode and synchronous pragmas in sqlite: database connection string.

API Changes:

Description

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

    diff --git a/trac/db/sqlite_backend.py b/trac/db/sqlite_backend.py
    index 7b9b139..992e214 100644
    a b class SQLiteConnector(Component):  
    207207        else:
    208208            cnx = self.get_connection(path, log, params)
    209209        cursor = cnx.cursor()
     210
     211        journal_mode = params.get('journal_mode', '').upper()
     212        if journal_mode == 'WAL' and sqlite_version < (3, 7, 0):
     213            journal_mode = None
     214        if journal_mode in ('DELETE', 'TRUNCATE', 'PERSIST', 'MEMORY', 'WAL',
     215                            'OFF'):
     216            cursor.execute('PRAGMA journal_mode = %s' % journal_mode)
     217
    210218        if schema is None:
    211219            from trac.db_default import schema
    212220        for table in schema:
    class SQLiteConnection(ConnectionWrapper):  
    290298                cnx.load_extension(ext)
    291299            cnx.enable_load_extension(False)
    292300
     301        synchronous = params.get('synchronous', '').upper()
     302        if synchronous == 'NORMAL' and sqlite_version < (2, 8, 0):
     303            synchronous = None
     304        if synchronous in ('OFF', 'NORMAL'):
     305            cursor = cnx.cursor()
     306            cursor.execute('PRAGMA synchronous = %s' % synchronous)
     307
    293308        ConnectionWrapper.__init__(self, cnx, log)
    294309
    295310    def cursor(self):

Attachments (0)

Change History (13)

comment:1 Changed 4 years ago by Jun Omae

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

comment:2 Changed 4 years ago by 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.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:3 Changed 4 years ago by 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.

comment:4 Changed 4 years ago by 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?

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.

comment:5 Changed 4 years ago by Ryan J Ollos

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

comment:6 Changed 4 years ago by 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

    diff --git a/trac/test.py b/trac/test.py
    index 4f30161..75b3327 100755
    a b def get_dburi():  
    172172                dburi += "&schema=tractest"
    173173            else:
    174174                dburi += "?schema=tractest"
     175        elif scheme == 'sqlite':
     176            # Speed-up tests with SQLite database
     177            dburi += ('&' if '?' in dburi else '?') + 'synchronous=off'
    175178        return dburi
    176179    return 'sqlite::memory:'
    177180
    class EnvironmentStub(Environment):  
    364367                self.global_databasemanager.shutdown()
    365368
    366369        with self.db_transaction as db:
    367             if scheme == 'sqlite':
    368                 # Speed-up tests with SQLite database
    369                 db("PRAGMA synchronous = OFF")
    370370            if default_data:
    371371                for table, cols, vals in db_default.get_data(db):
    372372                    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.

comment:7 in reply to:  6 ; Changed 4 years ago by 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.

comment:8 in reply to:  7 ; Changed 4 years ago by 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.

comment:9 in reply to:  4 Changed 4 years ago by 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)

comment:10 in reply to:  8 ; Changed 4 years ago by Jun Omae

Milestone: next-stable-1.0.x1.0.5
Owner: set to Jun Omae
Status: newassigned

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.

comment:11 in reply to:  10 Changed 4 years ago by 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.

comment:12 Changed 4 years ago by Jun Omae

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

Last edited 4 years ago by Jun Omae (previous) (diff)

comment:13 Changed 7 months ago by anonymous

Keywords: performance added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.