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.

Sign up for GitHub

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

fix: use accurate bucket logic #64

Merged
merged 1 commit into from
Apr 2, 2020
Merged

fix: use accurate bucket logic #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 10 additions & 10 deletions table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,18 @@ func (rt *RoutingTable) NearestPeers(id ID, count int) []peer.ID {
// Add peers from the target bucket (cpl+1 shared bits).
pds.appendPeersFromList(rt.buckets[cpl].list)

// If we're short, add peers from buckets to the right until we have
// enough. All buckets to the right share exactly cpl bits (as opposed
// to the cpl+1 bits shared by the peers in the cpl bucket).
// If we're short, add peers from all buckets to the right. All buckets
// to the right share exactly cpl bits (as opposed to the cpl+1 bits
// shared by the peers in the cpl bucket).
//
// Unfortunately, to be completely correct, we can't just take from
// buckets until we have enough peers because peers because _all_ of
// these peers will be ~2**(256-cpl) from us.
//
// However, we're going to do that anyways as it's "good enough"
// This is, unfortunately, less efficient than we'd like. We will switch
// to a trie implementation eventually which will allow us to find the
// closest N peers to any target key.

for i := cpl + 1; i < len(rt.buckets) && pds.Len() < count; i++ {
pds.appendPeersFromList(rt.buckets[i].list)
if pds.Len() < count {
Copy link

Choose a reason for hiding this comment

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

Yep. This is more accurate. And sorting happens later, I assume.

for i := cpl + 1; i < len(rt.buckets); i++ {
pds.appendPeersFromList(rt.buckets[i].list)
}
}

// If we're still short, add in buckets that share _fewer_ bits. We can
Expand Down
95 changes: 7 additions & 88 deletions table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,26 +311,13 @@ func TestTableFindMultiple(t *testing.T) {
}
}

func assertSortedPeerIdsEqual(t *testing.T, a, b []peer.ID) {
t.Helper()
if len(a) != len(b) {
t.Fatal("slices aren't the same length")
}
for i, p := range a {
if p != b[i] {
t.Fatalf("unexpected peer %d", i)
}
}
}

func TestTableFindMultipleBuckets(t *testing.T) {
t.Parallel()

local := test.RandPeerIDFatal(t)
localID := ConvertPeerID(local)
m := pstore.NewMetrics()

rt, err := NewRoutingTable(5, localID, time.Hour, m, NoOpThreshold)
rt, err := NewRoutingTable(5, ConvertPeerID(local), time.Hour, m, NoOpThreshold)
require.NoError(t, err)

peers := make([]peer.ID, 100)
Expand All @@ -339,96 +326,28 @@ func TestTableFindMultipleBuckets(t *testing.T) {
rt.TryAddPeer(peers[i], true)
}

targetID := ConvertPeerID(peers[2])

closest := SortClosestPeers(rt.ListPeers(), targetID)
targetCpl := CommonPrefixLen(localID, targetID)

// split the peers into closer, same, and further from the key (than us).
var (
closer, same, further []peer.ID
)
var i int
for i = 0; i < len(closest); i++ {
cpl := CommonPrefixLen(ConvertPeerID(closest[i]), targetID)
if targetCpl >= cpl {
break
}
}
closer = closest[:i]
closest := SortClosestPeers(rt.ListPeers(), ConvertPeerID(peers[2]))

var j int
for j = len(closer); j < len(closest); j++ {
cpl := CommonPrefixLen(ConvertPeerID(closest[j]), targetID)
if targetCpl > cpl {
break
}
}
same = closest[i:j]
further = closest[j:]
t.Logf("Searching for peer: '%s'", peers[2])

// should be able to find at least 30
// ~31 (logtwo(100) * 5)
found := rt.NearestPeers(targetID, 20)
found := rt.NearestPeers(ConvertPeerID(peers[2]), 20)
if len(found) != 20 {
t.Fatalf("asked for 20 peers, got %d", len(found))
}

// We expect this list to include:
// * All peers with a common prefix length > than the CPL between our key
// and the target (peers in the target bucket).
// * Then a subset of the peers with the _same_ CPL (peers in buckets to the right).
// * Finally, other peers to the left of the target bucket.

// First, handle the peers that are _closer_ than us.
if len(found) <= len(closer) {
// All of our peers are "closer".
assertSortedPeerIdsEqual(t, found, closer[:len(found)])
return
}
assertSortedPeerIdsEqual(t, found[:len(closer)], closer)

// We've worked through closer peers, let's work through peers at the
// "same" distance.
found = found[len(closer):]

// Next, handle peers that are at the same distance as us.
if len(found) <= len(same) {
// Ok, all the remaining peers are at the same distance.
// unfortunately, that means we may be missing peers that are
// technically closer.

// Make sure all remaining peers are _somewhere_ in the "closer" set.
pset := peer.NewSet()
for _, p := range same {
pset.Add(p)
}
for _, p := range found {
if !pset.Contains(p) {
t.Fatalf("unexpected peer %s", p)
}
for i, p := range found {
if p != closest[i] {
t.Fatalf("unexpected peer %d", i)
}
return
}
assertSortedPeerIdsEqual(t, found[:len(same)], same)

// We've worked through closer peers, let's work through the further
// peers.
found = found[len(same):]

// All _further_ peers will be correctly sorted.
assertSortedPeerIdsEqual(t, found, further[:len(found)])

// Finally, test getting _all_ peers. These should be in the correct
// order.

// Ok, now let's try finding all of them.
found = rt.NearestPeers(ConvertPeerID(peers[2]), 100)
if len(found) != rt.Size() {
t.Fatalf("asked for %d peers, got %d", rt.Size(), len(found))
}

// We should get _all_ peers in sorted order.
for i, p := range found {
if p != closest[i] {
t.Fatalf("unexpected peer %d", i)
Expand Down