validation: Make VerifyDB level 4 interruptible #19142

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

(intentionally left empty)

<!--- diff --git a/tmp/getblockchaininfo.before b/tmp/getblockchaininfo.after index 105067ae72..feda6acb22 100644 --- a/tmp/getblockchaininfo.before +++ b/tmp/getblockchaininfo.after @@ -5,7 +5,7 @@ "bestblockhash": "0000000057d420c4cc0108470cf834dcd995e4d78efbda46b683df27f6ee2974", "difficulty": 1, "mediantime": 1590788299, - "verificationprogress": 0.9995059441787998, + "verificationprogress": 0.9995034411929307, "initialblockdownload": true, "chainwork": "00000000000000000000000000000000000000000000015b6e40ba6cc8caf405", "size_on_disk": 27436174483, [marco@kscrwn btc_bitcoin_core]$ git diff -- /tmp/gettxoutsetinfo.{b,a}* diff --git a/tmp/gettxoutsetinfo.before b/tmp/gettxoutsetinfo.after index 1f3738c856..6b7b2c9dbe 100644 --- a/tmp/gettxoutsetinfo.before +++ b/tmp/gettxoutsetinfo.after @@ -5,6 +5,6 @@ "txouts": 23926548, "bogosize": 1794967757, "hash_serialized_2": "df4142cdb729b6d302b279e82f6e64563064fa2dcddd29e9b86bf3e4ed02810f", - "disk_size": 1312573410, + "disk_size": 1306908070, "total_amount": 20930150.29591721 }

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.

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.

Done. Force pushed a commit message change:

$ git range-diff bitcoin-core/master  faed13ea16 fa3b4f9b8e
1:  fa6e9b84ef ! 1:  fa1d5800d9 validation: Remove unused boost interruption_point
    @@ Metadata
      ## Commit message ##
         validation: Remove unused boost interruption_point
     
    -    ActivateBestChain is only called in the "msghand" or one of the RPC
    -    threads, neither of which is a boost::thread.
    +    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.
2:  faed13ea16 = 2:  fa3b4f9b8e validation: Make VerifyDB level 4 interruptible

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

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.

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

ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12

I will adjust the commit message accordingly.

Thanks.

Rechecked that a call to verifychain is interrupted:

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

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

This PR

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