tracker: make memory-store announce a uniform random peer sample

The previous ordered-scan-with-wraparound selection biased toward the
low end of the peer-id key space: because ETS ordered_set keys are
sorted (where Go maps iterate randomly) and real peer ids cluster, a
random start key usually landed past every entry, so announces kept
returning the same lowest-keyed peers instead of a varied subset. This
starves high-keyed peers and diverges from chihaya's randomized map
iteration. Replace it with an explicit uniform random sample over the
swarm's keys of the requested kind. The Redis store already matches
chihaya by taking the HKEYS-order prefix and is unchanged. Adds a
regression test with clustered peer ids.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DqKy7xpT2kp7ZATjLKsdhT

Author: Claude

tracker/lib/bento/tracker/storage/memory.ex

@@ -125,15 +125,15 @@ defmodule Bento.Tracker.Storage.Memory do
       peers =
         if seeder? do
           # Return as many leechers as possible.
-          collect(shard, info_hash, @leecher, num_want, nil)
+          sample(shard, info_hash, @leecher, num_want, nil)
         else
           # Return as many seeders as possible, then fill up with
           # leechers, excluding the announcing peer itself.
-          seeders = collect(shard, info_hash, @seeder, num_want, nil)
+          seeders = sample(shard, info_hash, @seeder, num_want, nil)
           remaining = num_want - length(seeders)
 
           if remaining > 0 do
-            seeders ++ collect(shard, info_hash, @leecher, remaining, peer_key(announcer))
+            seeders ++ sample(shard, info_hash, @leecher, remaining, peer_key(announcer))
           else
             seeders
           end
@@ -365,44 +365,24 @@ defmodule Bento.Tracker.Storage.Memory do
     count_infohashes(shard, :ets.next(shard, {info_hash, 2, 0}), count + 1)
   end
 
-  # Collects up to num_want peers of the given kind, starting from a
-  # random position in the swarm and wrapping around, so that repeated
-  # announces return varying subsets - the equivalent of Go's randomized
-  # map iteration order.
-  defp collect(_shard, _info_hash, _kind, num_want, _exclude) when num_want <= 0, do: []
-
-  defp collect(shard, info_hash, kind, num_want, exclude) do
-    start = :crypto.strong_rand_bytes(38)
-    first = :ets.next(shard, {info_hash, kind, start})
-    {peers, remaining} = walk(shard, first, info_hash, kind, nil, num_want, exclude, [])
-
-    if remaining > 0 do
-      from_begin = :ets.next(shard, {info_hash, kind, 0})
-      {peers, _} = walk(shard, from_begin, info_hash, kind, start, remaining, exclude, peers)
-      peers
-    else
-      peers
-    end
-  end
-
-  # Walks keys of the form {info_hash, kind, pk} collecting decoded
-  # peers. Stops at num_want, at the end of the kind's range, or - when
-  # stop_pk is given - once pk passes it (the wrap-around boundary).
-  defp walk(_shard, _key, _info_hash, _kind, _stop_pk, 0, _exclude, acc), do: {acc, 0}
+  # Returns up to num_want peers of the given kind for the swarm, chosen
+  # as a uniform random subset. chihaya achieves varied announce
+  # responses by relying on Go's randomized map iteration order; an ETS
+  # ordered_set is deterministic in key order, so we sample explicitly to
+  # avoid biasing toward any region of the (clustered) peer-id key space.
+  defp sample(_shard, _info_hash, _kind, num_want, _exclude) when num_want <= 0, do: []
 
-  defp walk(shard, {info_hash, kind, pk} = key, info_hash, kind, stop_pk, want, exclude, acc) do
-    cond do
-      stop_pk != nil and pk > stop_pk ->
-        {acc, want}
+  defp sample(shard, info_hash, kind, num_want, exclude) do
+    keys = :ets.select(shard, [{{{info_hash, kind, :"$1"}, :_}, [], [:"$1"]}])
+    keys = if exclude, do: List.delete(keys, exclude), else: keys
 
-      pk == exclude ->
-        walk(shard, :ets.next(shard, key), info_hash, kind, stop_pk, want, exclude, acc)
-
-      true ->
-        acc = [decode_peer_key(pk) | acc]
-        walk(shard, :ets.next(shard, key), info_hash, kind, stop_pk, want - 1, exclude, acc)
-    end
+    keys
+    |> take_random(num_want)
+    |> Enum.map(&decode_peer_key/1)
   end
 
-  defp walk(_shard, _key, _info_hash, _kind, _stop_pk, want, _exclude, acc), do: {acc, want}
+  # Enum.take_random with an explicit small-count fast path.
+  defp take_random(list, n) do
+    if length(list) <= n, do: list, else: Enum.take_random(list, n)
+  end
 end

tracker/test/bento/tracker/storage/memory_test.exs

@@ -84,4 +84,33 @@ defmodule Bento.Tracker.Storage.MemoryTest do
 
     assert Storage.stop(store) == :ok
   end
+
+  test "announce returns a varied random subset across repeated calls" do
+    alias Bento.Tracker.Peer
+
+    {:ok, store} = Storage.new("memory", %{shard_count: 4})
+    ih = String.duplicate("r", 20)
+
+    # Sequential low-valued peer IDs cluster at the low end of the key
+    # space, the case that defeated the earlier ordered-scan selection.
+    leechers = for i <- 1..40, do: %Peer{id: <<i::160>>, ip: {10, 0, 0, rem(i, 250)}, port: i}
+    for peer <- leechers, do: :ok = Storage.put_leecher(store, ih, peer)
+
+    announcer = %Peer{id: <<999::160>>, ip: {10, 0, 1, 1}, port: 999}
+
+    subsets =
+      for _ <- 1..20 do
+        {:ok, peers} = Storage.announce_peers(store, ih, true, 5, announcer)
+        assert length(peers) == 5
+        # No duplicates within a single response.
+        assert length(Enum.uniq_by(peers, &{&1.ip, &1.port})) == 5
+        Enum.map(peers, & &1.port) |> Enum.sort()
+      end
+
+    # The subset varies across calls rather than always returning the same
+    # (lowest-keyed) peers.
+    assert length(Enum.uniq(subsets)) > 1
+
+    assert Storage.stop(store) == :ok
+  end
 end