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

New Saturation for health implementation #1195

Closed
wants to merge 9 commits into from
Closed

New Saturation for health implementation #1195

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f81df3
swarm/network: new saturation for implementation
holisticode Feb 6, 2019
99df803
swarm/network: re-added saturation func in Kademlia as it is used els…
holisticode Feb 6, 2019
b64be4f
swarm/network: saturation with higher MinBinSize
holisticode Feb 7, 2019
6fba874
swarm/network: PeersPerBin with depth check
holisticode Feb 8, 2019
e187868
swarm/network: edited tests to pass new saturated check
holisticode Feb 8, 2019
14e2430
swarm/network: minor fix saturated check
holisticode Feb 11, 2019
627d061
swarm/network/simulations/discovery: fixed renamed RPC call
holisticode Feb 11, 2019
d0ca8b1
swarm/network: renamed to isSaturated and returns bool
holisticode Feb 12, 2019
b7c7d95
swarm/network: early depth check
holisticode Feb 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion swarm/network/kademlia.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ func (k *Kademlia) GetHealthInfo(pp *PeerPot) *Health {

// check saturation
holisticode marked this conversation as resolved.
Show resolved Hide resolved
unsaturatedBins := k.getUnsaturatedBins(pp.PeersPerBin, depth)
Copy link
Member

Choose a reason for hiding this comment

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

Note that now depth is not checked against the length of peersPerBin.
so if there are empty rows just after depth, the node will pass as healthy.

Copy link
Member

Choose a reason for hiding this comment

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

First create a testcase to show this problem

Copy link
Contributor Author

@holisticode holisticode Feb 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't understand these comments.

Note that now depth is not checked against the length of peersPerBin.

peersPerBin HAS the length of depth

so if there are empty rows just after depth, the node will pass as healthy.
First create a testcase to show this problem

I can't create a Kademlia with an empty row "after" depth (do you mean shallower? or deeper?)
As far as I understand this is not even possible today anymore, as per definition depth will not allow shallower empty rows?

Copy link
Member

Choose a reason for hiding this comment

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

the real depth should be compared to the length of PeersPerBin. if they are not then
with only the nearest neighbours connected and having depth == 0, you pass the saturated test!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we mark this as resolved?

saturated := len(unsaturatedBins) == 0
saturated := len(unsaturatedBins) == 0 && depth == len(pp.PeersPerBin)
holisticode marked this conversation as resolved.
Show resolved Hide resolved

log.Trace(fmt.Sprintf("%08x: healthy: knowNNs: %v, gotNNs: %v, saturated: %v\n", k.base, knownn, gotnn, saturated))
return &Health{
Expand Down
10 changes: 5 additions & 5 deletions swarm/network/kademlia_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func TestHealthStrict(t *testing.T) {
// know three peers, connected to the two deepest
// healthy
tk.Register("00000000")
tk.checkHealth(true)
tk.checkHealth(false)

// know three peers, connected to all three
// healthy
Expand Down Expand Up @@ -279,9 +279,9 @@ func TestHealthStrict(t *testing.T) {
tk.checkHealth(true)

// add peer in bin 1
// healthy, as it is known but not connected
// unhealthy, as it is known but not connected
tk.Register("10000000")
tk.checkHealth(true)
tk.checkHealth(false)

// connect peer in bin 1
// depth change, is now 1
Expand All @@ -305,9 +305,9 @@ func TestHealthStrict(t *testing.T) {
tk.checkHealth(true)

// add peer in bin 2
// healthy, no depth change
// unhealthy, no depth change
tk.Register("11000000")
tk.checkHealth(true)
tk.checkHealth(false)

// connect peer in bin 2
// depth change - as we already have peers in bin 3 and 4,
Expand Down