-
Notifications
You must be signed in to change notification settings - Fork 288
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
Feat consist load balance #600
Conversation
init github actions
add apache license
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #600 +/- ##
==========================================
+ Coverage 37.25% 37.38% +0.13%
==========================================
Files 175 176 +1
Lines 11659 11755 +96
==========================================
+ Hits 4343 4395 +52
- Misses 6952 6992 +40
- Partials 364 368 +4
|
} | ||
|
||
func (c *Consistent) put(key int64, session getty.Session) { | ||
c.RLock() |
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.
这里是不是应该用写锁而不是读锁?
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.
是,写的时候看错了
return consistentInstance | ||
} | ||
|
||
func ConsistentHashLoadBalance(sessions *sync.Map, xid string) getty.Session { |
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.
目前getty模块目前调用Select时的sessions地址是不变的,所以这里使用单例模式也就没有问题。
不过外部的代码随时可能改变,出于代码健壮性考虑,是不是处理下sessions地址改变的情况会更好?
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.
突然想起来,我这里有一个 refreshHashCircle 方法,如果 session 地址发生变更,session 相当于 close了,我会重新构造 hashCircle,所以应该能覆盖到这种变更的case
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.
@Wang
ConsistentHashLoadBalance实例的创建过程使用了once,当SessionManager管理的sessions *sync.Map地址发生变化,再次调用ConsistentHashLoadBalance,ConsistentHashLoadBalance内部的sessions还是原来的旧地址,新旧地址指向两组完全不同的sessions。
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.
@Wang ConsistentHashLoadBalance实例的创建过程使用了once,当SessionManager管理的sessions *sync.Map地址发生变化,再次调用ConsistentHashLoadBalance,ConsistentHashLoadBalance内部的sessions还是原来的旧地址,新旧地址指向两组完全不同的sessions。
对对对,早上还是不够清醒,哈哈
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.
我在 consistentInstance.pick(sessions, xid) 的时候会把最新的 sessions 传进去,如果检测到无效地址会重新构建单例对象的hash环,应该是cover住了这个场景。
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.
外部sessions中的元素可能会增加、减少,pick函数中只检测closed是不够的。
记录一下上次传入的sessions地址,每次被调用时比较下,地址不同就refresh。这个思路你觉得怎么样?
consistentInstance = &Consistent{ | ||
hashCircle: make(map[int64]getty.Session), | ||
} | ||
// construct hash circle |
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.
构建哈希环的这部分代码和refreshHashCircle函数中的高度重合,是不是可以重构下,调用refreshHashCircle函数。
return consistentInstance | ||
} | ||
|
||
func ConsistentHashLoadBalance(sessions *sync.Map, xid string) getty.Session { |
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.
外部sessions中的元素可能会增加、减少,pick函数中只检测closed是不够的。
记录一下上次传入的sessions地址,每次被调用时比较下,地址不同就refresh。这个思路你觉得怎么样?
已通知代码提交者处理冲突 |
看了下没冲突,是你已经解决了吗 |
是的 |
What this PR does:
implement consistent hash func
Which issue(s) this PR fixes:
Fixes #584
Special notes for your reviewer:
Does this PR introduce a user-facing change?: