Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

swarm/pss: Network tests with prox handlers #1016

Closed
wants to merge 3 commits into from

Conversation

nolash
Copy link
Contributor

@nolash nolash commented Nov 24, 2018

This PR closes #987 , adds network tests to the recently added neighborhood addressing functionality in pss.

It compares each message address to all nodes in the network and, and adds to a recipient list the nodes for whom the message address is within the nearest neighbor depth. It then checks that those recipients and only those recipients get those respective messages.

It chooses senders so that messages have to be routed as far as possible.

Copy link
Member

@zelig zelig left a comment

Choose a reason for hiding this comment

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

@nolash I know this is in progress but ...

@@ -0,0 +1,426 @@
package pss
Copy link
Member

Choose a reason for hiding this comment

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

this file should be renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I will after all the other merges.

swarm/pss/pss_new_test.go Outdated Show resolved Hide resolved
// temporary function for polling a "stable" network
// stability here means no conns or drops in network within a "serenity" duration
// timeout is max time to wait for a "stable" network
func serenityNowPlease(sim *simulation.Simulation, serenity time.Duration, timeout time.Duration) bool {
Copy link
Member

Choose a reason for hiding this comment

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

I really dont get this :) why would you need this? just use snapshots and no discovery

Copy link
Contributor Author

@nolash nolash Nov 25, 2018

Choose a reason for hiding this comment

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

Even if we use snapshot, the connections still have to be made in the object. The health check can fire already before all these connections are made. (for example when a node only has minProxBinSize peers, its depth will be 0 and actually defined as healthy now after the kademlia depth calc changes)

If we want to reliably measure that messages end up in their correct neighborhoods, we should make sure that the network has finished forming. Otherwise the po can be checked against the wrong depth and the end result will be wrong.

Copy link
Contributor Author

@nolash nolash Nov 25, 2018

Choose a reason for hiding this comment

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

By the way, I've noticed that stability formation in the network even with mere 16 peers takes quite a bit of time. Several seconds in fact. And even then there might be seemingly spurious connects afterwards. Intuitively it doesn't seem right to me.

Of course, that belongs to problems with discovery protocol or something related, and that would be a different issue.

Copy link
Contributor Author

@nolash nolash Nov 25, 2018

Choose a reason for hiding this comment

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

now after the kademlia depth calc changes

Coming to think of it; I guess it was the same before, that networks could be healthy if there were just two known peers.

Copy link
Contributor Author

@nolash nolash Nov 25, 2018

Choose a reason for hiding this comment

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

Actually, if there was a guarantee that when after the snapshot is loaded, and before the simulation is run, that all the resulting peers from that snapshot will populate the kademlia as known, THEN perhaps waiting for the healthy state will suffice.

I would have to review that this is the case, and maybe propose in a separate PR an amendment to the snapshot loading if it is not.

Copy link
Member

Choose a reason for hiding this comment

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

all snapshot based tests rely on this assumption and i think WaitTillHealth makes sure of that. @janos is that right?

Copy link
Member

Choose a reason for hiding this comment

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

As we discussed in the orange-lounge, WaitTillHealth ensures the health to be true, but not that the state of snapshot is fully loaded. We are using WaitTillHealth to wait, but it may not be the correct thing to do.

@frncmx
Copy link
Contributor

frncmx commented Jan 7, 2019

solves #987

@nolash nolash force-pushed the pss-networktest-prox branch from 9fc6a4b to d310766 Compare February 4, 2019 10:40
@zelig
Copy link
Member

zelig commented Mar 8, 2019

superceded by #1284

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PSS neighbourhood addressing simulation
6 participants