Skip to content

add test to demo missing peer records after listen#941

Merged
Stebalien merged 4 commits intomasterfrom
test/peer-records-no-listen-addrs
May 20, 2020
Merged

add test to demo missing peer records after listen#941
Stebalien merged 4 commits intomasterfrom
test/peer-records-no-listen-addrs

Conversation

@yusefnapora
Copy link
Contributor

This shows off what I think is causing #939

The problem is that we're starting our hosts without listen addrs, and apparently the peerstore isn't keeping our signed records since they don't have any addrs. Then, when we call .Listen, it takes ~ 5 seconds for the change to be detected and the new record to be generated.

cc @aarshkshah1992 @raulk @vyzo

@yusefnapora
Copy link
Contributor Author

The last commits register BasicHost as a network.Notifee so it can get informed when we start or stop listening on an address and call SignalAddressChanges, which causes a new signed record to be created and added to the peerstore right away instead of having to wait for the 5 second ticker to detect the change.

It's still a little racy - on my laptop I only have to sleep for a millisecond after listening before I can get the new record out of the peerstore, but that's not quite long enough on Travis, so I bumped it to 20ms in the test. Still better than five seconds though :)

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

We could get rid of that race by waiting, but that's likely more trouble than it's worth. Instead, we should just fix the panic.

@Stebalien Stebalien merged commit aa60461 into master May 20, 2020
@Stebalien Stebalien mentioned this pull request May 26, 2020
77 tasks
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.

2 participants