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

Opt #8

Closed
wants to merge 2 commits into from
Closed

Opt #8

wants to merge 2 commits into from

Conversation

shiling02404
Copy link
Member

@shiling02404 shiling02404 commented Apr 11, 2022

修复&优化在使用中发现的一些问题
#5
#6
#7
1、阻塞方式改为非阻塞方式
2、有创建某个se有问题时,会导致其他的创建不成功
3、consul 有变化时,本地由于serviceList中仍然是旧值,导致没有更新本地
4、同步时,由于consul meta或tag中没有指明协议时会走默认的tcp协议,导致不能创建基于http协议的se
5、同步时,在maxIndex不断变化的情况下,不停的从consul sever拉取数据,占用不必要的带宽

@@ -72,6 +75,7 @@ func (m *consulMonitor) watchConsul(stop <-chan struct{}) {
} else if consulWaitIndex != queryMeta.LastIndex {
consulWaitIndex = queryMeta.LastIndex
m.updateServiceRecord()
time.Sleep(updateServiceRecordWaitTime)
Copy link
Member

Choose a reason for hiding this comment

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

这个应该是在 consul 端修复吧? 对于正常的服务变化来说,每次变化后等待 10 分钟再同步会不会延迟太大了?
我建议缺省值设置小一点,例如 1 分钟。但可以提供一个环境变量参数,在 Consul 的故障没有修复之前,你们可以把这个环境变量参数设置大一些,例如 10 分钟。

Copy link
Member Author

Choose a reason for hiding this comment

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

是的

return defaultProto
}
for _, v := range serviceTags {
isExist := strings.Contains(v, "proto")
Copy link
Member

Choose a reason for hiding this comment

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

建议使用 protocol, proto 是 “原型” 的意思

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@@ -129,7 +129,7 @@ func (s *Controller) mainLoop(stop <-chan struct{}) {
if err != nil {
log.Errorf("Failed to synchronize consul services to Istio: %v", err)
// Retry if failed
s.pushChannel <- &ChangeEvent{}
// s.pushChannel <- &ChangeEvent{}
Copy link
Member

Choose a reason for hiding this comment

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

重试是否应该保留?

@zhaohuabing
Copy link
Member

@shiling02404

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.

2 participants