Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServerDB, Meta: add support for SQLite WAL. #2794

Merged
merged 1 commit into from Mar 5, 2017

Conversation

@mkrautz
Copy link
Member

commented Jan 30, 2017

Using SQLite's WAL (write-ahead log) can create less disk I/O while
still providing good consistency and durability.

This change uses SQLite's WAL with synchronous=NORMAL which can
can cause loss of transactions on power failure. Only the transactions
which haven't been synced to the disk by the OS are lost. The database
itself will still be in a consistent state, but it might not have all
recent changes.

@mkrautz mkrautz force-pushed the mkrautz:sqlite-wal branch from 37da88a to c8d22a4 Jan 30, 2017

@mkrautz mkrautz force-pushed the mkrautz:sqlite-wal branch from c8d22a4 to 0335c79 Mar 5, 2017

@mkrautz mkrautz changed the title WIP: ServerDB, Meta: add support for SQLite WAL. ServerDB, Meta: add support for SQLite WAL. Mar 5, 2017

@mkrautz mkrautz requested review from Kissaki, hacst and davidebeatrici Mar 5, 2017

@mkrautz mkrautz force-pushed the mkrautz:sqlite-wal branch from 0335c79 to b737992 Mar 5, 2017

@Kissaki

This comment has been minimized.

Copy link
Member

commented Mar 5, 2017

Typo in commit message "can can cause loss".

; If Murmur crashes, the database will be in a consistent state, but
; the most recent changes might be lost, if the operating system did
; not write them to disk yet. This option can improve Murmur's
; interactivity busy servers, or servers with slow storage.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Author Member

on busy servers

@@ -15,6 +15,25 @@
; murmur.sqlite in default locations or create it if not found.
database=

; Murmur defaults to using SQLite with its default rollback journal.
; In some situations, using SQLite's write-ahead log can be advantageous.

This comment has been minimized.

Copy link
@Kissaki

Kissaki Mar 5, 2017

Member

This may fuck up the formatting, but explicitly mentioning the WAL acronym when using "write-ahead log" could make the acronym usage below more intuitively understandable.


QStringList splitVersion = version.split(QLatin1String("."));
int major = 0, minor = 0;
if (splitVersion.count() >= 3) {

This comment has been minimized.

Copy link
@Kissaki

Kissaki Mar 5, 2017

Member

Check for the >4 case (unexpected versioning change in the future)? Why not check for ==3? The expectation here is that if the version changes to additional sub-parts the major and minors stay valid?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Author Member

I believe some SQLite releases have used 4 components.

https://www.sqlite.org/versionnumbers.html

Beginning with version 3.9.0 (2015-10-14) SQLite uses semantic versioning. Prior to that time, SQLite employed a version identifier that contained between two and four numbers.

and

This historical version numbering system used a two-, three-, or four-number version: W.X, W.X.Y, or W.X.Y.Z. W was the file format: 1 or 2 or 3. X was the major version. Y was the minor version. Z was used only for patch releases to fix bugs.

} else if (Meta::mp.iSQLiteWAL == 2) {
SQLDO("PRAGMA synchronous=FULL;");
}
qWarning("ServerDB: Configured SQLite for journal_mode=WAL, synchronous=NORMAL");

This comment has been minimized.

Copy link
@Kissaki

Kissaki Mar 5, 2017

Member

This logs NORMAL for both cases, even when it is not NORMAL.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Author Member

Fixed.

}
qWarning("ServerDB: Configured SQLite for journal_mode=WAL, synchronous=NORMAL");
} else {
qWarning("ServerDB: Unable to use SQLite in WAL mode due to incompatible SQLite version %i.%i", major, minor);

This comment has been minimized.

Copy link
@Kissaki

Kissaki Mar 5, 2017

Member

This will log version 0.0 if something with that went wrong. If both are 0, maybe log a warning on determining the version instead?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Author Member

Fixed.

qWarning("ServerDB: Unable to use SQLite in WAL mode due to incompatible SQLite version %i.%i", major, minor);
}
} else {
qFatal("ServerDB: Invalid value '%i' for sqlite_wal. Please use 0 (no wal), 1 (wal), 2 (wal with synchronous=full)");

This comment has been minimized.

Copy link
@Kissaki

Kissaki Mar 5, 2017

Member

We check for >0 in the if case, so 0 (the default) is an error case now? That should be the ignore-case, right?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Author Member

Fixed.

@mkrautz mkrautz force-pushed the mkrautz:sqlite-wal branch 2 times, most recently from 3ce6fb8 to 2f0e5d6 Mar 5, 2017

ServerDB, Meta: add support for SQLite WAL.
Using SQLite's WAL (write-ahead log) can create less disk I/O while
still providing good consistency and durability.

This change uses SQLite's WAL with synchronous=NORMAL which can
cause loss of transactions on power failure. Only the transactions
which haven't been synced to the disk by the OS are lost. The
database itself will still be in a consistent state, but it might
not have all recent changes.

@mkrautz mkrautz force-pushed the mkrautz:sqlite-wal branch from 2f0e5d6 to 1818476 Mar 5, 2017

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2017

@Kissaki PTAL

@Kissaki
Kissaki approved these changes Mar 5, 2017

@mkrautz mkrautz merged commit cad1bac into mumble-voip:master Mar 5, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@HarpyWar HarpyWar referenced this pull request May 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.