-
Notifications
You must be signed in to change notification settings - Fork 1
feat: new method GetNodes() #5
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
Open
prat0088
wants to merge
6
commits into
jlao:master
Choose a base branch
from
prat0088:feat-getnodes
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9772914
checkpoint: initial add of GetNodes(). Not tested.
prat0088 babbb50
checkpoint: refine GetNodes(), add tests
prat0088 db7a53d
chore: clean up diff
prat0088 ee597f2
chore: add back deleted file
prat0088 c3fae26
build: fix unimplemented error
prat0088 3791e30
docs: improve function doc wording
prat0088 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,33 +61,79 @@ public TNode GetNode(uint hash) | |
throw new InvalidOperationException("Ring is empty"); | ||
} | ||
|
||
int index = this.BinarySearch(hash, false, default(TNode)); | ||
|
||
if (index >= 0) | ||
int index = this.GetNodeIndex(hash); | ||
|
||
return this.ring[index].Node; | ||
} | ||
|
||
|
||
/// <summary> | ||
/// Gets the node that owns the hash, and the next n - 1 unique nodes | ||
/// on the ring. This method is useful for implementing the concept of | ||
/// replicas. | ||
/// | ||
/// If a node appears on the ring multiple times as virtual nodes, the | ||
/// first instance will be returned and the remaining appearances will | ||
/// be ignored. toward the limit. | ||
/// </summary> | ||
/// <param name="hash">The hash.</param> | ||
/// <param name="n">How many nodes to return. May be fewer than n if n is greater than the number of nodes in the ring.</param> | ||
/// <returns>The nodes that owns the hash, and the following n - 1 nodes.</returns> | ||
public List<TNode> GetNodes(uint hash, int n) | ||
{ | ||
if (this.IsEmpty) | ||
{ | ||
int prev = index - 1; | ||
while (prev >= 0 && this.ring[prev].Hash == hash) | ||
{ | ||
index = prev; | ||
prev--; | ||
} | ||
throw new InvalidOperationException("Ring is empty"); | ||
} | ||
|
||
return this.ring[index].Node; | ||
if (n < 1) | ||
{ | ||
throw new InvalidOperationException( | ||
$"GetNodes() parameter n must be greater or equal to 1, but it was {n}"); | ||
} | ||
else | ||
|
||
var toReturn = new List<TNode>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like we can initialize this list with capacity |
||
var seen = new List<TNode>(); // Faster for small values of n, which is the expected use case. | ||
Comment on lines
+95
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is |
||
|
||
int curIndex = this.GetNodeIndex(hash); | ||
n = Math.Min(n, ring.Count); | ||
|
||
// Loop over the ring, reading the hash's node and following nodes | ||
for (int tries = 0; tries < ring.Count; tries++) | ||
{ | ||
index = ~index; | ||
if (index == this.ring.Count) | ||
// We need to take the entry's ID. | ||
var curNode = ring[curIndex].Node; | ||
if (!seen.Contains(curNode)) | ||
{ | ||
return this.ring[0].Node; | ||
seen.Add(curNode); | ||
toReturn.Add(curNode); | ||
n--; | ||
} | ||
else | ||
{ | ||
return this.ring[index].Node; | ||
// We've already seen curNode node and added it. It's a | ||
// virtual node. Don't re-add it. | ||
} | ||
|
||
if (n == 0) | ||
{ | ||
// We've found all of the nodes the caller asked for. | ||
// Return. | ||
break; | ||
} | ||
|
||
if (++curIndex == ring.Count) | ||
{ | ||
// Wrap around. We're a ring, aren't we? Faster than | ||
// using modulo every loop. | ||
curIndex = 0; | ||
} | ||
} | ||
|
||
return toReturn; | ||
} | ||
|
||
|
||
/// <summary> | ||
/// Removes all instances of the node from the hash ring. | ||
/// </summary> | ||
|
@@ -174,6 +220,31 @@ private IEnumerable<Partition<TNode>> GetPartitions() | |
yield return new Partition<TNode>(first.Node, new HashRange(last.Hash, first.Hash)); | ||
} | ||
|
||
private int GetNodeIndex(uint hash) | ||
{ | ||
int index = this.BinarySearch(hash, false, default(TNode)); | ||
|
||
if (index >= 0) | ||
{ | ||
int prev = index - 1; | ||
while (prev >= 0 && this.ring[prev].Hash == hash) | ||
{ | ||
index = prev; | ||
prev--; | ||
} | ||
} | ||
else | ||
{ | ||
index = ~index; | ||
if (index == this.ring.Count) | ||
{ | ||
index = 0; | ||
} | ||
} | ||
|
||
return index; | ||
} | ||
|
||
struct RingItem | ||
{ | ||
public RingItem(TNode node, uint hash) | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an ArgumentException would be better here.