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

feat: implement multiple read single write for sqlite #3568

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Nov 14, 2021

Description

  • Implemented a generic system-wide pool connection manager for diesel SQLite connections that uses the modern Write-ahead Log (WAL) SQLite3 database mode to speed up database operations (see https://www.sqlite.org/wal.html) with a configurable pool size.
  • Changed the Wallet's SQLite database connection to make use of the pool connection manager.
  • Refined SQLite profiling trace logs to only log an entry if the total time is > 1ms - this cut down on noisy entries as most database calls are now < 1ms.
  • Notable SQLite pragma settings:

Motivation and Context

The wallet's database connection was not optimal.

How Has This Been Tested?

  • Unit tests
  • Cucumber tests (npm test -- --tags "not @long-running and not @broken")
  • System-level tests with positive results all around (see discussion):
    • Configuration: Base node + console wallet + merge mining proxy + XMRig -> Mining
    • Test 1: Moderate coin split (22 transactions producing 3124 outputs)
    • Test 2: Moderate stress test (2x simultaneous make-it-rain transactions from one sender to two receiving wallets of 250 transactions each)

The graphs below show the SQLite WAL mode profiling for the above two tests (sender wallet only) with a pool size of 16 for all read and write database operations where the total time (acquire lock time + db operation time) was larger than 1ms. The wallet in question had a communications breakdown to its connected base node for a while after all transactions were completed and when it eventually gained an RPC connection validation protocols for all 500 transactions queried the base node as fast as possible, which panned out in some of those protocols waiting for a pooled connection. The actual time db operations will wait for a write operation to conclude is managed by SQLite and bunched within the db op time measurement.

image

image

- Implemented a system-wide diesel sqlite pool connection manager that uses
the modern Write-ahead Log (WAL) SQLite3 database mode to speed up database
operations. See https://www.sqlite.org/wal.html.
- Implemented this for use by the Wallet's database connection.
Copy link
Contributor

@philipr-za philipr-za left a comment

Choose a reason for hiding this comment

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

Very nice, looks good to me.

let mut pool = SqliteConnectionPool::new(db_path.clone(), 1, true, true, Duration::from_secs(60));
pool.create_pool()
.unwrap_or_else(|_| panic!("Error connecting to {}", db_path));
// Note: For this test the connection pool is setup with a pool size of one; the pooled connection must go out
Copy link
Contributor

@philipr-za philipr-za Nov 15, 2021

Choose a reason for hiding this comment

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

Nit: this is fine, just a note that you could explicitly drop() the pool rather than using a {} scope which I think is a bit more explicit and easy to use.

@aviator-app aviator-app bot merged commit 8d22164 into tari-project:development Nov 15, 2021
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 15, 2021
* development:
  feat: implement multiple read single write for sqlite (tari-project#3568)
@hansieodendaal hansieodendaal deleted the ho_sqlite_db_concurrency branch November 15, 2021 09:57
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 18, 2021
* development: (32 commits)
  feat: add atomic swap refund transaction handling (tari-project#3573)
  feat: improve wallet connectivity status for console wallet (tari-project#3577)
  v0.21.1
  feat: add error codes to LibWallet for CipherSeed errors (tari-project#3578)
  ci: split cucumber job into two (tari-project#3583)
  feat(wallet): import utxo’s as EncumberedToBeReceived rather than Unspent (tari-project#3575)
  docs: rfc 0250_Covenants (tari-project#3574)
  feat: get fee for transactions for stratum transcoder (tari-project#3571)
  test: make monerod stagenet usage resilient (tari-project#3572)
  feat: add atomic swap htlc sending and claiming (tari-project#3552)
  feat: implement prometheus metrics for base node (tari-project#3563)
  feat: implement multiple read single write for sqlite (tari-project#3568)
  feat: trigger time lock balance update when block received (tari-project#3567)
  test: reduce cucumber ci to critical only (tari-project#3566)
  test: fix cucumber console wallet startup (tari-project#3564)
  chore: add node id/public key to log mdc (tari-project#3559)
  fix: avoid implicit using of the time crate (tari-project#3562)
  feat: one-click installer - cli edition (tari-project#3534)
  ci: add workflow dispatch to libwallet build action (tari-project#3556)
  fix: stop leak of value of recovered output (tari-project#3558)
  ...
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.

2 participants