Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Balanced peers in kademlia #1207

Merged
merged 21 commits into from
Mar 10, 2021
Merged

Balanced peers in kademlia #1207

merged 21 commits into from
Mar 10, 2021

Conversation

zbiljic
Copy link
Contributor

@zbiljic zbiljic commented Feb 8, 2021

relates to: #57

Using base address for starting node of 00000000 rest of description is made here.
For that base address we have following (random) addresses for different proximities (first 8 here):

0 -> "11011011"
1 -> "01011110"
2 -> "00100010"
3 -> "00010111"
4 -> "00001000"
5 -> "00000100"
6 -> "00000010"
7 -> "00000001"

Bits in common are what defines bin of some other address in relation to base address.

To have some sort of balancing, we try to have some dispersion of addresses for each bin (proximity).
We define this as bit suffix for bin, and if should be number > 0. If we set it to 1 we would have 2 possibilities, with 2 we have 4, with 3 we have 8, and so on.

For 2 following suffixes are produced for address:

00
01
10
11

Using this config, for each bin we generate table of "balanced" addresses toward which we aspire to have connections to (this is generated on Kademlia instantiation and is based on node 'base' address). We use prefix for each bin as a staring point, and to it we append each of these suffixes, to create pseudo address prefixes:

bin(0)
prefix = ""
balanced prefixes:
'100'
'101'
'110'
'111'

bin(1)
prefix = "0"
balanced prefixes:
'0100'
'0101'
'0110'
'0111'

bin(2)
prefix = "00"
balanced prefixes:
'00100'
'00101'
'00110'
'00111'

...

bin(4)
prefix = "0000"
balanced prefixes:
'0000100'
'0000101'
'0000110'
'0000111'

...

The additional bit in these examples comes after the prefix, and is opposite of the bit of the one in the base address. Index of that bit is at PO + 1.

With these prepared "balanced" address prefixes, for each bin we can check if we have balanced connections. We also use this information to decide if we want to connect to some other known peer in order to have balanced connected state.

The algorithm to determine if we need to connect to some other peer is done by:

  • going over each generated pseudo address (prefix) from each bin
  • then finding closest already connected peer to that pseudo address
  • if the proximity of that closest connected peer (to pseudo address) is less than 'bin + suffix length'
  • we connect to the closest known peer
    • closest known peer is again found in relation to pseudo address
    • and if proximity of it in relation to pseudo address is what we expect for balanced connection we attempt to connect to it

This process is added as separate function just before existing process for connecting to peers in Kademlia, which looks at neighborhood depth to decide if more peer connections need to be made.

@zbiljic zbiljic self-assigned this Feb 8, 2021
@zbiljic
Copy link
Contributor Author

zbiljic commented Feb 8, 2021

/run beekeeper

@zbiljic
Copy link
Contributor Author

zbiljic commented Feb 12, 2021

/run beekeeper

@zbiljic zbiljic marked this pull request as ready for review February 12, 2021 12:04
@zbiljic zbiljic added the ready for review The PR is ready to be reviewed label Feb 12, 2021
@acud
Copy link
Member

acud commented Feb 22, 2021

Looks good. Let's merge this after the release tomorrow

Copy link
Member

@metacertain metacertain left a comment

Choose a reason for hiding this comment

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

some occurences of a problem with check lengths, probably we should add some simple test as well that could show this

// check proximity
closestConnectedPO := swarm.Proximity(closestConnectedPeer.Bytes(), pseudoAddr.Bytes())

if int(closestConnectedPO) < i+k.bitSuffixLength {
Copy link
Member

Choose a reason for hiding this comment

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

I think we need i + k.bitSuffixLength + 1 here because of the extra flipped bit in the common address
Also in line 284


closestKnownPeerPO := swarm.Proximity(closestKnownPeer.Bytes(), pseudoAddr.Bytes())

if int(closestKnownPeerPO) != i+k.bitSuffixLength {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be < i+k.bitSuffixLength+1

for i := range k.commonBinPrefixes[bin] {
pseudoAddr := k.commonBinPrefixes[bin][i]

err := k.connectedPeers.EachBinRev(func(peer swarm.Address, po uint8) (bool, bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This could be substituted with checking proximity of closest connected peer

return false
}

binCheckBitLen := int(bin) + k.bitSuffixLength
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be int(bin) + k.bitSuffixLength + 1 because of flipped bit in common addresses

@acud
Copy link
Member

acud commented Feb 22, 2021

@metacertain let's do a revision of the branch before merging it

@metacertain
Copy link
Member

metacertain commented Mar 1, 2021

So one other thing that was missing, is that actually we need an extended Proximity function that has a higher limit than MaxPO
Because, if we want balance connections in (for example) bin [MaxPO-1], and suffix length is 3, than we need to be able to see proximities beyond MaxPO ( actually MaxPO-1 + suffix length + 1 ). So I duplicated this Proximity function as ExtendedProximity, after being riddled by anomalies beginning at PO 13. It was not obvious at all.

I think we can merge this now!

@metacertain
Copy link
Member

/run beekeeper

@metacertain
Copy link
Member

/run beekeeper

@ralph-pichler
Copy link
Member

/run beekeeper

@acud acud force-pushed the feature/balanced-kademlia branch from bf19793 to 99f8957 Compare March 9, 2021 16:16
@acud acud merged commit 8a73821 into master Mar 10, 2021
@acud acud deleted the feature/balanced-kademlia branch March 10, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull-request ready for review The PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants