Skip to content

[Merged by Bors] - PeerDB Status unknown bug fix #2907

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

Closed
wants to merge 4 commits into from

Conversation

AgeManning
Copy link
Member

Issue Addressed

The PeerDB was getting out of sync with the number of disconnected peers compared to the actual count. As this value determines how many we store in our cache, over time the cache was depleting and we were removing peers immediately resulting in errors that manifest as unknown peers for some operations.

The error occurs when dialing a peer fails, we were not correctly updating the peerdb counter because the increment to the counter was placed in the wrong order and was therefore not incrementing the count.

This PR corrects this.

@AgeManning AgeManning changed the base branch from stable to unstable January 13, 2022 07:14
@AgeManning AgeManning added the ready-for-review The code is ready for review label Jan 13, 2022
@divagant-martian
Copy link
Collaborator

Github insists this is targeted to != unstable for some reason

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more ERROs on my node while running this branch overnight! 🎉 Previously my node would hit this several times per hour, so I think we're good 👌

@michaelsproul
Copy link
Member

To update Github's branch status you need to push a new commit. Could rebase on #2906 once merged.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jan 13, 2022
@michaelsproul michaelsproul removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Jan 14, 2022
@AgeManning
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 14, 2022
## Issue Addressed

The PeerDB was getting out of sync with the number of disconnected peers compared to the actual count. As this value determines how many we store in our cache, over time the cache was depleting and we were removing peers immediately resulting in errors that manifest as unknown peers for some operations.

The error occurs when dialing a peer fails, we were not correctly updating the peerdb counter because the increment to the counter was placed in the wrong order and was therefore not incrementing the count. 

This PR corrects this.
@paulhauner
Copy link
Member

bors r+

@bors
Copy link

bors bot commented Jan 14, 2022

Already running a review

@bors
Copy link

bors bot commented Jan 14, 2022

@bors bors bot changed the title PeerDB Status unknown bug fix [Merged by Bors] - PeerDB Status unknown bug fix Jan 14, 2022
@bors bors bot closed this Jan 14, 2022
@AgeManning AgeManning deleted the status-unknown branch January 18, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants