Page MenuHomePhabricator

Make last disconnected block BLOCK_FAILED_VALID, even when aborted
ClosedPublic

Authored by fpelliccioni on Jan 13 2020, 18:01.

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 519b0bc):
https://github.com/bitcoin/bitcoin/pull/15402/commits/519b0bc5dc5155b6f7e2362c2105552bb7618ad0

Make last disconnected block BLOCK_FAILED_VALID, even when aborted

Depends on D4804

Test Plan
ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D4929
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9077
Build 16116: Default Diff Build & Tests
Build 16115: arc lint + arc unit

Event Timeline

fpelliccioni retitled this revision from Granular invalidateblock and RewindBlockIndex to Make last disconnected block BLOCK_FAILED_VALID, even when aborted.Jan 13 2020, 18:04
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.
deadalnix requested changes to this revision.Jan 15 2020, 15:45
deadalnix added inline comments.
src/validation.cpp
3097 ↗(On Diff #15410)

I don't think that this is correct.

This revision now requires changes to proceed.Jan 15 2020, 15:45

Snippet of first build failure:

[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git config lfs.storage /home/teamcity/buildAgent/system/git/git-48AA3180.git/lfs
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git config core.sparseCheckout true
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git config http.sslCAInfo
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref refs/tags/phabricator/diff/15494
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git log -n1 --pretty=format:%H%x20%s 6c498e20bb715b3a87f5bfc02b32716e8b6d0b6a --
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git -c credential.helper= checkout -q -f phabricator/diff/15494
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git show-ref refs/tags/phabricator/diff/15494
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] Cleaning Bitcoin ABC Staging in /home/teamcity/buildAgent/work/c4a5708f2bae7929 the file set ALL_UNTRACKED
[18:08:06] :			 [Update checkout directory (/home/teamcity/buildAgent/work/c4a5708f2bae7929)] /usr/bin/git clean -f -d -x
[18:08:06] : Build preparation done
[18:08:06]E: Step 1/1: Command Line
[18:08:06] :	 [Step 1/1] Ant JUnit report watcher
[18:08:06] :		 [Ant JUnit report watcher] Watching paths:
[18:08:06] :		 [Ant JUnit report watcher] +:build/test_bitcoin.xml
[18:08:06] :		 [Ant JUnit report watcher] +:build/**/junit_results*.xml
[18:08:06] :	 [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script4094997215296407092
[18:08:06] :	 [Step 1/1] in directory: /home/teamcity/buildAgent/work/c4a5708f2bae7929
[18:08:06]W:	 [Step 1/1] + : build-diff
[18:08:06] :	 [Step 1/1] Running build configuration 'build-diff'...
[18:08:06]W:	 [Step 1/1] + '[' -z build-diff ']'
[18:08:06]W:	 [Step 1/1] + echo 'Running build configuration '\''build-diff'\''...'
[18:08:06]W:	 [Step 1/1] ++ git rev-parse --show-toplevel
[18:08:06]W:	 [Step 1/1] + TOPLEVEL=/home/teamcity/buildAgent/work/c4a5708f2bae7929
[18:08:06]W:	 [Step 1/1] + export TOPLEVEL
[18:08:06]W:	 [Step 1/1] + trap print_sanitizers_log ERR
[18:08:06]W:	 [Step 1/1] +++ dirname ./contrib/teamcity/build-configurations.sh
[18:08:06]W:	 [Step 1/1] ++ cd ./contrib/teamcity
[18:08:06]W:	 [Step 1/1] ++ pwd
[18:08:06]W:	 [Step 1/1] + CI_SCRIPTS_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/contrib/teamcity
[18:08:06]W:	 [Step 1/1] + setup
[18:08:06]W:	 [Step 1/1] + : /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:08:06]W:	 [Step 1/1] + mkdir -p /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/output
[18:08:06]W:	 [Step 1/1] ++ cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:08:06]W:	 [Step 1/1] ++ pwd
[18:08:06]W:	 [Step 1/1] + BUILD_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:08:06]W:	 [Step 1/1] + export BUILD_DIR
[18:08:06]W:	 [Step 1/1] + TEST_RUNNER_FLAGS=--tmpdirprefix=output
[18:08:06]W:	 [Step 1/1] + cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build
[18:08:06]W:	 [Step 1/1] ++ nproc
[18:08:06]W:	 [Step 1/1] + THREADS=4
[18:08:06]W:	 [Step 1/1] + export THREADS
[18:08:06]W:	 [Step 1/1] + SAN_SUPP_DIR=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions
[18:08:06]W:	 [Step 1/1] + SAN_LOG_DIR=/tmp/sanitizer_logs
[18:08:06]W:	 [Step 1/1] + mkdir -p /tmp/sanitizer_logs
[18:08:06]W:	 [Step 1/1] + rm -rf '/tmp/sanitizer_logs/*'
[18:08:06]W:	 [Step 1/1] + export ASAN_OPTIONS=malloc_context_size=0:log_path=/tmp/sanitizer_logs/asan.log
[18:08:06]W:	 [Step 1/1] + ASAN_OPTIONS=malloc_context_size=0:log_path=/tmp/sanitizer_logs/asan.log
[18:08:06]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:08:06]W:	 [Step 1/1] + LSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/lsan:log_path=/tmp/sanitizer_logs/lsan.log
[18:08:06]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:08:06]W:	 [Step 1/1] + TSAN_OPTIONS=suppressions=/home/teamcity/buildAgent/work/c4a5708f2bae7929/test/sanitizer_suppressions/tsan:log_path=/tmp/sanitizer_logs/tsan.log
[18:08:06]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:08:06]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:08:06] :	 [Step 1/1] Error: Invalid build name 'build-diff'
[18:08:06]W:	 [Step 1/1] + case "$ABC_BUILD_NAME" in
[18:08:06]W:	 [Step 1/1] + echo 'Error: Invalid build name '\''build-diff'\'''
[18:08:06]W:	 [Step 1/1] + exit 2
[18:08:06]W:	 [Step 1/1] Process exited with code 2
[18:08:06]E:	 [Step 1/1] Process exited with code 2 (Step: Command Line)
deadalnix requested changes to this revision.Jan 17 2020, 01:51

See build failure.

This revision now requires changes to proceed.Jan 17 2020, 01:51

From what I can tell, this change is actually a bugfix and should have really been combined with D4802. Please get this landed soon.

markblundeberg added inline comments.
src/validation.cpp
3105 ↗(On Diff #15494)

Note: we don't have BLOCK_FAILED_CHILD (instead FailedParent), and also these comments need to mention parking with equal weight.

OK, looks good. It looks like there are more backports to do, at least https://github.com/bitcoin/bitcoin/pull/16849 (possibly others too).

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