Skip to content

Commit

Permalink
consistent-hash: ensure get_nodes returns the first pos >= search
Browse files Browse the repository at this point in the history
When collisions occur, we would stable-sort them such that the ring
would always be the same, regardless of input order.  However, the
binary search method (historical mistake) could end up on a dupicate
pos, and take the server as response, clearly not honouring the contract
of returning the /first/ >= pos match.
This change ensures collisions on pos are voided, and basically restores
pre-binary-search distribution introduced in v3.1.

This change should match the ring output with what bucky expects
jjneely/buckytools#17
  • Loading branch information
grobian committed Oct 11, 2017
1 parent 30807c5 commit 09bf949
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions consistent-hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,17 @@ ch_addnode(ch_ring *ring, server *s)
}

ring->entrycnt += ring->hash_replicas;
if (ring->entrycnt == ring->entrysize)
if (ring->entrycnt == ring->entrysize) {
qsort(&ring->entrylist[0], ring->entrycnt, sizeof(ch_ring_entry), cmp);

/* overwrite successive duplicates with the first entry to ensure
* our binary search hits the "first" matching entry */
for (i = 0; i < ring->entrycnt - 1; i++) {
if (ring->entrylist[i].pos == ring->entrylist[i + 1].pos)
ring->entrylist[i + 1].server = ring->entrylist[i].server;
}
}

return ring;
}

Expand Down Expand Up @@ -371,8 +379,9 @@ ch_get_nodes(
i = 0;
j = ring->entrycnt;
t = 0;
/* 1 3 4 6 6 8 8 9 */
while (1) {
t = ((j - i) / 2) + i;
t = (i + j) / 2;
if (ring->entrylist[t].pos == pos)
break; /* exact match */
if (ring->entrylist[t].pos < pos) {
Expand Down

0 comments on commit 09bf949

Please sign in to comment.