Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.
70 changes: 62 additions & 8 deletions swarm/network/kademlia.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,8 @@ func (k *Kademlia) string() string {
// used for testing only
// TODO move to separate testing tools file
type PeerPot struct {
NNSet [][]byte
NNSet [][]byte
PeersPerBin []int
}

// NewPeerPotMap creates a map of pot record of *BzzAddr with keys
Expand All @@ -651,6 +652,7 @@ func NewPeerPotMap(neighbourhoodSize int, addrs [][]byte) map[string]*PeerPot {

// all nn-peers
var nns [][]byte
peersPerBin := make([]int, depth)

// iterate through the neighbours, going from the deepest to the shallowest
np.EachNeighbour(a, Pof, func(val pot.Val, po int) bool {
Expand All @@ -664,14 +666,18 @@ func NewPeerPotMap(neighbourhoodSize int, addrs [][]byte) map[string]*PeerPot {
// a neighbor is any peer in or deeper than the depth
if po >= depth {
nns = append(nns, addr)
return true
} else {
// for peers < depth, we just count the number in each bin
// the bin is the index of the slice
peersPerBin[po]++
}
return false
return true
})

log.Trace(fmt.Sprintf("%x PeerPotMap NNS: %s", addrs[i][:4], LogAddrs(nns)))
log.Trace(fmt.Sprintf("%x PeerPotMap NNS: %s, peersPerBin", addrs[i][:4], LogAddrs(nns)))
ppmap[common.Bytes2Hex(a)] = &PeerPot{
NNSet: nns,
NNSet: nns,
PeersPerBin: peersPerBin,
}
}
return ppmap
Expand All @@ -695,6 +701,39 @@ func (k *Kademlia) saturation() int {
return prev
}

// getUnsaturatedBins returns an array of ints; each item in that array corresponds
// to the bin which is unsaturated (number of connections < k.MinBinSize).
// The bin is considered unsaturated only if there are actual peers in that PeerPot's bin (peersPerBin)
// (if there is no peer for a given bin, then no connection could ever be established;
// in a God's view this is relevant as no more peers will ever appear on that bin)
func (k *Kademlia) getUnsaturatedBins(peersPerBin []int, depth int) []int {
// depth could be calculated from k but as this is called from `Healthy()`,
// the depth has already been calculated so we can require it as a parameter
connectedPeersPerBin := make([]int, depth)
k.conns.EachBin(k.base, Pof, 0, func(po, size int, f func(func(val pot.Val) bool) bool) bool {

if po >= depth {
return false
}
log.Trace("peers per bin", "peersPerBin[po]", peersPerBin[po], "po", po)
// if there are actually peers in the PeerPot who can fulfill k.MinBinSize
if peersPerBin[po] >= k.MinBinSize {
Comment thread
holisticode marked this conversation as resolved.
Outdated
log.Trace("connections for po", "po", po, "size", size)
connectedPeersPerBin[po] += size
Comment thread
holisticode marked this conversation as resolved.
Outdated
}
return true
})

unsaturatedBins := make([]int, 0)
for i := 0; i < len(connectedPeersPerBin); i++ {
if connectedPeersPerBin[i] > 0 && connectedPeersPerBin[i] < k.MinBinSize {
unsaturatedBins = append(unsaturatedBins, i)
}
}
log.Trace("list of unsaturated bins", "unsaturatedBins", unsaturatedBins)
return unsaturatedBins
}

// knowNeighbours tests if all neighbours in the peerpot
// are found among the peers known to the kademlia
// It is used in Healthy function for testing only
Expand Down Expand Up @@ -777,8 +816,10 @@ type Health struct {
ConnectNN bool // whether node is connected to all its neighbours
CountConnectNN int // amount of neighbours connected to
MissingConnectNN [][]byte // which neighbours we should have been connected to but we're not
Saturated bool // whether we are connected to all the peers we would have liked to
Hive string
// Saturated: if in all bins < depth number of connections >= MinBinsize or,
// if number of connections < MinBinSize, to the number of available peers in that bin
Saturated bool
Hive string
}

// Healthy reports the health state of the kademlia connectivity
Expand All @@ -798,7 +839,10 @@ func (k *Kademlia) Healthy(pp *PeerPot) *Health {
gotnn, countgotnn, culpritsgotnn := k.connectedNeighbours(pp.NNSet)
knownn, countknownn, culpritsknownn := k.knowNeighbours(pp.NNSet)
depth := depthForPot(k.conns, k.NeighbourhoodSize, k.base)
saturated := k.saturation() < depth
// check saturation
Comment thread
holisticode marked this conversation as resolved.
unsaturatedBins := k.getUnsaturatedBins(pp.PeersPerBin, depth)

Copy link
Copy Markdown
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
Copy Markdown
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

@holisticode holisticode Feb 7, 2019

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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

log.Trace(fmt.Sprintf("%08x: healthy: knowNNs: %v, gotNNs: %v, saturated: %v\n", k.base, knownn, gotnn, saturated))
return &Health{
KnowNN: knownn,
Expand All @@ -811,3 +855,13 @@ func (k *Kademlia) Healthy(pp *PeerPot) *Health {
Hive: k.string(),
}
}

// IsHealthyStrict return the strict interpretation of `Healthy` given a `Health` struct
// definition of strict health: all conditions must be true:
// - we at least know one peer
// - we know all neighbors
// - we are connected to all known neighbors
// - it is saturated
func (h *Health) IsHealthyStrict() bool {
Comment thread
holisticode marked this conversation as resolved.
Outdated
return h.KnowNN && h.ConnectNN && h.CountKnowNN > 0 && h.Saturated
}
84 changes: 70 additions & 14 deletions swarm/network/kademlia_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,60 +176,116 @@ func TestHealthStrict(t *testing.T) {
// no peers
// unhealthy (and lonely)
tk := newTestKademlia(t, "11111111")
tk.checkHealth(false, false)
tk.checkHealth(false)

// know one peer but not connected
// unhealthy
tk.Register("11100000")
tk.checkHealth(false, false)
tk.checkHealth(false)

// know one peer and connected
// healthy
// unhealthy: not saturated
tk.On("11100000")
tk.checkHealth(true, false)
tk.checkHealth(true)

// know two peers, only one connected
// unhealthy
tk.Register("11111100")
tk.checkHealth(false, false)
tk.checkHealth(false)

// know two peers and connected to both
// healthy
tk.On("11111100")
tk.checkHealth(true, false)
tk.checkHealth(true)

// know three peers, connected to the two deepest
// healthy
tk.Register("00000000")
tk.checkHealth(true, false)
tk.checkHealth(true)

// know three peers, connected to all three
// healthy
tk.On("00000000")
tk.checkHealth(true, false)
tk.checkHealth(true)

// add fourth peer deeper than current depth
// unhealthy
tk.Register("11110000")
tk.checkHealth(false, false)
tk.checkHealth(false)

// connected to three deepest peers
// healthy
tk.On("11110000")
tk.checkHealth(true, false)
tk.checkHealth(true)

// add additional peer in same bin as deepest peer
// unhealthy
tk.Register("11111101")
tk.checkHealth(false, false)
tk.checkHealth(false)

// four deepest of five peers connected
// healthy
tk.On("11111101")
tk.checkHealth(true, false)
tk.checkHealth(true)

// add additional peer in bin 0
// unhealthy: unsaturated bin 0, 2 known but 1 connected
tk.Register("00000001")
tk.checkHealth(false)

// Connect second in bin 0
// healthy
tk.On("00000001")
tk.checkHealth(true)

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

// connect peer in bin 1
// depth change, is now 1
// healthy, 1 peer in bin 1 known and connected
tk.On("10000000")
tk.checkHealth(true)

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

// connect second peer in bin 1
// healthy,
tk.On("10000001")
tk.checkHealth(true)

// connect third peer in bin 1
// healthy,
tk.On("10000011")
tk.checkHealth(true)

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

// connect peer in bin 2
// depth change - as we already have peers in bin 3 and 4,
// we have contiguous bins, no bin < po 5 is empty -> depth 5
// healthy, every bin < depth has the max available peers,
// even if they are < MinBinSize
tk.On("11000000")
tk.checkHealth(true)

// add peer in bin 2
// unhealthy, peer bin is below depth 5 but
// has more available peers (2) than connected ones (1)
// --> unsaturated
tk.Register("11000011")
tk.checkHealth(false)
}

func (tk *testKademlia) checkHealth(expectHealthy bool, expectSaturation bool) {
func (tk *testKademlia) checkHealth(expectHealthy bool) {
tk.t.Helper()
kid := common.Bytes2Hex(tk.BaseAddr())
addrs := [][]byte{tk.BaseAddr()}
Expand All @@ -245,7 +301,7 @@ func (tk *testKademlia) checkHealth(expectHealthy bool, expectSaturation bool) {
// - we at least know one peer
// - we know all neighbors
// - we are connected to all known neighbors
health := healthParams.KnowNN && healthParams.ConnectNN && healthParams.CountKnowNN > 0
health := healthParams.IsHealthyStrict()
if expectHealthy != health {
tk.t.Fatalf("expected kademlia health %v, is %v\n%v", expectHealthy, health, tk.String())
}
Expand Down