Skip to content
This repository was 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
unsaturatedBins := k.getUnsaturatedBins(pp.PeersPerBin, depth)
Copy link
Member

@zelig zelig Feb 6, 2019

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

@zelig zelig Feb 6, 2019

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

@zelig zelig Feb 8, 2019

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

@holisticode holisticode Feb 12, 2019

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 && depth == len(pp.PeersPerBin)
saturated := depth == len(pp.PeersPerBin) && len(unsaturatedBins) == 0
Copy link
Member

@zelig zelig Feb 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

can we please just put this check in the function, ie. return immediately if it is false?
maybe we should rename the function to checkSaturation and have it return an error.

Copy link
Contributor Author

@holisticode holisticode Feb 12, 2019

Choose a reason for hiding this comment

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

Why return an error and not simply bool? There is nothing which can cause an error here.

It is either saturated or not. That is what is currently implemented


log.Trace(fmt.Sprintf("%08x: healthy: knowNNs: %v, gotNNs: %v, saturated: %v\n", k.base, knownn, gotnn, saturated))
return &Health{
Expand Down
6 changes: 3 additions & 3 deletions swarm/network/simulations/discovery/discovery_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func discoverySimulation(nodes, conns int, adapter adapters.NodeAdapter) (*simul
}

healthy := &network.Health{}
if err := client.Call(&healthy, "hive_healthy", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
if err := client.Call(&healthy, "hive_getHealthInfo", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
return false, fmt.Errorf("error getting node health: %s", err)
}
log.Debug(fmt.Sprintf("node %4s healthy: connected nearest neighbours: %v, know nearest neighbours: %v,\n\n%v", id, healthy.ConnectNN, healthy.KnowNN, healthy.Hive))
Expand Down Expand Up @@ -352,7 +352,7 @@ func discoveryPersistenceSimulation(nodes, conns int, adapter adapters.NodeAdapt
healthy := &network.Health{}
addr := id.String()
ppmap := network.NewPeerPotMap(network.NewKadParams().NeighbourhoodSize, addrs)
if err := client.Call(&healthy, "hive_healthy", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
if err := client.Call(&healthy, "hive_getHealthInfo", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
return fmt.Errorf("error getting node health: %s", err)
}

Expand Down Expand Up @@ -422,7 +422,7 @@ func discoveryPersistenceSimulation(nodes, conns int, adapter adapters.NodeAdapt
healthy := &network.Health{}
ppmap := network.NewPeerPotMap(network.NewKadParams().NeighbourhoodSize, addrs)

if err := client.Call(&healthy, "hive_healthy", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
if err := client.Call(&healthy, "hive_getHealthInfo", ppmap[common.Bytes2Hex(id.Bytes())]); err != nil {
return false, fmt.Errorf("error getting node health: %s", err)
}
log.Info(fmt.Sprintf("node %4s healthy: got nearest neighbours: %v, know nearest neighbours: %v", id, healthy.ConnectNN, healthy.KnowNN))
Expand Down