libp2p / libp2p/py-libp2p

flaky: dht_pair fixture races on routing-table population ("Node A should know about Node B")

Open Beginner friendly
#1,423 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
624
Forks
256
Avg merge
1d 34m
Merged PRs (30d)
47

Description

The dht_pair fixture in tests/core/kad_dht/test_kad_dht.py intermittently fails during setup with:

AssertionError: Node A should know about Node B

at test_kad_dht.py:177. Because it's a fixture, any test that uses it fails when this trips (currently ~5 tests: test_register_validator, test_find_node_reply_does_not_prepend_unknown_target, and others).

Evidence it's a flake, not a real bug

Hit on two unrelated PRs this week, neither of which touches kad_dht:

  • #1395 (webrtc varint dedup) — tox (3.11, core), failed on test_find_node_reply_does_not_prepend_unknown_target
  • #1409 (timed_cache de-flake) — tox (3.13, core), failed on test_register_validator

Both bottom out in the same fixture assertion. Passes on re-run / locally.

Why it races

The fixture does:

await dht_a.peer_routing.routing_table.add_peer(peer_b_info)
await dht_b.peer_routing.routing_table.add_peer(peer_a_info)
async with background_trio_service(dht_a), background_trio_service(dht_b):
    await trio.sleep(0.1)              # fixed wait
    try:
        await dht_a.find_peer(...)     # best-effort, exceptions only logged
        await dht_b.find_peer(...)
    except Exception:
        ...
    assert dht_a.routing_table.peer_in_table(...)   # hard assert

The hard assert depends on the peer still being resident after a fixed 0.1s sleep and a best-effort find_peer. Under CI load that window isn't always enough (the added peer may not have survived early service maintenance / discovery hasn't converged), so peer_in_table is False and setup blows up.

Suggested fix

Replace the fixed sleep + one-shot assert with a bounded poll until both routing tables are populated, e.g.:

with trio.fail_after(10):
    while not (
        dht_a.routing_table.peer_in_table(host_b.get_id())
        and dht_b.routing_table.peer_in_table(host_a.get_id())
    ):
        await trio.sleep(0.05)

Same idea used for the other CI de-flakes (#1401, #1408): wait for the condition, don't guess a duration. Happy to send a PR if that direction looks right.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in tests/core/kad_dht/test_kad_dht.py at the dht_pair fixture and its assertion near line 177. Run the affected core tests, including test_register_validator and test_find_node_reply_does_not_prepend_unknown_target, to reproduce the intermittent setup failure. Done means the fixture waits boundedly for both routing tables to contain their peers instead of relying on the fixed sleep and one-shot assertion.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.