Skip to content
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

Fix: event driven chain cache #976

Merged
merged 51 commits into from
Feb 7, 2021

Conversation

LaurenceLiZhixin
Copy link
Contributor

What this PR does:
This PR is based on pr #915
pr 915 aims at fix health check——add black list logic, and split conn check router from health check router.
This PR aims at using event-driven to let router send signal to notify channel, to notify chain to refresh its cache.
Not ticker dirven like before, but event driven.
This can descrease the CPU usage, as there is no longer useless comparasion of invoker list of chain, but only compare when invokers/router change.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


}

selectedInvoker := lb.Select(invokers, invocation)

//judge to if the selectedInvoker is invoked

//judge to if the selectedInvoker is invoked and available
Copy link
Contributor

Choose a reason for hiding this comment

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

can not get the meaning of this chinglish sentence

}

// TryRefreshBlackList start 3 gr to check at most block=16 invokers in black list
// if is available remove from black list
Copy link
Contributor

Choose a reason for hiding this comment

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

can not know what is the meaning of this sentence.

@AlexStocks AlexStocks force-pushed the develop branch 3 times, most recently from 25c3b98 to 9a666eb Compare January 23, 2021 11:12
@AlexStocks AlexStocks added this to the v1.5.6 milestone Jan 23, 2021

package conncheck

import (
Copy link
Contributor

Choose a reason for hiding this comment

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

pls split it.

@cityiron cityiron changed the base branch from develop to 1.5 January 26, 2021 13:55
@@ -148,7 +148,13 @@ conditions:
url := getConditionRouteUrl(applicationKey)
assert.NotNil(t, url)
factory := extension.GetRouterFactory(url.Protocol)
r, err := factory.NewPriorityRouter(url)
notify := make(chan struct{})
go func() {
Copy link
Member

Choose a reason for hiding this comment

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

for range ch {
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed~

@AlexStocks AlexStocks changed the title Fix/event driven chain cache Fix: event driven chain cache Feb 3, 2021
@@ -121,38 +121,45 @@ func (invoker *baseClusterInvoker) doSelect(lb cluster.LoadBalance, invocation p

func (invoker *baseClusterInvoker) doSelectInvoker(lb cluster.LoadBalance, invocation protocol.Invocation, invokers []protocol.Invoker, invoked []protocol.Invoker) protocol.Invoker {
if len(invokers) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

if need warn log ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个地方比较有意思,是改了个已有的坑。
invocation.Invoker()是个nil,所以一走到这一行就会panic。
后来发现这里不会被执行到。不存在invokers列表为空的情况。所以决定删掉这行日志了。

go func(ivks []Invoker, i int) {
defer wg.Done()
for j, _ := range ivks {
if j%3-i == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

merge the if.

return invokers[0]
}
protocol.SetInvokerUnhealthyStatus(invokers[0])
logger.Errorf("the invokers of %s is nil. ", invokers[0].GetUrl().ServiceKey())
Copy link
Member

Choose a reason for hiding this comment

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

这个错误日志看起来跟上下文无关,或者是否应该放在 SetInvokerUnhealthyStatus 里更合适

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里位置主要是针对invokers列表因为健康检查导致为空时,打印出对应失败的key。


for _, invoker := range invokers {
if !invoker.IsAvailable() {
for i := 0; i < 3; i++ {
Copy link
Member

Choose a reason for hiding this comment

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

当整个循环次数超过 3 次,最后还是会返回最开始不可用的 selectedInvoker ,是否符合预期

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you r right, I've fixed it.

@cityiron cityiron merged commit 6ad4ede into apache:1.5 Feb 7, 2021
AlexStocks pushed a commit that referenced this pull request Apr 14, 2021
Sign up for free to 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.

None yet

5 participants