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

Use CScheduler for wallet flushing, remove ThreadFlushWalletDB #9605

Merged
merged 3 commits into from Mar 7, 2017

Conversation

@TheBlueMatt
Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Jan 20, 2017

Also uses std::function instead of boost::function.

Copy link
Member

@sipa sipa left a comment

Concept ACK

@@ -23,7 +22,7 @@
//
// CScheduler* s = new CScheduler();
// s->scheduleFromNow(doSomething, 11); // Assuming a: void doSomething() { }
// s->scheduleFromNow(boost::bind(Class::func, this, argument), 3);
// s->scheduleFromNow(std::bind(Class::func, this, argument), 3);
// boost::thread* t = new boost::thread(boost::bind(CScheduler::serviceQueue, s));
Copy link
Member

@sipa sipa Jan 20, 2017

Adapt comment here as well?

Copy link
Contributor Author

@TheBlueMatt TheBlueMatt Jan 20, 2017

No, it still must run inside a boost::thread because it relies on boost::interruption_point. I believe @theuni is planning on cleaning that up.

Copy link
Contributor Author

@TheBlueMatt TheBlueMatt Jan 20, 2017

well technically boost::thread takes a boost::function, so I assume if you uses std::bind it would create a boost::function around the std::function......I'll let cory do that when we kill boost::threads.

Copy link
Member

@theuni theuni Jan 21, 2017

Yes, I have a local branch with CScheduler converted. I'm doing them in chunks, because the last huge PR proved to be too big to review/rebase. I'll submit that one next (#9566 is the current one).

@sipa
Copy link
Member

@sipa sipa commented Jan 20, 2017

@theuni
Copy link
Member

@theuni theuni commented Jan 21, 2017

Concept ACK!. I've considered doing this a few times as well.

@@ -768,67 +768,60 @@ DBErrors CWalletDB::ZapWalletTx(CWallet* pwallet, vector<CWalletTx>& vWtx)
return DB_LOAD_OK;
}

void ThreadFlushWalletDB()
void MaybeFlushWalletDB()
Copy link
Member

@laanwj laanwj Jan 23, 2017

If you're renaming this function please, finally, change it to something else than FlushWalletDB. What is being done here is not merely a flush (those are done all the time as CWalletDB instances go out of scope) but more like "database consolidation", merging the log files into wallet.dat so that the wallet is self-contained. This has confused many people over time.

Copy link
Contributor Author

@TheBlueMatt TheBlueMatt Jan 23, 2017

Done, called it "compact"

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 23, 2017

Concept ACK

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 26, 2017

Thank you! utACK 30f8d50

@laanwj laanwj added this to the 0.15.0 milestone Feb 6, 2017
@TheBlueMatt TheBlueMatt force-pushed the 2017-01-cscheduler-cleanups branch from 30f8d50 to dbb8072 Feb 8, 2017
@TheBlueMatt
Copy link
Contributor Author

@TheBlueMatt TheBlueMatt commented Feb 8, 2017

Rebased. Note that this now forms the first of a rather long patchset to move wallet callbacks into a background thread without introducing races in APIs or wallet consistency issues.

@theuni
Copy link
Member

@theuni theuni commented Mar 6, 2017

utACK. Needs rebase.

@TheBlueMatt TheBlueMatt force-pushed the 2017-01-cscheduler-cleanups branch from dbb8072 to 0235be1 Mar 6, 2017
@TheBlueMatt
Copy link
Contributor Author

@TheBlueMatt TheBlueMatt commented Mar 6, 2017

Rebased.

@laanwj
Copy link
Member

@laanwj laanwj commented Mar 7, 2017

utACK 0235be1

@laanwj laanwj merged commit 0235be1 into bitcoin:master Mar 7, 2017
1 check passed
laanwj added a commit that referenced this issue Mar 7, 2017
…lletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 21, 2019
…FlushWalletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jan 27, 2019
…FlushWalletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 29, 2019
…FlushWalletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Feb 5, 2019
…FlushWalletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Feb 5, 2019
…FlushWalletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Feb 5, 2019
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Feb 5, 2019
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Feb 5, 2019
TriKriSta referenced this issue in ivansib/sibcoin Jul 4, 2019
…lletDB

0235be1 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
735d9b5 Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)
73296f5 CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)

Tree-SHA512: c04f97beab65706c444c126be229d02887df9b0972d8fb15ca1f779ef0e628cf7ecef2bf533c650d9b44645b63e01de22f17266a05907e778938d64cc6e19de6
TriKriSta referenced this issue in ivansib/sibcoin Jul 4, 2019
furszy added a commit to PIVX-Project/PIVX that referenced this issue Jan 30, 2021
…llet

98d770f CScheduler boost->std::function, use millisecs for times, not secs (Matt Corallo)
67e068c Remove now unneeded ChainTip signal (furszy)
bcdd3e9 Move ChainTip sapling update witnesses and nullifiers to BlockConnected/BlockDisconnected. (furszy)
b799070 Take a CTransactionRef in AddToWalletIfInvolvingMe to avoid a copy (Matt Corallo)
d77244c Remove dead-code tracking of requests for blocks we generated (Matt Corallo)
10ccbbf Hold cs_wallet for whole block [dis]connection processing (Matt Corallo)
1a45036 fix compiler warning member functions not marked as 'override' (furszy)
d3867a7 An adaptation of Corallo's btc@461e49fee2935b1eb4d4ea7bae3023e655c0a6d8 (Matt Corallo)
f5ac648 [Refactor] Move Sapling ChainTip signal notification loop to use the latest connectTrace class structure (furszy)
8704d9d Handle SyncTransaction in ActivateBestChain instead of ConnectTrace (Matt Corallo)
6dcb6b0 Keep conflictedTxs in ConnectTrace per-block (Matt Corallo)
50d3e0e Handle conflicted transactions directly in ConnectTrace (furszy)
27fb897 Make ConnectTrace::blocksConnected private, hide behind accessors (Matt Corallo)
60329da Add pblock to connectTrace at the end of ConnectTip, not start (Matt Corallo)
e7c2789 Include missing #include in zmqnotificationinterface.h (Matt Corallo)
1b396b8 Move `NotifyEntryRemoved` to use the general interfaces::MakeHandler (furszy)
4cb5820 Better document usage of SyncTransaction (Alex Morcos)
21be4e2 Introduce MemPoolConflictRemovalTracker (Alex Morcos)
7326acb mempool: add notification for added/removed entries (Wladimir J. van der Laan)
a8605d2 Clean up tx prioritization when conflict mined (Casey Rodarmor)
e7db9ff remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos)
76c72c6 remove external usage of mempool conflict tracking (Alex Morcos)
ef429af tests: Stop node before removing the notification file (furszy)
15a21c2 tests: write the notification to different files to avoid race condition (Chun Kuan Lee)
466e97a [Wallet] Simplify InMempool (furszy)
85e18f0 Rename FlushWalletDB -> CompactWalletDB, add function description (Matt Corallo)
00f36ea Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (Matt Corallo)

Pull request description:

  Revamped the validation interface interaction with the wallet, encapsulating and improving the mempool and block signaling and each of the wallet signals handler.
  Restructured the Sapling witnesses and nullifiers update under the new signals.
  Solved several bugs that found on the way as well (Check each commit description).

  This PR concludes with #1726 long road :). Pushing our repository around 2 years ahead in btc time in the mempool and validation interface areas (without including RBF).
  The new validation -> wallet interaction architecture will enable further, and much more user facing important, improvements for the syncing process, overall software responsiveness and multithreading locking issues.

  Adapted backports:
  bitcoin#6464 --> Always clean up manual transaction prioritization (mempool)
  bitcoin#9240 --> Remove txConflicted (mempool)
  bitcoin#9371 --> Notify on removal (mempool)
  bitcoin#9605 --> Use CScheduler for wallet flushing, remove ThreadFlushWalletDB (walletdb)
  bitcoin#9725 --> CValidationInterface Cleanups (wallet, validation and validation interface)

ACKs for top commit:
  random-zebra:
    utACK 98d770f
  Fuzzbawls:
    utACK 98d770f

Tree-SHA512: 84c86567c2bff36b859b2ae73c558956a70dff0fffb4f73208708d92165b851bf42d35246410238c66c7a4b77e5bf51ec93885234a75fa48901fd182d2f70a28
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants