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

为什么 dubbo-go包中的http.go line 108 用了 service.Query.Get("timeout") 来设置 RemoteServiceReadDeadLine? #5

Closed
hxmhlt opened this issue Mar 7, 2019 · 10 comments

Comments

@hxmhlt
Copy link
Contributor

hxmhlt commented Mar 7, 2019

而这个timeout的来源其实是server端的Registry_Config配置,在这个配置中,这个单位是秒,并且默认是5。而在SetServiceReadDeadLine中其实默认单位是ms。
这样会造成client端在等待server端返回结果的conn中,等5ms就会超时。造成i/o timeout的bug。

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 7, 2019

经过测试,把server端的registry_config.timeout改成从3改成3000就不会出现i/o timeout的bug。

@AlexStocks
Copy link
Contributor

多谢 @hxmhlt 。能否提交一个pr啊?

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 7, 2019

已提

@AlexStocks
Copy link
Contributor

再次感谢,不过在代码中把超时时间提升到秒,个人觉得有两个问题:
第一,这会造成与客户端超时时间单位不一致;
第二,秒级的超时时间单位是否太粗粒度了?java dubbo的配置时间单位好像也是毫秒级。

个人觉得是不是把example下server中配置文件中相关数值改成对应的毫秒值是否更合理点。

以上是个人意见,可以继续讨论,看看到底如何处理更合适点。

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 7, 2019

可以改成灵活的配置,比如可以让用户配置成3ms,3s等,像其他字段一样。在代码里面转成time.duration类型,不太懂为什么要限定具体的单位。

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 7, 2019

不知道有多少地方用了这个配置,我再看下吧,看好不好改。

@AlexStocks
Copy link
Contributor

我的意思是不更改代码,仅仅把你提到的 ”把server端的registry_config.timeout改成从3改成3000" 这句话实施就可以了,或者说更明了一点,就是把 dev/server.yml#L20 或者 test/server.yml#L20 或者 release/server.yml#L20 的 3 改成 3000,是不是就可以了?

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 7, 2019

@AlexStocks 不行的,https://github.com/dubbo/dubbo-go/blob/master/registry/consumer.go 237行,会在链接zk的时候也用到这个值了,这里默认的是second

@hxmhlt
Copy link
Contributor Author

hxmhlt commented Mar 8, 2019

我这边改动了较多地方,就是把registry_config的timeout改成了golang的duration string格式,这里可以用户自己指定时间单位,然后zk连接和client.call用到这个时间的时候分别转换成自己需要的second和millsecond单位。你看下要不要merge,谢谢

@AlexStocks
Copy link
Contributor

LGTM! merge done!

AlexStocks pushed a commit that referenced this issue Aug 5, 2019
sxllwx added a commit that referenced this issue May 21, 2020
flycash referenced this issue in flycash/dubbo-go May 29, 2020
Patrick0308 pushed a commit to Patrick0308/dubbo-go that referenced this issue Jun 28, 2020
LaurenceLiZhixin pushed a commit to LaurenceLiZhixin/dubbo-go that referenced this issue Jul 17, 2021
Ftr: Add case for config center but exclude zookeeper
LaurenceLiZhixin added a commit to LaurenceLiZhixin/dubbo-go that referenced this issue Mar 31, 2022
* fix: interface and some ut

* fix: ci

* Fix: upgrade go version

* Fix: import formatter

* fix: some license

* fix: add workflow tidy

* fix: add workflow tidy

Co-authored-by: lizhixin.lzx <lizhixin.lzx@alibaba-inc.com>
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

No branches or pull requests

2 participants