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: improve lmdb dynamic growth #6242

Conversation

hansieodendaal
Copy link
Contributor

Description

  • Improved the LMDB resizing during block sync of many consecutive full blocks; this is required due to how the SMT works currently as it is replaced for every new block.
  • Added SMT database write profiling measurements.

Motivation and Context

See above.

How Has This Been Tested?

  • System-level testing in Windows and Ubuntu - archival node fresh block sync of many full blocks as generated during a stress test on esmeralda.
  • fn insert_tip_smt(&self, txn: &WriteTransaction<'_>, smt: &OutputSmt) always succeeded with these settings.

image

image

What process can a PR reviewer use to test or verify this change?

Code review

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@hansieodendaal hansieodendaal requested a review from a team as a code owner April 2, 2024 06:44
@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Apr 2, 2024
Copy link

github-actions bot commented Apr 2, 2024

Test Results (CI)

    3 files    120 suites   32m 42s ⏱️
1 273 tests 1 273 ✅ 0 💤 0 ❌
3 811 runs  3 811 ✅ 0 💤 0 ❌

Results for commit 9f7801b.

♻️ This comment has been updated with latest results.

Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

utACK

Nice, resizing looks more robust though heavy on storage. We might want to reduce this once the underlying cause is addressed. The profiling info is super helpful!

infrastructure/storage/src/lmdb_store/store.rs Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Apr 2, 2024

Test Results (Integration tests)

 2 files  11 suites   24m 44s ⏱️
33 tests 32 ✅ 0 💤 1 ❌
34 runs  33 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 9f7801b.

♻️ This comment has been updated with latest results.

@hansieodendaal hansieodendaal force-pushed the ho_improve_lmdb_dynamic_growth branch 2 times, most recently from a8746c6 to c8b2204 Compare April 3, 2024 07:06
@hansieodendaal
Copy link
Contributor Author

Previous commit:

2024-04-03 05:59:18.524515400 [tari_core::chain_storage::lmdb_db::helpers] TRACE lmdb_replace - 184 MB, serialize check in 148.30ms, serialize in 457.40ms
2024-04-03 05:59:18.592395600 [tari_core::chain_storage::lmdb_db::lmdb] TRACE lmdb_replace - 184 MB, lmdb write in 67.82ms
2024-04-03 05:59:19.209934000 [tari_core::chain_storage::lmdb_db::helpers] TRACE lmdb_replace - 184 MB, serialize check in 144.04ms, serialize in 460.26ms
2024-04-03 05:59:19.209983300 [tari_core::chain_storage::lmdb_db::lmdb_db] TRACE Inserted 184 MB with key '08000000' into 'tip_utxo_smt' (size 908576) in 1.29s

Latest commit:

2024-04-03 08:54:38.424826600 [tari_core::chain_storage::lmdb_db::helpers] TRACE lmdb_replace - 184 MB, serialize check in 11.00µs, serialize in 458.09ms
2024-04-03 08:54:38.491925800 [tari_core::chain_storage::lmdb_db::lmdb] TRACE lmdb_replace - 184 MB, lmdb write in 67.04ms
2024-04-03 08:54:38.501529800 [tari_core::chain_storage::lmdb_db::lmdb_db] TRACE Inserted ~194 MB with key '08000000' into 'tip_utxo_smt' (size 908601) in 534.84ms

Improved the LMDB resizing during block sync of many consecutive full blocks;
this is required due to how the SMT works currently as it is replaced for
every new block.
Added SMT database write profiling measurements.
SWvheerden
SWvheerden previously approved these changes Apr 3, 2024
@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Apr 3, 2024
@ghpbot-tari-project ghpbot-tari-project added the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Apr 3, 2024
@SWvheerden SWvheerden merged commit b48a830 into tari-project:development Apr 4, 2024
14 of 16 checks passed
@hansieodendaal hansieodendaal deleted the ho_improve_lmdb_dynamic_growth branch April 4, 2024 06:17
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
sdbondi added a commit to sdbondi/tari that referenced this pull request Apr 15, 2024
* development:
  fix!: avoid `Encryptable` domain collisions (tari-project#6275)
  ci(fix): docker image build fix and ci improvements (tari-project#6270)
  feat: keep smt memory (tari-project#6265)
  feat: show warning when GRPC method is disallowed (tari-project#6246)
  fix(chat): metadata panic (tari-project#6247)
  feat: add monerod detection as an option to the merge mining proxy (tari-project#6248)
  chore(deps): bump h2 from 0.3.24 to 0.3.26 (tari-project#6250)
  feat: improve lmdb dynamic growth (tari-project#6242)
  feat: allow wallet type from db to have preference (tari-project#6245)
  feat: prevent mempool panic (tari-project#6239)
  ci: bump nightly version (tari-project#6241)
  feat: add validation for zero confirmation block sync (tari-project#6237)
  feat: new template with coinbase call (tari-project#6226)
  feat: improve wallet sql queries (tari-project#6232)
  chore: remove ahash as dependancy (tari-project#6238)
  feat: add dynamic growth to lmdb (tari-project#6231)
  chore(deps): bump borsh from 0.10.3 to 1.0.0 in /applications/minotari_ledger_wallet (tari-project#6236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants