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

feat: enable pinging connected peers by default #1647

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

danisharora099
Copy link
Collaborator

@danisharora099 danisharora099 commented Oct 10, 2023

Problem

We have been seeing some problems re disconnection of js-waku from service nodes after ~10 mins mark when there is no activity on the peers.
We do have the ability for users to keep pinging this needs, to maintain the connection open. However, this is set disabled by default.

Solution

Enable pinging service nodes we're connected to, by default, at an interval of 5 minutes.

Notes

@danisharora099 danisharora099 requested a review from a team as a code owner October 10, 2023 08:31
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 83.28 KB (+0.07% 🔺) 1.7 s (+0.07% 🔺) 2.4 s (+20.38% 🔺) 4.1 s
Waku Simple Light Node 263.25 KB (+0.03% 🔺) 5.3 s (+0.03% 🔺) 5.5 s (-2.35% 🔽) 10.7 s
ECIES encryption 79.15 KB (0%) 1.6 s (0%) 3 s (-13.89% 🔽) 4.6 s
Symmetric encryption 79.16 KB (0%) 1.6 s (0%) 3 s (-26.99% 🔽) 4.6 s
DNS discovery 111.01 KB (-0.04% 🔽) 2.3 s (-0.04% 🔽) 4.2 s (-0.09% 🔽) 6.5 s
Privacy preserving protocols 131.43 KB (-0.11% 🔽) 2.7 s (-0.11% 🔽) 4.5 s (+3.87% 🔺) 7.1 s
Light protocols 81.12 KB (0%) 1.7 s (0%) 3.2 s (+27.47% 🔺) 4.8 s
History retrieval protocols 80.22 KB (0%) 1.7 s (0%) 1.8 s (-25.26% 🔽) 3.4 s
Deterministic Message Hashing 5.65 KB (0%) 113 ms (0%) 430 ms (-9.94% 🔽) 543 ms

Copy link
Collaborator

@fryorcraken fryorcraken left a comment

Choose a reason for hiding this comment

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

I thought it was reported that there was an unhandled exception when a user enabled the timeout themselves?

@danisharora099
Copy link
Collaborator Author

danisharora099 commented Oct 11, 2023

I thought it was reported that there was an unhandled exception when a user enabled the timeout themselves?

right. this was happening in the period between attempting a reconnection with the peer and error is being thrown when we try to ping a peer while we don't have an active connection with them.

#1646 should be solved with 55e56f6 which makes sense to be part of the same PR.

@danisharora099 danisharora099 merged commit 1d60c4b into master Oct 11, 2023
9 of 11 checks passed
@danisharora099 danisharora099 deleted the feat/enable-pings-default branch October 11, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pingKeepAlive throws uncaught exception Frequent disconnections at regular intervals
2 participants