Page MenuHomePhabricator

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

Authored by fpelliccioni on Dec 26 2019, 19:29.

Details

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 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.

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

Investigate build failure.

This revision now requires changes to proceed.Jan 3 2020, 05:43
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

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)

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