[Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) #4805
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/wallet.cpp
Outdated
Member
There was a problem hiding this comment.
Maybe make fFlushOnClose a constructor argument?
Member
|
ACK on concept. It is very important to flush immediately on a keypool update, but less so when adding a transaction (as this can be corrected) and then especially en-masse during a rescan. |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4805_04dbecac9dcc1cc0b8ff450d48104bc09f32d3ab/ for binaries and test log. |
Contributor
Author
|
Addressed inline comments. |
Member
|
Needs rebase. |
Contributor
Author
|
rebased |
Contributor
|
concept ACK. Makes me dislike our RAII CDB method of I/O for the wallet even more ;p Pondering an alternate way of accomplishing the same thing. Gave this an "it works" tests. |
Member
|
The best "alternate solution" would be to get rid of berkeleydb completely (eg #5686). |
laanwj
added a commit
that referenced
this pull request
Jan 26, 2015
44bc988 [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) (Cozz Lovan)
Warrows
added a commit
to Warrows/PIVX
that referenced
this pull request
Jul 20, 2019
Backport of bitcoin#4805 ( commit 44bc988 )
Warrows
added a commit
to Warrows/PIVX
that referenced
this pull request
Oct 9, 2019
Backport of bitcoin#4805 ( commit 44bc988 )
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Oct 9, 2019
91b48c7 [Build] Add new merkle files to CMake lists (warrows) 48a3aff [Wallet] Ignore coinbase and zc tx "conflicts" (warrows) 3572354 [Wallet] Fix an error in tx depth computation (warrows) 6928369 [Tests] Enable abandonconflict functional test (warrows) 34cd496 Fix that CWallet::AbandonTransaction would only traverse one level (Ben Woosley) aba5b75 Fix calculation of balances and available coins. (Alex Morcos) 48d705f Remove stale wallet transactions on initial load. (presstab) 12985ae Flush wallet after abandontransaction (Alex Morcos) 8f87956 [Wallet] sort pending wallet transactions before reaccepting (dexX7) 9c2f445 [Wallet] Call notification signal when a transaction is abandoned (Jonas Schnelli) 778ebf3 Add new rpc call: abandontransaction (Alex Morcos) 0e86c3e Make wallet descendant searching more efficient (Alex Morcos) d0083a8 Make sure conflicted wallet tx's update balances (Alex Morcos) 6a50e03 [Wallet] Keep track of explicit wallet conflicts instead of using mempool (warrows) 7ccb2b5 [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) (warrows) 47345be [Refactor] Move wallet functions out of header (warrows) ab9efb8 [Wallet] Switch to a constant-space Merkle root/branch algorithm (warrows) 5447622 [Wallet] Do not store Merkle branches in the wallet (warrows) Pull request description: This pull request is a happy melting pot of improvements regarding transactions handling. Most of them are backports from bitcoin. I advise reviewers to check the code of the different commits independently to understand them more easily. However, testing is probably better done all at once. I am making a single pull request because these changes are all entangled and introducing some of them without others would probably introduce temporary bugs. ## Commits details ## - 6c3e2ac backport of bitcoin#6550 - 5304fdf backport of bitcoin#6508 - c3eeeac simple code move from the header to the cpp file. It contains no functional change. - 6cc4d37 backport of bitcoin#4805 - 10be1db backport of bitcoin#7105 - 8a34c32 backport of bitcoin#7306 - 3caf123, 9e17178 and 240f5b4 are the backport for bitcoin#7312 - ad6d0b1 backport of bitcoin#5511 - fcc07c3562 backport of bitcoin#9311 - 5ed5e266794 is an update of #825 - 392d504 backport of bitcoin#7715 - 7199f3a backport of bitcoin#13652 - f09d999 enables and fixes the test from bitcoin#7312 - 4fd43c5 fixes an oversight in bitcoin#7105 backport ACKs for top commit: random-zebra: ACK 91b48c7 Fuzzbawls: ACK 91b48c7 Tree-SHA512: 2628cebe98805b8048b920b51ee26fd4f0c53643d78da9b8cb265aede52dfe1d40c8c19d34293c232c5c35be7f1ab89ff5b4a07073a4b27c371ea70eb8708669
wqking
pushed a commit
to wqking-temp/Vitae
that referenced
this pull request
May 24, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
Kokary
pushed a commit
to Kokary/wagerr
that referenced
this pull request
May 26, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
Kokary
pushed a commit
to Kokary/wagerr
that referenced
this pull request
Nov 13, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
Kokary
pushed a commit
to Kokary/wagerr
that referenced
this pull request
Nov 17, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
Kokary
pushed a commit
to Kokary/wagerr
that referenced
this pull request
Nov 17, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
KolbyML
pushed a commit
to KolbyML/bitcoin
that referenced
this pull request
Nov 21, 2020
Backport of bitcoin#4805 ( commit 44bc988 )
Kokary
pushed a commit
to wagerr/wagerr
that referenced
this pull request
Nov 24, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
Cryptarchist
pushed a commit
to wagerr/wagerr
that referenced
this pull request
Nov 30, 2020
Backport of bitcoin/bitcoin#4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 )
lyricidal
added a commit
to PRCYCoin/PRCYCoin
that referenced
this pull request
Jul 24, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are currently flushing the wallet after every operation.
This is very slow. In case of syncing txs, we need performance, rather than
durability. Our SetBestChain-mechanism ensures, that txs from a block are always synced,
even on a crash, as we rescan the necessary blocks on startup.
The FlushWalletThread only flushes, if there is no wallet-update for 2 seconds.
I tested replacing the txn_checkpoint(..) call with
log_flush, but this does not help.
Currently I can only add about 10 txs per second to the wallet on my machine.
After this patch its more than 1000 txs per second.
For example importaddress mvqaxopu9Uef2qjXSV3fGrp6DcEYnCoYWK
adds about 20000 txs to the wallet.
Together with these optimizations #4702 #4712 you hardly notice the difference between a normal
rescan and adding 20000 txs to the wallet.
Currently this takes hours, I had to let my client running over night, in order to finish.
The overall performance increase is like factor 500 or so on a larger wallet.