Skip to content

Kad search improvements: parallel searches, alpha widening, More button#505

Merged
mrjimenez merged 4 commits into
amule-project:masterfrom
got3nks:pr-b-kad-search-parallelism
Apr 29, 2026
Merged

Kad search improvements: parallel searches, alpha widening, More button#505
mrjimenez merged 4 commits into
amule-project:masterfrom
got3nks:pr-b-kad-search-parallelism

Conversation

@got3nks

@got3nks got3nks commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Four small Kad-search improvements distilled from #428. Each commit is independently meaningful and small enough to revert in isolation.

  1. Bump ALPHA_QUERY from 3 to 5 (one line + comment).
  2. Lift the once-per-search invariant on KADEMLIA_FIND_VALUE_MORE and expose it as CSearch::RequestMoreResults().
  3. Stop the GUI from cancelling the previous Kad search when the user starts a new one — this is the actual reason starting a second Kad search appeared to silently kill the first.
  4. Wire the (until-now stubbed) "More" button on the search dialog to RequestMoreResults, monolithic only. amulegui leaves the button disabled until a follow-up PR adds an EC opcode.

What's in this PR

1. Kad: bump ALPHA_QUERY from 3 to 5

ALPHA_QUERY is the maximum number of in-flight FIND_VALUE queries during a Kad search. CSearch::ProcessResponse's existing cascade keeps that many queries in flight as long as new closer contacts keep arriving. The original Kademlia paper (MIT 2002) calls 3 the lower bound of the typical α range and lists 3-10 as the practical span; aMule has carried eMule's historical 3 forever, which slow-converges searches for unpopular keywords. 5 keeps network load modest while letting the cascade maintain more parallelism. No protocol change — peers already accept any contact-count up to KADEMLIA_FIND_VALUE_MORE.

2. Kad: expose user-triggered search widening (CSearch::RequestMoreResults)

CSearch already implements a wider variant: when JumpStart detects all best-distance peers are dead it sends KADEMLIA_FIND_VALUE_MORE (= 11 contacts) instead of the default 2 to one peer. The mechanism was gated to once per search via a wxFAIL on second use.

This commit:

  • Replaces the single CContact* m_requestedMoreNodesContact with std::set<CUInt128> m_requestedMoreNodes (set of already-reasked client IDs). The dead-nodes-fallback at JumpStart keeps firing once because m_requestedMoreNodes.empty() guards it — semantics preserved.
  • Drops the wxFAIL in SendFindValue's reaskMore branch; it just inserts the contact into the set.
  • Updates ProcessResponse's "node sent more contacts than requested" check to key on set membership rather than a single tracked contact pointer.
  • Adds public bool CSearch::RequestMoreResults() that walks m_responded (closest-first) for the next contact not yet reasked and dispatches SendFindValue(contact, /*reaskMore=*/true). Bounded by KADEMLIA_FIND_VALUE_MORE_REASKS = 4 (new constant in Defines.h) to limit per-search outgoing traffic.

No wire-format change.

3. SearchDlg: don't stop the previous Kad search when starting a new one

Pressing Search fires CSearchDlg::OnBnClickedStart, which after the 2-second rate-limit calls OnBnClickedStop(nullEvent) before StartNewSearch(). OnBnClickedStop in turn calls CSearchList::StopSearch() with the default globalOnly=false; for a Kad search that routes through CSearchManager::StopSearch(m_currentSearch, /*delayDelete=*/false) and deletes the previous CSearch immediately. ~CSearch fires Notify_KadSearchEnd(searchID)CSearchDlg::KadSearchEnd strips the ! indicator from the previous tab and result delivery stops because the CSearch object is gone.

Net effect: starting a second Kad search appeared to silently cancel the first. The comment at CSearchDlg::KadSearchEnd had been telling on itself for years ("// 0: just update all pages (there is only one KAD search running at a time anyway)") — that assumption is what this commit lifts.

The Kad data layer (CSearchManager::m_searches) supports parallel searches by design (map keyed by target hash). The pre-stop is needed for ED2K — the server has a single in-flight search packet per session and m_searchPacket has to be reset before the next search — but is gratuitous for Kad.

Change OnBnClickedStart to call StopSearch(/*globalOnly=*/true) which:

  • Resets ED2K Global state (deletes m_searchPacket, stops the timer, clears m_searchInProgress).
  • Leaves Kad searches alone (the existing m_searchType == KadSearch && !globalOnly guard in StopSearch is now skipped on the new-start path).

The Cancel button (OnBnClickedStop) and Clear button (OnBnClickedClear) keep their full StopSearch() behaviour — those are meant to stop the current Kad search.

4. SearchDlg: wire the "More" button to CSearch::RequestMoreResults

The "More" button on the search dialog has been a stub for years — its tooltip read "Searches for more results on eD2k. Not supported for Kad yet." and it had no event handler, just an Enable(false) default. This commit wires it for Kad.

  • CSearchManager grows two static helpers: IsKadSearch(uint32_t) and RequestMoreResults(uint32_t). Both linear-scan m_searches (keyed by target hash) for the matching searchID; CSearch counts at any one time are tiny so the cost is negligible.
  • CSearchList exposes the same two methods on its public API. Monolithic forwards to CSearchManager. CSearchListRem (amulegui remote-GUI) stubs both as false for now — amulegui has no direct Kad layer access; an EC opcode (EC_OP_KAD_SEARCH_MORE_RESULTS) is a follow-up PR. The button is shown disabled in amulegui mode, never clickable.
  • CSearchDlg::OnBnClickedSearchMore reads the currently-selected notebook page's searchID, calls theApp->searchlist->RequestMoreResults, and logs one of:
    • Kad search: requested wider results from one more peer.
    • Kad search: no peer left to reask for more results (cap reached or no responses yet).
  • Button enable state is recomputed in three places: tab creation, tab change (OnSearchPageChanged), and search end (KadSearchEnd). Enabled iff theApp->searchlist->IsKadSearch returns true on the visible tab's searchID — so ED2K tabs leave it disabled and ended Kad searches drop it back to disabled too.
  • muuli_wdr.cpp tooltip updated.

Backward compatibility

  • No wire / protocol changes. All four commits leverage existing Kad opcodes that peers already accept.
  • ED2K Local / Global searches behave exactly as before. Cancel / Clear buttons unchanged.
  • amulegui (remote GUI over EC) compiles cleanly because the new CSearchList methods are stubbed in the remote variant. Button is shown but greyed out — same UX as the prior never-wired stub.
  • CSearchManager::m_searches static map is untouched in shape; new helpers are linear scans over it.

Tested

  • macOS Apple Silicon Release.
  • Verified with two simultaneous Kad searches that:
    • Both tabs keep their ! indicator and keep returning results.
    • The previously-broken pre-stop path no longer fires (no ~CSearch for the first search when starting the second).
    • The "More" button enables/disables correctly when switching between Kad and ED2K tabs.

Files changed

File Purpose
src/kademlia/kademlia/Defines.h ALPHA_QUERY 3 → 5; new KADEMLIA_FIND_VALUE_MORE_REASKS = 4.
src/kademlia/kademlia/Search.h m_requestedMoreNodes set (replaces single pointer); RequestMoreResults() declaration.
src/kademlia/kademlia/Search.cpp Implementation of the above; JumpStart and ProcessResponse updated to use the set.
src/kademlia/kademlia/SearchManager.h / .cpp IsKadSearch(searchID) and RequestMoreResults(searchID) static helpers.
src/SearchList.h / .cpp Same two methods on CSearchList (monolithic implementation forwards to CSearchManager).
src/amule-remote-gui.h Stub implementations for CSearchListRem (return false until EC opcode lands).
src/SearchDlg.h / .cpp OnBnClickedSearchMore handler; EVT_BUTTON(IDC_SEARCHMORE, …); pre-stop changed from full StopSearch() to StopSearch(/*globalOnly=*/true); button enable/disable logic in CreateNewTab, OnSearchPageChanged, KadSearchEnd.
src/muuli_wdr.cpp "More" button tooltip updated.

got3nks added 4 commits April 29, 2026 15:54
ALPHA_QUERY is the maximum number of in-flight FIND_VALUE queries
during a Kad search.  At each step the initiator picks up to
ALPHA_QUERY closest unqueried nodes and asks them for closer
contacts; CSearch::ProcessResponse's existing cascade keeps that
many queries in flight as long as new closer contacts keep arriving.

aMule has carried eMule's historical value of 3.  The original
Kademlia paper (MIT, 2002) calls 3 the lower bound of the typical
α range and lists 3-10 as the practical span; 3 makes sense as a
conservative default but is slow to converge for unpopular
keywords where the routing-table neighbourhood is sparse.

Bump to 5.  The cost is up to 67% more outgoing FIND_VALUE traffic
per search (still bounded by SEARCH_LIFETIME = 45 s) and a slightly
wider initial burst.  The win is a faster expansion of the search
frontier, which surfaces additional file matches for keywords whose
top-3 closest nodes don't index them.

No protocol or wire-format change — peers already accept any
FIND_VALUE request count up to KADEMLIA_FIND_VALUE_MORE.
CSearch already implements a wider FIND_VALUE variant
(KADEMLIA_FIND_VALUE_MORE = 11 contacts instead of 2): JumpStart
fires it once as a dead-nodes fallback when the closest two queried
peers are unreachable, on the assumption that limiting the answer
to 2 contacts hid duplicates of those dead peers.  The wire opcode
is the existing KADEMLIA_FIND_VALUE_MORE; peers already accept it.

This commit lifts the existing once-per-search invariant on the
mechanism and adds a public entry point so a search can be widened
on demand: each call asks the next-closest already-responded peer
for KADEMLIA_FIND_VALUE_MORE contacts instead of 2, and the existing
ProcessResponse cascade then queries the 9 new neighbours with
FIND_VALUE.  Bounded by KADEMLIA_FIND_VALUE_MORE_REASKS (4) to keep
per-search outgoing traffic finite.

* m_requestedMoreNodesContact (single CContact*) becomes
  m_requestedMoreNodes (std::set<CUInt128> of contact ClientIDs).
  The dead-nodes fallback at JumpStart still fires once because
  m_requestedMoreNodes.empty() guards it, and the old NULL/non-NULL
  semantics map cleanly onto empty/non-empty.
* SendFindValue's reaskMore branch drops the wxFAIL on second use
  and just inserts the contact into the set (caller bounds the rate).
* ProcessResponse's "Node sent more contacts than requested" check
  now keys on set membership of the responder rather than equality
  with a single tracked contact.
* RequestMoreResults() walks m_responded (closest-first) for the
  first contact not yet in m_requestedMoreNodes and dispatches the
  reask, returning false when no candidate remains or when the cap
  is hit.

No protocol or wire-format change.  No caller in this commit — the
public API is a hook for a follow-up GUI / EC tag PR (e.g. a "find
more results" button on SearchDlg) that can drive the widening
without rewriting the search engine.
Pressing the Search button (or hitting Enter in the search box)
fires CSearchDlg::OnBnClickedStart, which after the 2-second
rate-limit calls OnBnClickedStop(nullEvent) before StartNewSearch().
OnBnClickedStop in turn calls CSearchList::StopSearch() with the
default globalOnly=false; for a Kad search this routes through
CSearchManager::StopSearch(m_currentSearch, /*delayDelete=*/false),
which deletes the previous CSearch immediately.  ~CSearch fires
Notify_KadSearchEnd(searchID) -> CSearchDlg::KadSearchEnd, which
strips the "!" indicator from the previous tab; the search engine
also stops returning results because the CSearch object is gone.

Net effect: starting a second Kad search appeared to silently
cancel the first.  CSearchDlg::KadSearchEnd has had a comment for
years that reads "// 0: just update all pages (there is only one
KAD search running at a time anyway)" — that assumption is what
this commit lifts.

The Kad data layer (CSearchManager::m_searches) supports parallel
searches by design: the map is keyed by target hash and there is no
code path that stops siblings when a new one starts.  The pre-stop
in OnBnClickedStart is needed for ED2K (the server has a single
in-flight search packet per session and m_searchPacket has to be
reset before the next search), but is gratuitous for Kad.

Change OnBnClickedStart to call StopSearch(/*globalOnly=*/true)
which:
  * resets ED2K Global state (deletes m_searchPacket, stops the
    timer, clears m_searchInProgress);
  * leaves Kad searches alone (the existing
    `m_searchType == KadSearch && !globalOnly` guard in StopSearch
    is now skipped on the new-start path).

The Cancel button (OnBnClickedStop) and Clear button
(OnBnClickedClear) keep their full StopSearch() behaviour — those
ARE intended to stop the current Kad search.
The "More" button on the search dialog has been a stub for years —
its tooltip read "Searches for more results on eD2k. Not supported
for Kad yet." and it had no event handler, just an Enable(false)
default.  Wire it up for Kad searches.

* CSearchManager grows two static helpers:
    - bool IsKadSearch(uint32_t) — true if the searchID corresponds
      to an active CSearch in m_searches.
    - bool RequestMoreResults(uint32_t) — looks up the CSearch by
      searchID and dispatches the per-search reask.
  Both are linear scans over m_searches keyed by target hash; CSearch
  counts at any one time are tiny so the cost is negligible.

* CSearchList exposes the same two methods on its public API.  The
  monolithic implementation forwards to CSearchManager.  CSearchListRem
  (amulegui remote-GUI) stubs both as false for now — an EC opcode
  (EC_OP_KAD_SEARCH_MORE_RESULTS) for amulegui-driven widening is a
  follow-up PR.

* CSearchDlg::OnBnClickedSearchMore reads the currently-selected
  notebook page's searchID and calls theApp->searchlist->RequestMoreResults
  on it.  No-ops cleanly when the active tab is an ED2K search or
  when the per-search cap (KADEMLIA_FIND_VALUE_MORE_REASKS = 4) is hit
  or no eligible peer remains; logs the no-op for diagnostics.

* Button enable state is recomputed in three places: when a tab is
  created, when the user switches tabs (OnSearchPageChanged), and
  when a Kad search ends (KadSearchEnd).  Enabled iff
  theApp->searchlist->IsKadSearch returns true on the visible tab's
  searchID — so ED2K tabs leave it disabled and ended Kad searches
  drop it back to disabled too.

* muuli_wdr.cpp: tooltip updated to describe the new (Kad) behaviour;
  Initial Enable(false) preserved (no tabs at startup).
@mrjimenez mrjimenez merged commit 29faaf9 into amule-project:master Apr 29, 2026
9 checks passed
@got3nks got3nks deleted the pr-b-kad-search-parallelism branch May 3, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants