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] fix: sdk load balance #261

Merged
merged 2 commits into from Dec 11, 2022
Merged

[FIX] fix: sdk load balance #261

merged 2 commits into from Dec 11, 2022

Conversation

dougsong
Copy link
Contributor

fix #260

@mergify
Copy link
Contributor

mergify bot commented Nov 18, 2022

感谢您提出Pull Request,我会尽快Review。我会在1-2日内进行查看或者回复,如果遇到节假日可能会处理较慢,敬请谅解。

@zouyx
Copy link
Member

zouyx commented Dec 9, 2022

ci 好像没过?我重启试试。

@dougsong
Copy link
Contributor Author

dougsong commented Dec 9, 2022

应该是remote里的TestApolloConfig_SyncABC1Error测试,没有初始化server列表,setDown节点没有生效导致一直在重试。感觉在RequestRecovery里判断下server.ipMap[appConfig.GetHost()]是否为空,为空直接返回error应该就可以了,如果没问题,我就这么修改下

@zouyx zouyx deleted the branch apolloconfig:develop December 10, 2022 04:29
@zouyx zouyx closed this Dec 10, 2022
@zouyx zouyx reopened this Dec 10, 2022
@zouyx zouyx changed the base branch from master to develop December 10, 2022 04:34
@zouyx
Copy link
Member

zouyx commented Dec 10, 2022

应该是remote里的TestApolloConfig_SyncABC1Error测试,没有初始化server列表,setDown节点没有生效导致一直在重试。感觉在RequestRecovery里判断下server.ipMap[appConfig.GetHost()]是否为空,为空直接返回error应该就可以了,如果没问题,我就这么修改下

十分感谢,我感觉我看出了问题在哪。

  1. server.SetDownNode(appConfig.GetHost(), host)

    这里的参数传入不是太对,应该是反过来。因为第一个参数需要的是连接的 configService,而第二个参数需要的是连接的具体服务,是在前面 LoadBalance 返回的 host,可能是写的时候没注意。你可以修改一下参数名


  2. 确实如你所说 setDownNode 没生效,需要在这里初始化一下,应该就可以。我认为应该是防 configSerivce 失效没初始化 server列表用的。

@dougsong
Copy link
Contributor Author

应该是remote里的TestApolloConfig_SyncABC1Error测试,没有初始化server列表,setDown节点没有生效导致一直在重试。感觉在RequestRecovery里判断下server.ipMap[appConfig.GetHost()]是否为空,为空直接返回error应该就可以了,如果没问题,我就这么修改下

十分感谢,我感觉我看出了问题在哪。

  1. server.SetDownNode(appConfig.GetHost(), host)

    这里的参数传入不是太对,应该是反过来。因为第一个参数需要的是连接的 configService,而第二个参数需要的是连接的具体服务,是在前面 LoadBalance 返回的 host,可能是写的时候没注意。你可以修改一下参数名

  2. 确实如你所说 setDownNode 没生效,需要在这里初始化一下,应该就可以。我认为应该是防 configSerivce 失效没初始化 server列表用的。

我按照你说的修改了下代码,当 serverMap 为空时会直接初始化 map,把 serverHost 加到 configService 的 serverMap 中,这样做之前的单测就没问题了。

@zouyx zouyx self-assigned this Dec 11, 2022
@zouyx zouyx added the bug label Dec 11, 2022
env/server/server.go Show resolved Hide resolved
@mergify mergify bot merged commit 9459f7a into apolloconfig:develop Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]负载均衡无法切换节点
2 participants