Skip to content

Add wrapping around to key_to_nodes - #40

Open
ruslandoga wants to merge 1 commit into
bitwalker:mainfrom
ruslandoga:fix-14
Open

Add wrapping around to key_to_nodes#40
ruslandoga wants to merge 1 commit into
bitwalker:mainfrom
ruslandoga:fix-14

Conversation

@ruslandoga

@ruslandogaruslandoga commented Feb 10, 2025

Copy link
Copy Markdown

👋

This is a possible fix for #39

@ruslandoga

ruslandoga commented Feb 10, 2025

Copy link
Copy Markdown
Author

Kindly pinging @Gazler as well :)
(as the original contributor of key_to_nodes)

Comment threadlib/ring.ex
case :gb_trees.iterator_from(hash, r) |> :gb_trees.next() do
{_key, node, iter} ->
find_nodes_from_iter(iter, count - 1, [node])
find_nodes_from_iter(r, iter, count - 1, [node], _restarted? = false)

@ruslandogaruslandogaFeb 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if restarted? is needed, but I added it to be safe.


On second though, if we count = min(length(nodes), count) it's not needed.

Comment threadlib/ring.ex
results
:none ->
if restarted? do
Enum.reverse(results)

@ruslandogaruslandogaFeb 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

results were not reversed originally, but I think they need to be since the "happy path" does it (on line 257).

@ruslandogaruslandoga changed the title add wrapping over to key_to_nodesadd wrapping around to key_to_nodesFeb 10, 2025
@ruslandogaruslandoga changed the title add wrapping around to key_to_nodesAdd wrapping around to key_to_nodesFeb 10, 2025
@ruslandogaruslandoga changed the title Add wrapping around to key_to_nodesAdd wrapping around to key_to_nodesFeb 10, 2025
anas-ksibi added a commit to enreach-labs/libring that referenced this pull request Mar 11, 2025
Sign up for freeto 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.

1 participant

@ruslandoga