This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
client/network: Use request response for block requests #7478
Merged
Merged
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
69ef3fa
client/network: Add scaffolding for finality req to use req resp
mxinden e8df535
client/network/src/finality_requests: Remove
mxinden 3207c53
client/network/src/behaviour: Pass request id down to sync
mxinden e3ef56e
client/network: Use request response for block requests
mxinden 544ebfe
client/network: Move handler logic into *_*_handler.rs
mxinden 8abd813
client/network: Track ongoing finality requests in protocol.rs
mxinden 8da90c5
Merge branch 'paritytech/master' into use-request-response
mxinden 45de155
client/network: Remove commented out finalization initialization
mxinden 456568f
client/network: Add docs for request handlers
mxinden 1a6b00e
client/network/finality_request_handler: Log errors
mxinden 06dbd98
client/network/block_request_handler: Log errors
mxinden 25e298f
client/network: Format
mxinden 2ed6dd1
client/network: Handle block request failure
mxinden aacb507
protocols/network: Fix tests
mxinden 1a72e0c
client/network/src/behaviour: Handle request sending errors
mxinden 1385f5c
client/network: Move response handling into custom method
mxinden 8fdf1bd
client/network/protocol: Handle block response errors
mxinden 7460a21
client/network/protocol: Remove tracking of obsolete requests
mxinden f1e7691
client/network/protocol: Remove block request start time tracking
mxinden 401a7f8
client/network/protocol: Refactor on_*_request_started
mxinden b375b84
client/network: Pass protocol config instead of protocol name
mxinden 107aa83
client/network: Pass protocol config in tests
mxinden 0535723
client/network/config: Document request response configs
mxinden 7a3d71e
client/network/src/_request_handler: Document protocol config gen
mxinden 64d2222
client/network/src/protocol: Document Peer request values
mxinden 9560e32
client/network: Rework request response to always use oneshot
mxinden 8b7b32d
client/network: Unified metric reporting for all request protocols
mxinden 109fa3f
Merge branch 'paritytech/master' into use-request-response
mxinden d6a6a52
Merge branch 'paritytech/master' into use-request-response
mxinden 1cd8a71
client/network: Move protobuf parsing into protocol.rs
mxinden d5ea27e
client/network/src/protocol: Return pending events after poll
mxinden 9bfa9ca
Merge branch 'paritytech/master' into use-request-response
mxinden 006fd6e
client/network: Improve error handling and documentation
mxinden 1bca347
client/network/behaviour: Remove outdated error types
mxinden 2797595
Update client/network/src/block_request_handler.rs
wheresaddie aaa981c
Update client/network/src/finality_request_handler.rs
wheresaddie d1361f2
Merge branch 'paritytech/master' into use-request-response
mxinden c19885e
client/network/protocol: Reduce reputation on timeout
mxinden 81cadfc
client/network/protocol: Refine reputation changes
mxinden 9236113
client/network/block_request_handler: Set and explain queue length
mxinden 4c018f1
client/service: Deny block requests when light client
mxinden 07c0d2f
client/service: Fix role matching
mxinden 8c8bcb2
client: Enforce line width
mxinden 9481e6d
client/network/request_responses: Fix unit tests
mxinden 707c1bd
client/network: Expose time to build response via metrics
mxinden e05c1ad
Merge branch 'paritytech/master' into use-request-response
mxinden df5907f
client/network/request_responses: Fix early connection closed error
mxinden 4096019
Merge branch 'paritytech/master' into use-request-response
mxinden 24e14c8
Merge branch 'paritytech/master' into use-request-response
mxinden 94a1a79
client/network/protocol: Fix line length
mxinden db776ca
Merge branch 'paritytech/master' into use-request-response
mxinden 8e846e0
client/network/protocol: Disconnect on most request failures
mxinden 4c45b4b
Merge branch 'paritytech/master' into use-request-response
mxinden 5277841
Merge branch 'paritytech/master' into use-request-response
mxinden bccc53e
Merge branch 'paritytech/master' into use-request-response
mxinden e0ae207
Merge branch 'paritytech/master' into use-request-response
mxinden d2c5161
client/network/protocol: Disconnect peer when oneshot is canceled
mxinden cc1bff0
Merge branch 'paritytech/master' into use-request-response
mxinden 9acc7ee
Merge branch 'paritytech/master' into use-request-response
mxinden 34d0351
client/network/protocol: Disconnect peer even when connection closed
mxinden 2cb76ef
client/network/protocol: Remove debugging log line
mxinden d525b96
client/network/request_response: Use Clone::clone for error
mxinden 1da9269
client/network/request_response: Remove outdated comment
mxinden File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
client/service: Deny block requests when light client
- Loading branch information
commit 4c018f174dba29ed11b3231e0100cb44cbc86615
There are no files selected for viewing
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
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
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
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metric was there to potentially detect requests that are stuck, or still in progress in general.
I don't necessarily want to keep it, but what's the reason for removing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit that I dropped it with removing block_requests.rs but never reintroduced it when switching to request-responses.rs.
Without the metric one can not tell how many requests are in-flight at a given point in time, though one can estimate it via
requests_out_success_total. Given thatrequest_responses.rsdoes not spawn new connections, requests to the same peer are done in sequence, capped with a limit and refused requests (requests over the limit) are reported inrequests_out_failure_totalI am not sure we still need it.It does not take a lot of work to reimplement it, nor to maintain it, thus I am fine reintroducing the metric. Let me know what you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really care one way or the other. The metric is only useful in the presence of a bug, so it's a matter of being optimistic or pessimistic.