Details

Reviewers

deadalnix
markblundeberg

Group Reviewers

Restricted Project

Commits

rSTAGINGe7fc0238127a: Optimization: don't add txn back to mempool after 10 invalidates
rABCe7fc0238127a: Optimization: don't add txn back to mempool after 10 invalidates

Summary

This PR makes a number of improvements to the InvalidateBlock (invalidateblock RPC) and RewindBlockIndex functions, primarily around breaking up their long-term cs_main holding. In addition:

They're made safely interruptible (bitcoind can be shutdown, and no progress in either will be lost, though if incomplete, invalidateblock won't continue after restart and will need to be called again)
The validation queue is prevented from overflowing (meaning invalidateblock on a very old block will not drive bitcoind OOM) (see #14289).
invalidateblock won't bother to move transactions back into the mempool after 10 blocks (optimization).

Partial Backport of Bitcoin Core PR15402 (commit 8d22041):
https://github.com/bitcoin/bitcoin/pull/15402/commits/8d220417cd7bc34464e28a4861a885193ec091c2

Optimization: don't add txn back to mempool after 10 invalidates

Depends on D4803

Test Plan

ninja check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

fpelliccioni created this revision.Dec 26 2019, 19:29

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 26 2019, 19:29

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

fpelliccioni retitled this revision from Granular invalidateblock and RewindBlockIndex to Optimization: don't add txn back to mempool after 10 invalidates.Dec 26 2019, 19:30

fpelliccioni edited the summary of this revision. (Show Details)

fpelliccioni edited the test plan for this revision. (Show Details)

fpelliccioni edited reviewers, added: markblundeberg; removed: Fabien.

fpelliccioni added a parent revision: D4803: Prevent callback overruns in InvalidateBlock and RewindBlockIndex.

Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:
lines=16,COUNTEREXAMPLE```[19:35:44] : [Step 1/1] [19:35:44] : [Step 1/1] [0m [0;34mrpc_decodescript.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_deprecated.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_estimatefee.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_fundrawtransaction.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_getblockstats.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_getchaintips.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_help.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_invalidateblock.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_named_arguments.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_net.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_preciousblock.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_psbt.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_rawtransaction.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_scantxoutset.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_signmessage.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_signrawtransaction.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_txoutproof.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_uptime.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_users.py [19:35:44] : [Step 1/1] [0m [0;34mrpc_zmq.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_abandonconflict.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_basic.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_disable.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_disableprivatekeys.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_disableprivatekeys.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_dump.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_encryption.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_groups.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_hd.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_import_rescan.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_importmulti.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_importprunedfunds.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_keypool.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_keypool_topup.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_labels.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_listreceivedby.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_listsinceblock.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_listtransactions.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_multiwallet.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_multiwallet.py --usecli [19:35:44] : [Step 1/1] [0m [0;34mwallet_resendwallett [19:35:44] : [Step 1/1] [0m [0;34mwallet_txn_clone.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_txn_clone.py --mineblock [19:35:44] : [Step 1/1] [0m [0;34mwallet_txn_doublespend.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_txn_doublespend.py [19:35:44] : [Step 1/1] [0m [0;34mwallet_zapwallettxes.py [19:35:44] : [Step 1/1] [0m [0;31mfeature_notifications.py [19:35:44] : [Step 1/1] [0m [0;31m [1m
[19:35:44] : [Step 1/1] ALL [19:35:44] : [Step 1/1] [0m [0mRuntime: 244 s
[19:35:44] : [Step 1/1]
[19:35:44] : [Step 1/1] * Output of /tmp/saniti [19:35:44]W: [Step 1/1] ++ print_sanitizers_log
[19:35:44]W: [Step 1/1] ++ for log in "${SAN_LOG_D [19:35:44]W: [Step 1/1] ++ echo '* [19:35:44]W: [Step 1/1] ++ cat '/tmp/sanitizer_log [19:35:44]W: [Step 1/1] cat: '/tmp/sanitizer_logs/ [19:35:44]W: [Step 1/1] Process exited with code [19:35:44]E: [Step 1/1] Process exited with code [0m [0;34mrpc_createmultisig.py | ✓ Passed | 2 s
| ✓ Passed | 0 s
| ✓ Passed | 1 s
| ✓ Passed | 0 s
| ✓ Passed | 35 s
| ✓ Passed | 0 s
| ✓ Passed | 2 s
| ✓ Passed | 0 s
| ✓ Passed | 5 s
| ✓ Passed | 0 s
| ✓ Passed | 0 s
| ✓ Passed | 1 s
| ✓ Passed | 7 s
| ✓ Passed | 21 s
| ✓ Passed | 4 s
| ✓ Passed | 0 s
| ✓ Passed | 0 s
| ✓ Passed | 2 s
| ✓ Passed | 0 s
| ✓ Passed | 2 s
| ✓ Passed | 1 s
| ✓ Passed | 7 s
| ✓ Passed | 25 s
| ✓ Passed | 0 s
| ✓ Passed | 0 s
--usecli | ✓ Passed | 0 s
| ✓ Passed | 3 s
| ✓ Passed | 5 s
| ✓ Passed | 8 s
| ✓ Passed | 5 s
| ✓ Passed | 4 s
| ✓ Passed | 2 s
| ✓ Passed | 1 s
| ✓ Passed | 3 s
| ✓ Passed | 3 s
| ✓ Passed | 4 s
| ✓ Passed | 8 s
| ✓ Passed | 3 s
| ✓ Passed | 9 s
| ✓ Passed | 6 s
| ✓ Passed | 6 s
ransactions.py | ✓ Passed | 1 s
| ✓ Passed | 2 s
| ✓ Passed | 1 s
| ✓ Passed | 2 s
--mineblock | ✓ Passed | 1 s
| ✓ Passed | 2 s
| ✖ Failed | 11 s
| ✖ Failed | 429 s (accumulated)
zer_logs/*.log.* *
IR}"/*.log.*
Output of /tmp/sanitizer_logs/*.log.* *'
s/*.log.*'
*.log.*': No such file or directory
1
1 (Step: Command Line)

Each failure log is accessible here:
[[https://build.bitcoinabc.org/viewLog.html?tab=buildLog&logTab=tree&filter=debug&expand=all&buildId=24774&guest=1&_focus=2410 | bitcoin_abc_tests: feature_notifications.py]]

Harbormaster failed remote builds in B8691: Diff 15086!Dec 26 2019, 19:35

Investigate build failure.

This revision now requires changes to proceed.Jan 3 2020, 05:43

fpelliccioni planned changes to this revision.Jan 3 2020, 12:55

rebase

Build Bitcoin-ABC / Diffs / Diff Testing started.

fpelliccioni edited the test plan for this revision. (Show Details)Jan 10 2020, 18:59

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8837: Diff 15302.Jan 10 2020, 19:01

deadalnix requested changes to this revision.Jan 11 2020, 16:58

deadalnix added inline comments.

src/validation.cpp
3072 ↗	(On Diff #15302)	braces

This revision now requires changes to proceed.Jan 11 2020, 16:58

fpelliccioni added a child revision: D4929: Make last disconnected block BLOCK_FAILED_VALID, even when aborted.Jan 13 2020, 18:04

braces

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster failed remote builds in B8907: Diff 15411!Jan 13 2020, 18:17

Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[18:17:18] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git config core.sparseCheckout true
[18:17:18] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git config http.sslCAInfo
[18:17:18] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref
[18:17:18] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref refs/tags/phabricator/diff/15411
[18:17:18] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git log -n1 --pretty=format:%H%x20%s 8edacfeac9f811f606809735c377b913426abf1e --
[18:17:19] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git -c credential.helper= checkout -q -f phabricator/diff/15411
[18:17:19] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref refs/tags/phabricator/diff/15411
[18:17:19] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] Cleaning Bitcoin ABC Staging in /home/teamcity/buildAgent/work/c4a5708f2bae7929 the file set ALL_UNTRACKED
[18:17:19] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git clean -f -d -x
[18:17:19] : Build preparation done
[18:17:19]E: Step 1/1: Command Line
[18:17:19] :	 [Step 1/1] Ant JUnit report watcher
[18:17:19] :		 [Ant JUnit report watcher] Watching paths:
[18:17:19] :		 [Ant JUnit report watcher] +:build/test_bitcoin.xml
[18:17:19] :		 [Ant JUnit report watcher] +:test/functional/junit_results.xml
[18:17:19] :		 [Ant JUnit report watcher] +:build/junit_results*.xml
[18:17:19] :	 [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script3620834589169209390
[18:17:19] :	 [Step 1/1] in directory: /home/teamcity/buildAgent/work/c4a5708f2bae7929
[18:17:19]W:	 [Step 1/1] + : build-diff
[18:17:19] :	 [Step 1/1] Running build configuration 'build-diff'...
[18:17:19]W:	 [Step 1/1] + '[' -z build-diff ']'
[18:17:19]W:	 [Step 1/1] + echo 'Running build configuration '\''build-diff'\''...'
[18:17:19]W:	 [Step 1/1] ++ git rev-parse --show-toplevel
[18:17:19]W:	 [Step 1/1] + TOPLEVEL=/home/teamcity/buildAgent/work/c4a5708f2bae7929
[18:17:19]W:	 [Step 1/1] + export TOPLEVEL
[18:17:19]W:	 [Step 1/1] + trap print_sanitizers_log ERR
[18:17:19]W:	 [Step 1/1] +++ dirname ./contrib/teamcity/build-configurations.sh
[18:17:19]W:	 [Step 1/1] ++ cd ./contrib/teamcity
[18:17:19]W:	 [Step 1/1] ++ pwd
[18:17:19]W:	 [Step 1/1] + CI_SCRIPTS_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/contrib/teamcity
[18:17:19]W:	 [Step 1/1] + setup
[18:17:19]W:	 [Step 1/1] + : /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:17:19]W:	 [Step 1/1] + mkdir -p /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/output
[18:17:19]W:	 [Step 1/1] ++ cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:17:19]W:	 [Step 1/1] ++ pwd
[18:17:19]W:	 [Step 1/1] + BUILD_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:17:19]W:	 [Step 1/1] + export BUILD_DIR
[18:17:19]W:	 [Step 1/1] + TEST_RUNNER_FLAGS=--tmpdirprefix=output
[18:17:19]W:	 [Step 1/1] + cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:17:19]W:	 [Step 1/1] ++ nproc
[18:17:19]W:	 [Step 1/1] + THREADS=12
[18:17:19]W:	 [Step 1/1] + export THREADS
[18:17:19]W:	 [Step 1/1] + SAN_SUPP_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions
[18:17:19]W:	 [Step 1/1] + SAN_LOG_DIR=/tmp/sanitizer_logs
[18:17:19]W:	 [Step 1/1] + mkdir -p /tmp/sanitizer_logs
[18:17:19]W:	 [Step 1/1] + rm -rf '/tmp/sanitizer_logs/*'
[18:17:19]W:	 [Step 1/1] + export ASAN_OPTIONS=malloc_context_size=0:log_path=/tmp/sanitizer_logs/asan.log
[18:17:19]W:	 [Step 1/1] + ASAN_OPTIONS=malloc_context_size=0:log_path=/tmp/sanitizer_logs/asan.log
[18:17:19]W:	 [Step 1/1] + export LSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/lsan:log_path=/tmp/sanitizer_logs/lsan.log
[18:17:19]W:	 [Step 1/1] + LSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/lsan:log_path=/tmp/sanitizer_logs/lsan.log
[18:17:19]W:	 [Step 1/1] + export TSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/tsan:log_path=/tmp/sanitizer_logs/tsan.log
[18:17:19] :	 [Step 1/1] Error: Invalid build name 'build-diff'
[18:17:19]W:	 [Step 1/1] + TSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/tsan:log_path=/tmp/sanitizer_logs/tsan.log
[18:17:19]W:	 [Step 1/1] + export UBSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:log_path=/tmp/sanitizer_logs/ubsan.log
[18:17:19]W:	 [Step 1/1] + UBSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:log_path=/tmp/sanitizer_logs/ubsan.log
[18:17:19]W:	 [Step 1/1] + case "$ABC_BUILD_NAME" in
[18:17:19]W:	 [Step 1/1] + echo 'Error: Invalid build name '\''build-diff'\'''
[18:17:19]W:	 [Step 1/1] + exit 2
[18:17:19]W:	 [Step 1/1] Process exited with code 2
[18:17:19]E:	 [Step 1/1] Process exited with code 2 (Step: Command Line)

rebased

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8947: Diff 15469.Jan 14 2020, 21:34

I'm not 100% convinced we want this, but so be it. We can always remove it later.

This revision is now accepted and ready to land.Jan 15 2020, 15:42

rebase from master

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8961: Diff 15489.Jan 15 2020, 17:36

Closed by commit rABCe7fc0238127a: Optimization: don't add txn back to mempool after 10 invalidates (authored by Pieter Wuille <pieter.wuille@gmail.com>, committed by fpelliccioni). · Explain WhyJan 15 2020, 17:37

This revision was automatically updated to reflect the committed changes.