wallet: Remove WalletDatabase refcounting and enforce only one Batch access the database at a time #20096
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
achow101
force-pushed
the
rm-bdb-refcount
branch
from
24949d8 to
a1e957c
Compare
achow101
force-pushed
the
rm-bdb-refcount
branch
3 times, most recently
from
c483870 to
c833944
Compare
achow101
force-pushed
the
rm-bdb-refcount
branch
from
c833944 to
4326ae9
Compare
achow101
force-pushed
the
rm-bdb-refcount
branch
2 times, most recently
from
3b7a37d to
2669bae
Compare
src/wallet/bdb.cpp
Outdated
There was a problem hiding this comment.
this is failing for me:
AssertionError: Unexpected stderr Assertion failed: (m_refcount <= 1), function AddRef, file wallet/bdb.cpp, line 794. !=
achow101
force-pushed
the
rm-bdb-refcount
branch
from
2669bae to
03d82a0
Compare
Make sure the WalletBatch in NewKeyPool writes and destructs before TopUp.
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
Concept ACK, Improves/simplifies the codebase by dropping useless support for concurrent batches.
Do you want to rebase to get a fresh CI?
|
It failed with So I don't think this is a CI issues that can be fixed by a rebase |
|
Ok, fix and rebase 🙉 |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
Closing as the constraint is no longer true due to nesting. |
6 participants
We never have multiple threads making able to multiple
WalletBatchs due to the extensive use ofcs_wallet,cs_KeyStore, andcs_desc_man, so the use of refcounting to handle the case where that may happen is unnecessary. Thus we can use an assert to enforce that only oneBerkeleyBatchis accessing theBerkeleyDatabaseat a time. The only instances where multipleBerkeleyBatchs were being created were where one was made in a caller function, and then the called function created another for itself. For those cases, we instead either pass in the caller'sWalletBatchor limit the scope of the caller'sWalletBatchso that there are no overlaps.The refcounting in
BerkeleyDatabaseis kept because it did more than just count the number ofBerkeleyBatchs, it also indicated when the database was flushed. However theAddRefandRemoveReffunctions have been removed fromWalletDatabase.