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.

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

eth,p2p: count timeout packet towards rtt #25588

Closed
wants to merge 1 commit into from

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Aug 24, 2022

This PR changes the way we calculate RTT a bit. I noticed that for some nodes (particularly our azure nodes), there are a lot of dropped/failed trie heal messages.

Whenever we time out a packet, we update the capacity for that particular kind to 0. But what we do not do is update the rtt estimate. This PR changes that, so that a failed delivery does update the RTT, by pretending that it completed in 2x the amount of time spent so far. So if it times out after two seconds, it RTT estimate is updated as if it had been successfully delivered after 4 seconds. The idea is to make timeouts bump up the RTT estimate.

This is from one of the bootnodes, which is syncing. The graphs shows Unexpected trienode heal occurrences in the log. I restarted it with this PR at 11:50, when if flattens out. The dotted line represents 500 messages.
Screenshot 2022-08-24 at 12-04-56 bootnode-azure-koreasouth-001 - Papertrail
It's only one isolated and perhaps misrepresenting metric, but in that instance it seems to have improved the misdeliveries.

@holiman
Copy link
Contributor Author

holiman commented Aug 24, 2022

After some more time: this PR does not seem to make much of an impact on the log messages:

Screenshot 2022-08-24 at 13-05-33 bootnode-azure-koreasouth-001 - Papertrail

As for the grafana chart, I'm not sure how to interpret it:
Screenshot 2022-08-24 at 13-06-48 Single Geth - Grafana

@holiman
Copy link
Contributor Author

holiman commented Aug 31, 2022

Closing this. In case a couple of peers do not have the thing we're asking for, and legitimately never return responses, this would "indefinitely" keep degrading the rtt.

@holiman holiman closed this Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants