net: introduce block tracker to retry to download blocks after failure #27837
+330
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Jun 8, 2023
|
Maybe mark as draft if CI is red and this is still based on something else? |
|
Oh, didn't know about the CI failure. Seems to be just an unused variable. But sure, draft until #27836 sounds good. |
This was referenced Jul 5, 2023
This was referenced Jul 22, 2023
1311875 to
aab7a2f
Compare
This was referenced Aug 2, 2023
This was referenced Aug 15, 2023
Merged
This was referenced Sep 15, 2023
fe0f9be to
d589be0
Compare
achow101
added a commit
that referenced
this pull request
May 23, 2024
6629d1d test: improve robustness of connect_nodes() (furszy) Pull request description: Decoupled from #27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200. The `connect_nodes` function in the test framework relies on a stable number of peer connections to verify that the new connection between the nodes is successfully established. This approach is fragile, as any of the peers involved in the process can drop, lose, or create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely even when the peers in question were connected successfully. This commit improves the situation by using the nodes' subversion and the connection direction (inbound/outbound) to identify the exact peer connection and perform the checks exclusively on it. ACKs for top commit: stratospher: reACK 6629d1d. achow101: ACK 6629d1d maflcko: utACK 6629d1d AngusP: re-ACK 6629d1d Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
d589be0 to
0545d24
Compare
|
🚧 At least one of the CI tasks failed. HintsMake sure to run all tests locally, according to the documentation. The failure may happen due to a number of reasons, for example:
Leave a comment here, if you need help tracking down a confusing failure. |
0545d24 to
8c4f665
Compare
8c4f665 to
c31233a
Compare
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Oct 3, 2024
6629d1d test: improve robustness of connect_nodes() (furszy) Pull request description: Decoupled from bitcoin#27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200. The `connect_nodes` function in the test framework relies on a stable number of peer connections to verify that the new connection between the nodes is successfully established. This approach is fragile, as any of the peers involved in the process can drop, lose, or create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely even when the peers in question were connected successfully. This commit improves the situation by using the nodes' subversion and the connection direction (inbound/outbound) to identify the exact peer connection and perform the checks exclusively on it. ACKs for top commit: stratospher: reACK 6629d1d. achow101: ACK 6629d1d maflcko: utACK 6629d1d AngusP: re-ACK 6629d1d Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
If the initial block fetching process fails, the p2p layer will be in charge of fetching the block from 'any' connected peer. Re-trying to download the block from different peers until it is received.
If no 'peer_id' is provided, 'getblockfrompeer' will just delegate the peer selection to the internal block downloading logic.
Allowing what we had before, a single block request with no automatic retry nor tracking mechanism.
c31233a to
b18c72c
Compare
panleone
pushed a commit
to panleone/PIVX
that referenced
this pull request
Nov 11, 2024
6629d1d test: improve robustness of connect_nodes() (furszy) Pull request description: Decoupled from bitcoin#27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200. The `connect_nodes` function in the test framework relies on a stable number of peer connections to verify that the new connection between the nodes is successfully established. This approach is fragile, as any of the peers involved in the process can drop, lose, or create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely even when the peers in question were connected successfully. This commit improves the situation by using the nodes' subversion and the connection direction (inbound/outbound) to identify the exact peer connection and perform the checks exclusively on it. ACKs for top commit: stratospher: reACK 6629d1d. achow101: ACK 6629d1d maflcko: utACK 6629d1d AngusP: re-ACK 6629d1d Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
This was referenced Dec 30, 2024
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5483
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Coming from #27652, part of #29183.
The general idea is to keep track of the user requested blocks so, in
case of a bad behaving peer or a network disconnection, they can be
fetched from another one automatically without any further user interaction.
This was requested by users because the
getblockfrompeerRPC commandlacks the functionality to notify them about block request failures or peer
disconnections (which is expected due to the asynchronous nature of the block
requests).
Currently, this new functionality is limited to blocks requested by the
user via the 'getblockfrompeer' RPC command.
In the future, this class could expand its scope and be utilized in the
regular chain synchronization process. Or, even could be employed in
special procedures like a prune node rescan that uses BIP158 block filters,
or even into BIP157 itself.