You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/04/05 09:21:16 UTC
[GitHub] [dubbo-go] ztelur opened a new pull request #1135: fix reExporter bug when config-center {applicationName}.configurator data change
ztelur opened a new pull request #1135:
URL: https://github.com/apache/dubbo-go/pull/1135
### What this PR does:
- 修改 protocol.go 文件中 getCacheKey 函数实现；
- 在 registryProtocol 的 reExport 函数中增加 ServiceMap.Register 操作。
### Fixes
- 修复issue #894 配置中心下发配置无法实时生效
- 修复多次配置更改，实时生效时，reExport 操作的Error [DubboExporter.Unexport] error: no services for dubbo 问题
### Special notes for your reviewer:
- 旧的 getCacheKey 函数根据url中所有参数生成key，精度太高，url稍微有变化就生成不同的key，导致配置下发时根据providerUrl生成的key和Export存储时用的key不同，无法进行配置更新。
- Export 时使用 OverrideUrl 修改后的 providerUrl 生成key，应该使用 invoker 中最原始的 providerUrl 生成 key
```
// 初始化时
func (proto *registryProtocol) Export(invoker protocol.Invoker) protocol.Exporter {
.....
providerUrl := getProviderUrl(invoker) // 1 clone了一份url，所以修改不会影响原url
.....
proto.providerConfigurationListener.OverrideUrl(providerUrl)
.....
serviceConfigurationListener.OverrideUrl(providerUrl)
// 2 上述两个OverrideUrl修改了providerUrl的值
.....
key := getCacheKey(providerUrl)
cachedExporter, loaded := proto.bounds.Load(key)
if loaded {
logger.Infof("The exporter has been cached, and will return cached exporter!")
} else {
// 3 将key和cachedExproter的映射关系存入bounds .....
proto.bounds.Store(key, cachedExporter)
}
....
}
// 配置变更时，该函数会进行处理
func (nl *overrideSubscribeListener) doOverrideIfNecessary() {
providerUrl := getProviderUrl(nl.originInvoker)
key := getCacheKey(providerUrl)
// 4 再次getProviderUrl获取的是旧的url，导致getCachedKey获得key和Export时的不同
if exporter, ok := nl.protocol.bounds.Load(key); ok {
....
}
}
```
- 配置实时生效时会调用reExport方法，reExport会调用 protocol.Exporter 的 unexport 和 export 方法，该方法进行了ServiceMap.UnRegister操作，但是 export 时却没有进行 ServiceMap.Register 操作，导致再次配置实时生效时报错；
- 为了减少修改影响范围，所以在 reExport 函数中添加重新进行 ServiceMap.Register 的操作
```
func (proto *registryProtocol) reExport(invoker protocol.Invoker, newUrl *common.URL) {
url := getProviderUrl(invoker)
key := getCacheKey(url)
if oldExporter, loaded := proto.bounds.Load(key); loaded {
wrappedNewInvoker := newWrappedInvoker(invoker, newUrl)
// unexport
oldExporter.(protocol.Exporter).Unexport()
proto.bounds.Delete(key)
// export
proto.Export(wrappedNewInvoker)
// TODO: unregister & unsubscribe
}
}
func (de *DubboExporter) Unexport() {
interfaceName := de.GetInvoker().GetUrl().GetParam(constant.INTERFACE_KEY, "")
de.BaseExporter.Unexport()
err := common.ServiceMap.UnRegister(interfaceName, DUBBO, de.GetInvoker().GetUrl().ServiceKey())
if err != nil {
logger.Errorf("[DubboExporter.Unexport] error: %v", err)
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] ztelur closed pull request #1135: fix reExporter bug when config-center {applicationName}.configurator data change
Posted by GitBox <gi...@apache.org>.
ztelur closed pull request #1135:
URL: https://github.com/apache/dubbo-go/pull/1135
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] gaoxinge commented on a change in pull request #1135: fix reExporter bug when config-center {applicationName}.configurator data change
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1135:
URL: https://github.com/apache/dubbo-go/pull/1135#discussion_r607907462
##########
File path: registry/protocol/protocol.go
##########
@@ -221,17 +223,47 @@ func (proto *registryProtocol) Export(invoker protocol.Invoker) protocol.Exporte
}
func (proto *registryProtocol) reExport(invoker protocol.Invoker, newUrl *common.URL) {
- url := getProviderUrl(invoker)
- key := getCacheKey(url)
+ key := getCacheKey(invoker)
if oldExporter, loaded := proto.bounds.Load(key); loaded {
wrappedNewInvoker := newWrappedInvoker(invoker, newUrl)
oldExporter.(protocol.Exporter).Unexport()
proto.bounds.Delete(key)
+
+ // oldExporter Unexport function unRegister rpcService from the serviceMap, so need register it again as far as possible
+ if err := registerServiceMap(invoker); err != nil {
Review comment:
问一个问题：为什么不把`registerServicemap`放到`proto.Export`里面去？感觉注册服务也是`proto.Export`的职责。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] ztelur commented on a change in pull request #1135: fix reExporter bug when config-center {applicationName}.configurator data change
Posted by GitBox <gi...@apache.org>.
ztelur commented on a change in pull request #1135:
URL: https://github.com/apache/dubbo-go/pull/1135#discussion_r611150074
##########
File path: registry/protocol/protocol.go
##########
@@ -221,17 +223,47 @@ func (proto *registryProtocol) Export(invoker protocol.Invoker) protocol.Exporte
}
func (proto *registryProtocol) reExport(invoker protocol.Invoker, newUrl *common.URL) {
- url := getProviderUrl(invoker)
- key := getCacheKey(url)
+ key := getCacheKey(invoker)
if oldExporter, loaded := proto.bounds.Load(key); loaded {
wrappedNewInvoker := newWrappedInvoker(invoker, newUrl)
oldExporter.(protocol.Exporter).Unexport()
proto.bounds.Delete(key)
+
+ // oldExporter Unexport function unRegister rpcService from the serviceMap, so need register it again as far as possible
+ if err := registerServiceMap(invoker); err != nil {
Review comment:
在初始化时，service_config.Export会进行registerServiceMap，如果再放在proto.Export 中，就会注册两次，所以那个方案改动会很多。这次修改是尽量争取影响范围最小的修改。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org