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

Enable WAL mode on Sqlite #1871

Merged
merged 11 commits into from
Jul 15, 2021
Merged

Enable WAL mode on Sqlite #1871

merged 11 commits into from
Jul 15, 2021

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Jul 12, 2021

Write-Ahead Logging is both much more performant in general, and more suited to our particular access patterns.

With a simple throughput performance test, it improves performance by a factor of 5-20x depending on the sync flag.

version throughput
mode=journal sync=normal (*) 11 htlc/s
mode=journal sync=full 7 htlc/s
mode=wal sync=normal 248 htlc/s
mode=wal sync=full (**) 62 htlc/s

(*) previous setting
(**) new setting

I went with a conservative new setting of wal+full sync, which is both 5x more performant, and more secure than what we had before.

In WAL mode when synchronous is NORMAL (1), the WAL file is synchronized before each checkpoint and the database file is synchronized after each completed checkpoint and the WAL file header is synchronized when a WAL file begins to be reused after a checkpoint, but no sync operations occur during most transactions. With synchronous=FULL in WAL mode, an additional sync operation of the WAL file happens after each transaction commit. The extra WAL sync following each transaction help ensure that transactions are durable across a power loss. Transactions are consistent with or without the extra syncs provided by synchronous=FULL. If durability is not a concern, then synchronous=NORMAL is normally all one needs in WAL mode.

@joostjager I couldn't help but notice that despite using a different test setting, I get exactly the same result as you with the baseline, on my laptop. Is there any chance you could re-run your test with this branch?

Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this is a very good find!

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2021

Codecov Report

Merging #1871 (0672cdc) into master (95fffe3) will increase coverage by 0.21%.
The diff coverage is 86.20%.

❗ Current head 0672cdc differs from pull request most recent head bcfb290. Consider uploading reports for the commit bcfb290 to get more accurate results

@@            Coverage Diff             @@
##           master    #1871      +/-   ##
==========================================
+ Coverage   87.09%   87.30%   +0.21%     
==========================================
  Files         156      159       +3     
  Lines       11723    11943     +220     
  Branches      455      471      +16     
==========================================
+ Hits        10210    10427     +217     
- Misses       1513     1516       +3     
Impacted Files Coverage Δ
.../scala/fr/acinq/eclair/db/sqlite/SqliteUtils.scala 82.60% <81.81%> (-17.40%) ⬇️
...ir-core/src/main/scala/fr/acinq/eclair/Setup.scala 80.45% <100.00%> (+0.11%) ⬆️
.../src/main/scala/fr/acinq/eclair/db/Databases.scala 86.48% <100.00%> (+4.34%) ⬆️
...main/scala/fr/acinq/eclair/router/Validation.scala 90.76% <0.00%> (-2.70%) ⬇️
...ain/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala 90.24% <0.00%> (-2.42%) ⬇️
...a/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala 94.04% <0.00%> (-1.85%) ⬇️
...n/scala/fr/acinq/eclair/json/JsonSerializers.scala 86.48% <0.00%> (-0.70%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 95.94% <0.00%> (-0.01%) ⬇️
.../fr/acinq/eclair/blockchain/fee/FeeEstimator.scala 100.00% <0.00%> (ø)
...q/eclair/crypto/keymanager/ChannelKeyManager.scala 87.50% <0.00%> (ø)
... and 15 more

@joostjager
Copy link

@joostjager I couldn't help but notice that despite using a different test setting, I get exactly the same result as you with the baseline, on my laptop. Is there any chance you could re-run your test with this branch? Maybe even a 2nd run with eclair.db.sqlite.sync=normal

If you provide a PR on the repo with the exact change, I am happy to run it on our rig.

@pm47 pm47 mentioned this pull request Jul 15, 2021
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
@pm47 pm47 merged commit ca51a2d into master Jul 15, 2021
dpad85 added a commit that referenced this pull request Jul 16, 2021
This commit is adapted from #1871
@pm47 pm47 deleted the sqlite-wal-mode branch August 24, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants