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.

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

WIP: iterative find cleanup #3503

Closed
wants to merge 1 commit into from
Closed

WIP: iterative find cleanup #3503

wants to merge 1 commit into from

Conversation

shyba
Copy link
Member

@shyba shyba commented Dec 3, 2021

This makes the iterative find algorithm behave mostly like described in the wiki, which looks in sync with how I found it described everywhere else so far.

Missing and some notes:

  • add the stop condition in a way that is not too aggressive (tried as described, but it stops immediately)
    • the stop condition described in the wiki happens when the last element, sorted by distance, in list of nodes we have to return is closer to the key we are searching than the nodes we have available to query. The current issues trying to implement that are:
      • it is concurrent, so this can happen intermittently at many points in time, which makes it only feasible to do it on the _search_round which kind of has the whole context
      • stops too early when done there... which is weird (stops improving for real?)
      • when that happens, sometimes lowering alpha helps and increasing worsens it
  • some kind of metric to see if we are improving or not, which I suppose could fit mainly 3 categories:
    • quality (average distance of returned nodes to key)
    • speed (time to find first value?)
    • traffic (crashing routers is no fun)
  • feedback (later stage)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.641% when pulling 400c4e4 on iterative_find into 6343771 on master.

@lyoshenka
Copy link
Member

lyoshenka commented Jan 10, 2022

@kauffj kauffj closed this Jan 26, 2022
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.

5 participants