Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Aug 23, 2024

Fixes #30704

The goal of the test is to check the stalling timeout, not the block download timeout.

On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 23, 2024

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK tdb3, brunoerg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Aug 23, 2024
Copy link
Contributor

@tdb3 tdb3 left a comment

Choose a reason for hiding this comment

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

CR ACK fa5b58e

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

utACK fa5b58e

@fanquake fanquake merged commit e53b1c1 into bitcoin:master Aug 27, 2024
16 checks passed
@maflcko maflcko deleted the 2408-test-stall-not-timeout branch August 27, 2024 11:36
fanquake added a commit that referenced this pull request Sep 2, 2024
…th_minchainwork.py

fa247e6 test: Avoid intermittent timeout in p2p_headers_sync_with_minchainwork.py (MarcoFalke)

Pull request description:

  Similar to #30705:

  The goal of this test case is to check that the sync works at all, not to check any timeout.

  On extremely slow hardware (for example qemu virtual hardware), downloading the 4110 BLOCKS_TO_MINE may take longer than the block download timeout.

  Fix it by pinning the time using mocktime temporarily, and advance it immediately after the sync.

ACKs for top commit:
  stratospher:
    ACK fa247e6. Checked the timeout downloading block logs before/after using `setmocktime`.
  tdb3:
    ACK fa247e6

Tree-SHA512: f61632a8d9e484f1b888aafbf87f7adf71b8692387bd77f603cdbc0de49f30d42e654741d46ae1ff8b9706a5559ee0faabdb192ed0db7449010b68bfcdbaa42d
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 25, 2024
…in p2p_ibd_stalling

fa5b58e test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)

Pull request description:

  Fixes bitcoin#30704

  The goal of the test is to check the stalling timeout, not the block download timeout.

  On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

  Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.

ACKs for top commit:
  tdb3:
    CR ACK fa5b58e
  brunoerg:
    utACK fa5b58e

Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 26, 2024
…in p2p_ibd_stalling

fa5b58e test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)

Pull request description:

  Fixes bitcoin#30704

  The goal of the test is to check the stalling timeout, not the block download timeout.

  On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

  Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.

ACKs for top commit:
  tdb3:
    CR ACK fa5b58e
  brunoerg:
    utACK fa5b58e

Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 27, 2024
…in p2p_ibd_stalling

fa5b58e test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)

Pull request description:

  Fixes bitcoin#30704

  The goal of the test is to check the stalling timeout, not the block download timeout.

  On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

  Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.

ACKs for top commit:
  tdb3:
    CR ACK fa5b58e
  brunoerg:
    utACK fa5b58e

Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 27, 2024
b654479 Merge bitcoin#30705: test: Avoid intermittent block download timeout in p2p_ibd_stalling (merge-script)
745a819 Merge bitcoin#30690: devtools, utxo-snapshot: Fix block height out of range in script (Ava Chow)
01b570e Merge bitcoin#29999: guix: fix suggested fake date for openssl-1.1.1l (Ava Chow)
432f352 Merge bitcoin#30580: doc: Add note about distro's `g++-mingw-w64-x86-64-posix` version (merge-script)
1bd090e Merge bitcoin#30597: doc: Drop no longer needed workaround for WSL (merge-script)
8a12237 Merge bitcoin#30630: doc: Update ccache website link (merge-script)
f66547f Merge bitcoin#30588: depends: fix ZMQ CMake getcachesize check (merge-script)
ddaec96 Merge bitcoin#30565: depends: Fix `zeromq` build on OpenBSD (merge-script)
e4e5605 Merge bitcoin#30552: test: fix constructor of msg_tx (merge-script)
df3c239 Merge bitcoin#26950: cleanse: switch to SecureZeroMemory for Windows cross-compile (merge-script)
57945ce Merge bitcoin#30506: depends: Cleanup postprocess commands after switching to CMake (merge-script)
e016ffa Merge bitcoin#29878: depends: build expat with CMake (merge-script)
62dcd43 Merge bitcoin#29880: depends: build FreeType with CMake (merge-script)
745addf Merge bitcoin#30245: net: Allow -proxy=[::1] on nodes with IPV6 lo only (Ava Chow)
4e144be Merge bitcoin-core/gui#795: Keep focus on "Hide" while ModalOverlay is visible (Hennadii Stepanov)
69c04b2 Merge bitcoin#30372: util: Use SteadyClock in RandAddSeedPerfmon (merge-script)
ebed8af Merge bitcoin#30336: depends: update doc in Qt pwd patch (merge-script)
9793fb1 Merge bitcoin#30340: test: Added coverage to Block not found error using gettxoutsetinfo (Ava Chow)
479cb8b Merge bitcoin#30312: contrib: add R(UN)PATH check to ELF symbol-check (merge-script)
ca83773 Merge bitcoin#30283: upnp: fix build with miniupnpc 2.2.8 (merge-script)
63e139d Merge bitcoin#30185: guix: show `*_FLAGS` variables in pre-build output (merge-script)
3be0d3e Merge bitcoin#30097: crypto: disable asan for sha256_sse4 with clang and -O0 (merge-script)
3070c3e Merge bitcoin#30078: depends: set AR & RANLIB for CMake (merge-script)

Pull request description:

  ## Issue being fixed or feature implemented
  Trivial backports

  ## What was done?

  ## How Has This Been Tested?
  built locally

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK b654479
  kwvg:
    utACK b654479

Tree-SHA512: 10b5af4e92c83fa9d6764b20bf066bba8e4c600402966fd5c1d6dad07b0549d8a42151a33f21e2f8263336c12a810a6f3fc2828d90bc98153e09c165d9e5b043
@maflcko
Copy link
Member Author

maflcko commented Nov 28, 2024

Fixup in #31383

Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 21, 2024
Summary:
```
The goal of the test is to check the stalling timeout, not the block download timeout.

On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.
```

Backport of [[bitcoin/bitcoin#30705 | core#30705]].

Test Plan:
  ./test/functional/test_runner.py p2p_ibd_stalling

Reviewers: #bitcoin_abc, roqqit

Reviewed By: roqqit

Differential Revision: https://reviews.bitcoinabc.org/D17382
roqqit pushed a commit to doged-io/doged that referenced this pull request Jan 7, 2025
Summary:
```
The goal of the test is to check the stalling timeout, not the block download timeout.

On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.
```

Backport of [[bitcoin/bitcoin#30705 | core#30705]].

Test Plan:
  ./test/functional/test_runner.py p2p_ibd_stalling

Reviewers: #bitcoin_abc, roqqit

Reviewed By: roqqit

Differential Revision: https://reviews.bitcoinabc.org/D17382
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent timeout on p2p_ibd_stalling.py
5 participants