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

[p2p] Try to fix P2P memory issue #3795

Merged
merged 3 commits into from
Jun 23, 2021
Merged

Conversation

AlexiaChen
Copy link
Contributor

@AlexiaChen AlexiaChen commented Jun 21, 2021

Issue

# github.com/harmony-one/harmony/p2p/types
p2p/types/peerAddr.go:89:8: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak

Since this library is a dependency of libp2p-kad-dht, choose to upgrade and update libp2p-kad-dht

Test

According to the latest analysis and speculation, there is no evidence that the DNS node memory issue is related to these Fixes, but there is some correlation. So try to reproduce on my local https://github.com/harmony-one/harmony-ops-priv/issues/36#issuecomment-864986243, test is on going

@AlexiaChen AlexiaChen self-assigned this Jun 21, 2021
@AlexiaChen AlexiaChen added bug Something isn't working WIP Work in progress don't merge yet! labels Jun 21, 2021
@LeoHChen
Copy link
Contributor

Can you upgrade the entire libp2p version, not just kad-dht?

Copy link
Contributor

@JackyWYX JackyWYX left a comment

Choose a reason for hiding this comment

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

The code looks good, and it should definitely be resolved. But it seems this is still not the root cause though, since the method is only called in limited times.

Is there anything with libp2p that is possibly related to memory leak?

@LeoHChen LeoHChen merged commit b8cd520 into harmony-one:main Jun 23, 2021
@AlexiaChen
Copy link
Contributor Author

The code looks good, and it should definitely be resolved. But it seems this is still not the root cause though, since the method is only called in limited times.

Is there anything with libp2p that is possibly related to memory leak?

@JackyWYX you are definitely right,the root cause is not related to this fix. so another test is on going

@AlexiaChen AlexiaChen deleted the memory-issue branch June 24, 2021 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working WIP Work in progress don't merge yet!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants