add hash circler locator #1362
add hash circler locator #1362
Conversation
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
from
61dba9d to
5f84bcc
Compare
Codecov Report
@@ Coverage Diff @@
## master #1362 +/- ##
==========================================
+ Coverage 51.46% 52.02% +0.56%
==========================================
Files 133 135 +2
Lines 8744 8903 +159
==========================================
+ Hits 4500 4632 +132
- Misses 3866 3888 +22
- Partials 378 383 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Why don't use a consistent hash and separate a node into multi discontinuous virtual nodes to implement it instead of the pkg/hashcircler.
The current implementation schedules all the keys which on a node(A) into its previous node(B) when the node(A) is disabled. It means that the previous node(B) will immediately double the pressure.
dfget/locator/hashcircler_locator.go
Outdated
| default: | ||
| } | ||
|
|
||
| ev, ok := h.evQueue.PollTimeout(time.Second) |
There was a problem hiding this comment.
It can be written:
if ev, ok := h.evQueue.PollTimeout(time.Second); ok {
h.handleEvent(ev.(*SuperNodeEvent))
}
dfget/locator/hashcircler_locator.go
Outdated
| } | ||
|
|
||
| func (h *hashCirclerLocator) handleEvent(ev *SuperNodeEvent) { | ||
| if ev.evType == enableEv { |
There was a problem hiding this comment.
How about using switch...case... instead of if?
pkg/algorithm/algorithm.go
Outdated
|
|
||
| // DedupStringArr removes duplicate string in array. | ||
| func DedupStringArr(input []string) []string { | ||
| dedupMap := make(map[string]struct{}) |
There was a problem hiding this comment.
Another implementation can return a sorted and deduplicated result:
// out is copied from input
sort.Strings(out)
idx := 0
for i := 1; i < len(out); i++ {
if out[idx] != out[i] {
idx++
out[idx] = out[i]
}
}
return out[:idx+1]
pkg/hashcircler/hash_circler.go
Outdated
| } | ||
|
|
||
| if len(PreSetKeys) == 0 { | ||
| return nil, fmt.Errorf("taraget arr is nil") |
There was a problem hiding this comment.
just log "empty preSetKeys" or "preSetKeys is nil"
pkg/hashcircler/hash_circler.go
Outdated
|
|
||
| // NewPreSetHashCircler constructs an instance of preSetHashCircler from presetKeys. And this is thread safety. | ||
| // if hashFunc is nil, it will be set to default hash func. | ||
| func NewPreSetHashCircler(PreSetKeys []string, hashFunc func(string) uint64) (HashCircler, error) { |
pkg/hashcircler/hash_circler.go
Outdated
|
|
||
| hashIndex := h.hashFunc(input) | ||
| rangeIndex := sort.Search(len(h.circleArr), func(i int) bool { | ||
| if hashIndex <= h.circleArr[i].end { |
There was a problem hiding this comment.
func(i int) bool {
return hashIndex <= h.circleArr[i].end
}
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
2 times, most recently
from
e26d62b to
2d819ea
Compare
Do an Implementation by consistent hasher now. |
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
2 times, most recently
from
6e795c6 to
0c5a6e9
Compare
pkg/hashcircler/hash_circler.go
Outdated
| break | ||
| } | ||
|
|
||
| v := rand.Uint64() |
There was a problem hiding this comment.
It's not a good solution to use rand.Unit64() with a rand.Seed(time.Now().UnixNano()) directly to create virtual nodes, it may cause 2 problems:
- it gets different results in different nodes and each execution, this is unexpected
- it cannot guarantee that the virtual nodes are evenly distributed
There was a problem hiding this comment.
Now [0, math.MaxUint64] is spilt into pieces, and range pieces will be evenly distributed by keys.
There was a problem hiding this comment.
Once keys are determined, the owners which the divided range belong to are determined again.
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
from
0c5a6e9 to
7c081f9
Compare
Signed-off-by: allen.wq <[email protected]>
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
from
7c081f9 to
0da63b0
Compare
|
Now Consistent hash circler has been implemented for HashCircler, Could you have a review |
|
optimize the hash circle, the algorithm of insert/delete/find is updated to rbtree instead of array. |
…ted to rbtree instead of array. Signed-off-by: allen.wq <[email protected]>
wangforthinker
force-pushed
the
feat/add-hash-circler-supernode-locator
branch
from
0963638 to
0fdd9e7
Compare
Signed-off-by: allen.wq [email protected]
Ⅰ. Describe what this PR did
Add an implementation of SuperNodeLocator. HashCirclerLocator allow Add/Delete supernode, and the deleted supernode will not be selected.
Ⅱ. Does this pull request fix one issue?
NONE.
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
add ut.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews