validation: Make VerifyDB level 4 interruptible

MarcoFalke commented at 5:41 pm on June 2, 2020: member

level 0,1,2, and 3 are already interruptible, so make level 4 also interruptible

MarcoFalke added the label Validation on Jun 2, 2020

MarcoFalke commented at 6:26 pm on June 2, 2020: member

(intentionally left empty)

fanquake commented at 8:32 am on June 3, 2020: member

Concept ACK

In fa6e9b84ef25b266729dd03337ae00c0ab44e52a:

ActivateBestChain is only called in the “msghand” or one of the RPC threads, neither of which is a boost::thread.

Is this correct? ActivateBestChain() is also called by ThreadImport(), both directly as well via LoadExternalBlockFile. ThreadImport is run in the boost::thread_group:

https://github.com/bitcoin/bitcoin/blob/657b82cef0e8e8695fc189d013a4353bdbebb041/src/init.cpp#L1847-L1849

VerifyDB is only called in the main thread (“init”) or one of the RPC threads, neither of which is a boost::thread.

Checked that the calls to CVerifyDB().VerifyDB() are in the init thread and verifychain RPC call.

MarcoFalke commented at 9:53 am on June 3, 2020: member

Is this correct? ActivateBestChain() is also called by ThreadImport(), both directly as well via LoadExternalBlockFile. ThreadImport is run in the boost::thread_group:

Good catch, you are correct. However, after #18786 (init: Remove boost from ThreadImport), it should be possible to simply switch ThreadImport to a std::thread. Also see that in line 2929 there is another non-boost breakpoint: if (ShutdownRequested()) break;.

I will adjust the commit message accordingly.

validation: Remove unused boost interruption_point

ActivateBestChain (ABC) is only called in the "msghand" or one of the
RPC threads, neither of which is a boost::thread. However, ABC is also
called in ThreadImport (which currently happens to be a boost::thread).
In all cases, the interruption_point is redundant with the breakpoint in
ABC that triggers when ShutdownRequested()

VerifyDB is only called in the main thread ("init") or one of the RPC
threads, neither of which is a boost::thread.

fa1d5800d9

validation: Make VerifyDB level 4 interruptible fa3b4f9b8e

MarcoFalke force-pushed on Jun 3, 2020

MarcoFalke commented at 10:07 am on June 3, 2020: member

Done. Force pushed a commit message change:

 0$ git range-diff bitcoin-core/master  faed13ea16 fa3b4f9b8e
 11:  fa6e9b84ef ! 1:  fa1d5800d9 validation: Remove unused boost interruption_point
 2    @@ Metadata
 3      ## Commit message ##
 4         validation: Remove unused boost interruption_point
 5     
 6    -    ActivateBestChain is only called in the "msghand" or one of the RPC
 7    -    threads, neither of which is a boost::thread.
 8    +    ActivateBestChain (ABC) is only called in the "msghand" or one of the
 9    +    RPC threads, neither of which is a boost::thread. However, ABC is also
10    +    called in ThreadImport (which currently happens to be a boost::thread).
11    +    In all cases, the interruption_point is redundant with the breakpoint in
12    +    ABC that triggers when ShutdownRequested()
13     
14         VerifyDB is only called in the main thread ("init") or one of the RPC
15         threads, neither of which is a boost::thread.
162:  faed13ea16 = 2:  fa3b4f9b8e validation: Make VerifyDB level 4 interruptible

DrahtBot commented at 12:28 pm on June 4, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18731 (refactor: Make CCheckQueue RAII-styled by hebasto)
#18710 (Add local thread pool to CCheckQueue by hebasto)

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.

laanwj commented at 12:48 pm on June 4, 2020: member

Code review ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 Thank you for cleaning up more boost interruption point usage.

fanquake approved

fanquake commented at 1:42 pm on June 4, 2020: member

ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12

I will adjust the commit message accordingly.

Thanks.

Rechecked that a call to verifychain is interrupted:

 0diff --git a/src/validation.cpp b/src/validation.cpp
 1index 1ce440dc0..6e813e00a 100644
 2--- a/src/validation.cpp
 3+++ b/src/validation.cpp
 4@@ -4337,7 +4337,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview,
 5                 return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
 6             if (!::ChainstateActive().ConnectBlock(block, state, pindex, coins, chainparams))
 7                 return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString());
 8-            if (ShutdownRequested()) return true;
 9+            //if (ShutdownRequested()) return true;
10         }
11     }

 02020-06-04T13:04:33.970347Z [msghand] New outbound peer connected: version: 70015, blocks=633028, peer=17 (block-relay)
 12020-06-04T13:04:47.234325Z [httpworker.0] Verifying last 100 blocks at level 4
 22020-06-04T13:04:47.234376Z [httpworker.0] [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...tor: Thread interrupt
 32020-06-04T13:05:11.967041Z [init] Shutdown: In progress...
 42020-06-04T13:05:11.967992Z [addcon] addcon thread exit
 52020-06-04T13:05:11.971026Z [torcontrol] torcontrol thread exit
 62020-06-04T13:05:11.978165Z [opencon] opencon thread exit
 72020-06-04T13:05:11.979705Z [net] net thread exit
 82020-06-04T13:05:13.444961Z [httpworker.0] [70%]...[80%]...[90%]...[DONE].
 92020-06-04T13:05:21.697665Z [httpworker.0] No coin database inconsistencies in last 100 blocks (187923 transactions)
102020-06-04T13:05:21.786008Z [msghand] msghand thread exit
112020-06-04T13:05:21.948084Z [scheduler] scheduler thread exit
122020-06-04T13:05:21.988007Z [shutoff] Writing 0 unbroadcast transactions to disk.
132020-06-04T13:05:22.000826Z [shutoff] Dumped mempool: 0.001229s to copy, 0.024559s to dump
142020-06-04T13:05:22.032434Z [shutoff] FlushStateToDisk: write coins cache to disk (272667 coins, 38381kB) started
152020-06-04T13:05:22.189725Z [shutoff] FlushStateToDisk: write coins cache to disk (272667 coins, 38381kB) completed (0.16s)
162020-06-04T13:05:22.227430Z [shutoff] FlushStateToDisk: write coins cache to disk (0 coins, 3292kB) started
172020-06-04T13:05:22.227563Z [shutoff] FlushStateToDisk: write coins cache to disk (0 coins, 3292kB) completed (0.00s)
182020-06-04T13:05:22.289343Z [shutoff] [default wallet] Releasing wallet
192020-06-04T13:05:22.294110Z [shutoff] Shutdown: done

This PR

 02020-06-04T13:26:17.699492Z [msghand] New outbound peer connected: version: 70015, blocks=633028, peer=8 (full-relay)
 12020-06-04T13:26:39.372328Z [httpworker.0] Verifying last 100 blocks at level 4
 22020-06-04T13:26:39.372380Z [httpworker.0] [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...tor: Thread interrupt
 32020-06-04T13:27:24.895974Z [addcon] addcon thread exit
 42020-06-04T13:27:24.896071Z [torcontrol] torcontrol thread exit
 52020-06-04T13:27:24.896148Z [init] Shutdown: In progress...
 62020-06-04T13:27:24.905837Z [net] net thread exit
 72020-06-04T13:27:24.969283Z [msghand] msghand thread exit
 82020-06-04T13:27:24.969452Z [opencon] opencon thread exit
 92020-06-04T13:27:25.134754Z [scheduler] scheduler thread exit
102020-06-04T13:27:25.174710Z [shutoff] Writing 0 unbroadcast transactions to disk.
112020-06-04T13:27:25.186320Z [shutoff] Dumped mempool: 0.001838s to copy, 0.02701s to dump
122020-06-04T13:27:25.217315Z [shutoff] FlushStateToDisk: write coins cache to disk (273951 coins, 38546kB) started
132020-06-04T13:27:25.389811Z [shutoff] FlushStateToDisk: write coins cache to disk (273951 coins, 38546kB) completed (0.17s)
142020-06-04T13:27:25.424695Z [shutoff] FlushStateToDisk: write coins cache to disk (0 coins, 3292kB) started
152020-06-04T13:27:25.424895Z [shutoff] FlushStateToDisk: write coins cache to disk (0 coins, 3292kB) completed (0.00s)
162020-06-04T13:27:25.492179Z [shutoff] [default wallet] Releasing wallet
172020-06-04T13:27:25.497838Z [shutoff] Shutdown: done

fanquake merged this on Jun 4, 2020

fanquake closed this on Jun 4, 2020

MarcoFalke deleted the branch on Jun 4, 2020

sidhujag referenced this in commit 151c9974a4 on Jun 4, 2020

fanquake referenced this in commit b13c8a70ed on Jun 6, 2020

fanquake referenced this in commit aeb100f79e on Jun 7, 2020

luke-jr referenced this in commit c9254a2d5a on Jun 9, 2020

fanquake referenced this in commit 2aca26d5dd on Jun 12, 2020

fanquake referenced this in commit 83fd3a6d73 on Jun 12, 2020

ComputerCraftr referenced this in commit 3d8217c1b4 on Jun 16, 2020

fanquake referenced this in commit 057bd3189f on Jun 19, 2020

stackman27 referenced this in commit a30d3155c7 on Jun 26, 2020

sidhujag referenced this in commit 3598bbfdba on Jul 7, 2020

Fabcien referenced this in commit 9ecabf23be on Feb 24, 2021

deadalnix referenced this in commit ae09ca09e4 on Jul 23, 2021

DrahtBot locked this on Feb 15, 2022

validation: Make VerifyDB level 4 interruptible #19142

Conflicts