Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Feb 28, 2020

The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

@maflcko
Copy link
Member Author

maflcko commented Feb 28, 2020

This was easier to debug thanks to f9abf4a by @jkczyz

@fanquake fanquake added the Tests label Feb 28, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 28, 2020

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

Conflicts

No conflicts as of last run.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in #12217 and to wallet_balance.py in #16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Is this an appveyor-only or Win-only issue? I was unable to reproduce locally on Debian; 200 test runs passed without and without the change:

$ (for i in {1..200}; do test/functional/wallet_resendwallettransactions.py --pdbonfailure ; done)

@maflcko
Copy link
Member Author

maflcko commented Feb 29, 2020

I was unable to reproduce locally

Those races generally only become visible when one of the threads (e.g scheduler thread) gets paused for a long time on a CPU due to congestion.

You might be able to reproduce locally by adding a MilliSleep pause in the code of the validation interface.

@luke-jr
Copy link
Member

luke-jr commented Mar 4, 2020

Fixes a bug introduced in #15646 IIUC

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Mar 5, 2020
@maflcko maflcko merged commit e2d3663 into bitcoin:master Mar 12, 2020
@maflcko maflcko deleted the 2002-testFixRace branch March 12, 2020 13:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 13, 2020
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 7, 2021
Summary:
> The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

This is a backport of Core [[bitcoin/bitcoin#18228 | PR18228]]

Test Plan: test/functional/test_runner.py wallet_resend*

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8830
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 28, 2021
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants