swarm/network, swarm/pss, p2p/simulations: adapt test kademlia params #1022
Conversation
acud
requested review from
holisticode,
janos,
lmars,
nolash,
nonsense and
zelig
as code owners
… to correctly check node reachability
acud
force-pushed
the
fix-wait-healthy
branch
2 times, most recently
from
977f5f9
to
718622b
Compare
acud
force-pushed
the
fix-wait-healthy
branch
from
718622b
to
6ba7436
Compare
acud
force-pushed
the
fix-wait-healthy
branch
from
ab51d7e
to
9ca145f
Compare
|
otherwise LGTM |
|
@zelig that did it. now, I understand that conceptually this if statement should be in the discovery test (i had a weird feeling seeing it there in the first place). what I don't get is why it was failing. |
|
@justelad should 'unskipping' those simulation tests that use |
| conn.Up = false | ||
| conn.initiated = time.Now().Add(-DialBanTimeout) | ||
| conn.wasInitiated = false |
There was a problem hiding this comment.
Why is it now correct to "allow" disconnection of a disconnected node?
| if err != nil { | ||
| log.Trace("net.InitConn returned an error", "err", err) | ||
| r := rand.New(rand.NewSource(time.Now().UnixNano())) | ||
| t := time.Duration(time.Duration(r.Intn(200)) * time.Millisecond) |
There was a problem hiding this comment.
I read this as skewing of time offsets in case nodes try to connect simultaneously? Is this actually a solution to the problem?
| default: | ||
| r := rand.New(rand.NewSource(time.Now().Unix())) | ||
| timeToSleep := time.Duration(200 + int64(r.Intn(200))) | ||
| time.Sleep(timeToSleep * time.Millisecond) |
There was a problem hiding this comment.
Should this be a parameter not literal 200?
| copy(enode[:], e.BzzAddr.Over()) | ||
| if !k.Reachable(enode) { | ||
| r := rand.New(rand.NewSource(time.Now().UnixNano())) | ||
| t := time.Duration(time.Duration(r.Intn(400)) * time.Millisecond) |
There was a problem hiding this comment.
Not sure about all these literals, amigo...
acud
added
on hold
meta
This PR addresses an incorrect node setup on certain simulations, in which the
Reachablefunction in the Kademlia was not called at all. This resulted in tests flaking due to peer dialups not being sanctioned correctly.Similar tests that use
ServiceContexton setup have been amended to correctly reference the correctReachablefunction pointer from theServiceContext.fixes #994