pd: remove client sync timeouts #2932
Merged
Conversation
This file contains 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
|
Spinning up a node to test this prior to merge. |
conorsch
requested changes
Aug 14, 2023
There was a problem hiding this comment.
Suggesting a higher timeout, if you don't have objections. The process I used to test was:
- create fullnode with https rpc access
- cherry-pick these commits on top of v0.58.1
- bounce pd
- run
pcli -n <node> view syncthree times
2s wasn't enough, 10s was enough, seems like a happy medium for now. I think we're both eager to wash our hands of the timeout tedium. If you +1 this change, I'm happy to get it out the door.
erwanor
changed the title
pd: re-enable req-resp timeout pd(oblivious): increase timeout to 2s pd: fix type inferrence error
conorsch
force-pushed
the
longer-timeout
branch
from
ca11ed1 to
00040bc
Compare
|
I've rebased the changes here into a single commit for the timeout-removal, so it's more straightforward to cherry-pick into a patch release branch. Prior to merge, I'll test again on a dedicated node, and confirm pcli sync behavior is sound. |
conorsch
approved these changes
Aug 16, 2023
|
Was able to sync pcli three times to latest block height. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Part of #2918 (cc @conorsch ). We have isolated that the stream terminates because of the timeout on polling compact blocks. This PR reenable the req-resp timeout and increase the polling timeout to
2s.