Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bump PoV request timeout #5885

Closed
AndreiEres opened this issue Oct 1, 2024 · 0 comments · Fixed by #5924
Closed

Bump PoV request timeout #5885

AndreiEres opened this issue Oct 1, 2024 · 0 comments · Fixed by #5924
Assignees
@AndreiEres

Comments

@AndreiEres
Copy link
Contributor

AndreiEres commented Oct 1, 2024 •
edited
Loading

The current PoV request timeout is set to 1.2s because of synchronous backing. There we have a very small overall time budget. This is no longer true for asynchronous backing.

Considering that this is a hard timeout where we completely drop the response if exceeded, it might make sense to think about bumping it a bit. This would add robustness, especially with concurrent requests. Given that a single fetch should take around 200ms it probably is still ok. But there is no harm in bumping though. If we have 10 parallel requests, than the timeout should be around 2s.

In any case let's test this with Gluttons on Kusama, Lot's of 10MB PoVs ... let's see what happens.

Edited comment by @eskimor in #5753 (comment)
@AndreiEres AndreiEres self-assigned this Oct 4, 2024
@AndreiEres AndreiEres mentioned this issue Oct 4, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 9, 2024
@AndreiEres
Bump PoV request timeout (#5924)
3ad1291 
# Description

We previously set the PoV request timeout to 1.2s based on synchronous
backing, which allowed for 5 PoVs per relay block. With asynchronous
backing, we no longer have a time budget and can increase the value to
2s.

Fixes #5885

## Integration

This PR shouldn't affect downstream projects.

## Review Notes

This PR can be followed by experiments with Gluttons on Kusama to
confirm that the timeout is sufficient.
github-actions bot pushed a commit that referenced this issue Nov 4, 2024
@AndreiEres @github-actions
Bump PoV request timeout (#5924)
f5594f1 
# Description

We previously set the PoV request timeout to 1.2s based on synchronous
backing, which allowed for 5 PoVs per relay block. With asynchronous
backing, we no longer have a time budget and can increase the value to
2s.

Fixes #5885

## Integration

This PR shouldn't affect downstream projects.

## Review Notes

This PR can be followed by experiments with Gluttons on Kusama to
confirm that the timeout is sufficient.

(cherry picked from commit 3ad1291)
github-actions bot pushed a commit that referenced this issue Nov 4, 2024
@AndreiEres @github-actions
Bump PoV request timeout (#5924)
94d69f0 
# Description

We previously set the PoV request timeout to 1.2s based on synchronous
backing, which allowed for 5 PoVs per relay block. With asynchronous
backing, we no longer have a time budget and can increase the value to
2s.

Fixes #5885

## Integration

This PR shouldn't affect downstream projects.

## Review Notes

This PR can be followed by experiments with Gluttons on Kusama to
confirm that the timeout is sufficient.

(cherry picked from commit 3ad1291)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AndreiEres AndreiEres

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bump PoV request timeout paritytech/polkadot-sdk
1 participant
@AndreiEres