-
Notifications
You must be signed in to change notification settings - Fork 690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use maximum allowed response size for request/response protocols #5503
Labels
I5-enhancement
An additional feature request.
Comments
As mentioned by @dmitry-markin in #5334 : We should indeed double-check timeout values whether they are still fine. |
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 19, 2024
# Description Adjust the PoV response size to the default values used in the substrate. Fixes #5503 ## Integration The changes shouldn't impact downstream projects since we are only increasing the limit. ## Review Notes You can't see it from the changes, but it affects all protocols that use the `POV_RESPONSE_SIZE` constant. - Protocol::ChunkFetchingV1 - Protocol::ChunkFetchingV2 - Protocol::CollationFetchingV1 - Protocol::CollationFetchingV2 - Protocol::PoVFetchingV1 - Protocol::AvailableDataFetchingV1 ## Increasing timeouts https://github.com/paritytech/polkadot-sdk/blob/fae15379cba0c876aa16c77e11809c83d1db8f5c/polkadot/node/network/protocol/src/request_response/mod.rs#L126-L129 I assume the current PoV request timeout is set to 1.2s to handle 5 consecutive requests during a 6s block. This setting does not relate to the PoV response size. I see no reason to change the current timeouts after adjusting the response size. However, we should consider networking speed limitations if we want to increase the maximum PoV size to 10 MB. With the number of parallel requests set to 10, validators will need the following networking speeds: - 5 MB PoV: at least 42 MB/s, ideally 50 MB/s. - 10 MB PoV: at least 84 MB/s, ideally 100 MB/s. The current required speed of 50 MB/s aligns with the 62.5 MB/s specified [in the reference hardware requirements](https://wiki.polkadot.network/docs/maintain-guides-how-to-validate-polkadot#reference-hardware). Increasing the PoV size to 10 MB may require a higher networking speed. --------- Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>
github-actions bot
pushed a commit
that referenced
this issue
Sep 30, 2024
# Description Adjust the PoV response size to the default values used in the substrate. Fixes #5503 ## Integration The changes shouldn't impact downstream projects since we are only increasing the limit. ## Review Notes You can't see it from the changes, but it affects all protocols that use the `POV_RESPONSE_SIZE` constant. - Protocol::ChunkFetchingV1 - Protocol::ChunkFetchingV2 - Protocol::CollationFetchingV1 - Protocol::CollationFetchingV2 - Protocol::PoVFetchingV1 - Protocol::AvailableDataFetchingV1 ## Increasing timeouts https://github.com/paritytech/polkadot-sdk/blob/fae15379cba0c876aa16c77e11809c83d1db8f5c/polkadot/node/network/protocol/src/request_response/mod.rs#L126-L129 I assume the current PoV request timeout is set to 1.2s to handle 5 consecutive requests during a 6s block. This setting does not relate to the PoV response size. I see no reason to change the current timeouts after adjusting the response size. However, we should consider networking speed limitations if we want to increase the maximum PoV size to 10 MB. With the number of parallel requests set to 10, validators will need the following networking speeds: - 5 MB PoV: at least 42 MB/s, ideally 50 MB/s. - 10 MB PoV: at least 84 MB/s, ideally 100 MB/s. The current required speed of 50 MB/s aligns with the 62.5 MB/s specified [in the reference hardware requirements](https://wiki.polkadot.network/docs/maintain-guides-how-to-validate-polkadot#reference-hardware). Increasing the PoV size to 10 MB may require a higher networking speed. --------- Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> (cherry picked from commit 0c9d8fe)
github-actions bot
pushed a commit
that referenced
this issue
Sep 30, 2024
# Description Adjust the PoV response size to the default values used in the substrate. Fixes #5503 ## Integration The changes shouldn't impact downstream projects since we are only increasing the limit. ## Review Notes You can't see it from the changes, but it affects all protocols that use the `POV_RESPONSE_SIZE` constant. - Protocol::ChunkFetchingV1 - Protocol::ChunkFetchingV2 - Protocol::CollationFetchingV1 - Protocol::CollationFetchingV2 - Protocol::PoVFetchingV1 - Protocol::AvailableDataFetchingV1 ## Increasing timeouts https://github.com/paritytech/polkadot-sdk/blob/fae15379cba0c876aa16c77e11809c83d1db8f5c/polkadot/node/network/protocol/src/request_response/mod.rs#L126-L129 I assume the current PoV request timeout is set to 1.2s to handle 5 consecutive requests during a 6s block. This setting does not relate to the PoV response size. I see no reason to change the current timeouts after adjusting the response size. However, we should consider networking speed limitations if we want to increase the maximum PoV size to 10 MB. With the number of parallel requests set to 10, validators will need the following networking speeds: - 5 MB PoV: at least 42 MB/s, ideally 50 MB/s. - 10 MB PoV: at least 84 MB/s, ideally 100 MB/s. The current required speed of 50 MB/s aligns with the 62.5 MB/s specified [in the reference hardware requirements](https://wiki.polkadot.network/docs/maintain-guides-how-to-validate-polkadot#reference-hardware). Increasing the PoV size to 10 MB may require a higher networking speed. --------- Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> (cherry picked from commit 0c9d8fe)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
... related to #5334
The maximum response size for fetching PoVs and collations is hardcoded and needs to be updated before we can increase the on-chain maximum pov size:
polkadot-sdk/polkadot/node/network/protocol/src/request_response/mod.rs
Line 220 in 7e7c334
We should remove the hardcoding and use the limit in substrate networking:
polkadot-sdk/substrate/client/network/sync/src/block_request_handler.rs
Line 92 in 717bbb2
The text was updated successfully, but these errors were encountered: