diff --git a/table.go b/table.go index 0cfdfa1..c106fbb 100644 --- a/table.go +++ b/table.go @@ -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 { + 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 diff --git a/table_test.go b/table_test.go index 215e82f..1c93fd0 100644 --- a/table_test.go +++ b/table_test.go @@ -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) @@ -339,88 +326,21 @@ 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) @@ -428,7 +348,6 @@ func TestTableFindMultipleBuckets(t *testing.T) { 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)