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/09/22 06:35:08 UTC
[GitHub] [dubbo] owen200008 opened a new pull request #8872: Close status error noclose exception，i think #7410 have bug
owen200008 opened a new pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872
## What is the purpose of the change
channel nerver close。
we use ReferenceConfig to same provider, destroy last one and create new one
the channel connect is the ReferenceConfig * 5 (default 5 connects)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on a change in pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on a change in pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#discussion_r713906773
##########
File path: dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeChannel.java
##########
@@ -168,7 +169,7 @@ public void close(int timeout) {
if (closed) {
return;
}
- closed = true;
+ close();
Review comment:
async clients will not receive any result if you move close() here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么每台引用这些服务的客户端与各服务端之间的总连接数就是10（前提：客户端和各个服务端之间的网络都是通的），无论是否发生invoker调用，这些连接会一直保持直到客户端结束运行或者服务端退出，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了3处bug:
1. HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
2. ReferenceCountExchangeClient的replaceWithLazyClient()判断是否创建LazyConnectExchangeClient的条件不应该包含client.isClose()，因为replaceWithLazyClient方法是在调用client的close()方法后执行的，这个条件好比1==1恒真，这样LazyConnectExchangeClient关闭几次就会创建几次LazyConnectExchangeClient，这样的效果应该不是设计LazyConnectExchangeClient之初的本意；
3. LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925716060
建议你开一个issue，把你修改的目的和思路讲一下，pr只是为了提交代码，不适合沟通问题
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924859547
maybe both close() and close(int timeout) in HeaderExchangeChannel should do nothing but return directly to support lazy client mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] codecov-commenter commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925722301
# [Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8872](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2ff343) into [master](https://codecov.io/gh/apache/dubbo/commit/db175899d73966f52bbd1a9043503ca749a107e8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db17589) will **decrease** coverage by `0.03%`.
> The diff coverage is `67.10%`.
[](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8872 +/- ##
============================================
- Coverage 60.76% 60.72% -0.04%
- Complexity 446 447 +1
============================================
Files 1100 1101 +1
Lines 44428 44444 +16
Branches 6471 6472 +1
============================================
- Hits 26997 26990 -7
- Misses 14459 14468 +9
- Partials 2972 2986 +14
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL0R1YmJvUHJvdG9jb2wuamF2YQ==) | `60.79% <38.46%> (-3.07%)` | :arrow_down: |
| [...c/protocol/dubbo/ReferenceCountExchangeClient.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL1JlZmVyZW5jZUNvdW50RXhjaGFuZ2VDbGllbnQuamF2YQ==) | `56.66% <55.26%> (-6.67%)` | :arrow_down: |
| [...exchange/support/header/HeaderExchangeChannel.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L2hlYWRlci9IZWFkZXJFeGNoYW5nZUNoYW5uZWwuamF2YQ==) | `75.00% <100.00%> (-5.77%)` | :arrow_down: |
| [...l/dubbo/NerverDieReferenceCountExchangeClient.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL05lcnZlckRpZVJlZmVyZW5jZUNvdW50RXhjaGFuZ2VDbGllbnQuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...in/java/org/apache/dubbo/common/utils/JVMUtil.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvSlZNVXRpbC5qYXZh) | `81.13% <0.00%> (-11.33%)` | :arrow_down: |
| [...ting/exchange/support/header/HeartbeatHandler.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L2hlYWRlci9IZWFydGJlYXRIYW5kbGVyLmphdmE=) | `83.72% <0.00%> (-9.31%)` | :arrow_down: |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0.00%> (-4.35%)` | :arrow_down: |
| [...pport/merger/DefaultProviderURLMergeProcessor.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9tZXJnZXIvRGVmYXVsdFByb3ZpZGVyVVJMTWVyZ2VQcm9jZXNzb3IuamF2YQ==) | `78.26% <0.00%> (-4.35%)` | :arrow_down: |
| [.../main/java/org/apache/dubbo/qos/server/Server.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL3NlcnZlci9TZXJ2ZXIuamF2YQ==) | `75.00% <0.00%> (-4.17%)` | :arrow_down: |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `83.05% <0.00%> (-3.39%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [db17589...b2ff343](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925534726
i see it call HeaderExchangeClient no closeAll function， so it use default same call close(timeout)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925531121
maybe you should use DubboInvoker.destroyAll() instead of destroy(), because destroy() don't close the channel of ReferenceCountExchangeClient if it's reference count > 1.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925597404
可能我一开始就不太喜欢LazyConnectExchangeClient的实现，感觉是个补丁
dubboprotocal的share模式下使用ReferenceCountExchangeClient，如果share模式referenceClientMap的的原子性解决，理论上是不需要LazyConnectExchangeClient去保证连接是有效的
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925664670
提醒一下，应用创建的invoker并不是全部的invoker，dubbo自己也会创建，比如RegistryDirectory， ServiceDiscoveryRegistryDirectory里面的urlInvokerMap，这些map里面的invoker是客户端收到注册中心服务变动通知时自动创建的，即使应用程序把自己创建的所有invoker都调了destroy()，但是还有RegistryDirectory， ServiceDiscoveryRegistryDirectory创建的invoker没有调用destroy()，Reference count还是大于0，close方法是不会实际调用的。
为了解决这个问题，才有了destroyAll()方法，虽然这样的实现也并不完美。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] codecov-commenter edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925722301
# [Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8872](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62e9d6e) into [master](https://codecov.io/gh/apache/dubbo/commit/db175899d73966f52bbd1a9043503ca749a107e8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db17589) will **decrease** coverage by `0.74%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8872 +/- ##
============================================
- Coverage 60.76% 60.02% -0.75%
Complexity 446 446
============================================
Files 1100 1101 +1
Lines 44428 44402 -26
Branches 6471 6441 -30
============================================
- Hits 26997 26651 -346
- Misses 14459 14774 +315
- Partials 2972 2977 +5
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL0R1YmJvUHJvdG9jb2wuamF2YQ==) | `60.64% <38.46%> (-3.21%)` | :arrow_down: |
| [...c/protocol/dubbo/ReferenceCountExchangeClient.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL1JlZmVyZW5jZUNvdW50RXhjaGFuZ2VDbGllbnQuamF2YQ==) | `56.66% <55.26%> (-6.67%)` | :arrow_down: |
| [...l/dubbo/NerverDieReferenceCountExchangeClient.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL05lcnZlckRpZVJlZmVyZW5jZUNvdW50RXhjaGFuZ2VDbGllbnQuamF2YQ==) | `92.85% <92.85%> (ø)` | |
| [...exchange/support/header/HeaderExchangeChannel.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L2hlYWRlci9IZWFkZXJFeGNoYW5nZUNoYW5uZWwuamF2YQ==) | `75.00% <100.00%> (-5.77%)` | :arrow_down: |
| [...ookeeper/ZookeeperDynamicConfigurationFactory.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbkZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [...gcenter/wrapper/CompositeDynamicConfiguration.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL2NvbmZpZ2NlbnRlci93cmFwcGVyL0NvbXBvc2l0ZUR5bmFtaWNDb25maWd1cmF0aW9uLmphdmE=) | `0.00% <0.00%> (-63.34%)` | :arrow_down: |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...fig/configcenter/TreePathDynamicConfiguration.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL2NvbmZpZ2NlbnRlci9UcmVlUGF0aER5bmFtaWNDb25maWd1cmF0aW9uLmphdmE=) | `31.42% <0.00%> (-54.29%)` | :arrow_down: |
| [.../dubbo/metadata/report/MetadataReportInstance.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tbWV0YWRhdGEvZHViYm8tbWV0YWRhdGEtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9tZXRhZGF0YS9yZXBvcnQvTWV0YWRhdGFSZXBvcnRJbnN0YW5jZS5qYXZh) | `12.50% <0.00%> (-40.63%)` | :arrow_down: |
| [.../configcenter/support/zookeeper/CacheListener.java](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9DYWNoZUxpc3RlbmVyLmphdmE=) | `0.00% <0.00%> (-32.44%)` | :arrow_down: |
| ... and [75 more](https://codecov.io/gh/apache/dubbo/pull/8872/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [db17589...62e9d6e](https://codecov.io/gh/apache/dubbo/pull/8872?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925725373
#8895
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924844824
after close(), reference count exchange client will be replaced with lazy client. so the client is still alive.
this is the reason why test_counter_error() failed because this pull destroys such mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), and the closed variable value should keep false in HeaderExchangeChannel close(), then HeaderExchangeChannel close(int timeout) will be called at destroy() - the last statement in test_counter_error(). otherwise the status of the invoker will keep available because
the LazyConnectExchangeClient that was just closed will be recreated at replaceWithLazyClient() in ReferenceCountExchangeClient.java if the variable ```closed``` of client is true:
```
if (!(client instanceof LazyConnectExchangeClient) || client.isClosed()) {
... ...
client = new LazyConnectExchangeClient(lazyUrl, client.getExchangeHandler());
}
```
maybe HeaderExchangeChannel close() codes should like current branch 3.0 (just revert #7410 ):
```
@Override
public void close() {
try {
// graceful close
DefaultFuture.closeChannel(channel);
channel.close();
} catch (Throwable e) {
logger.warn(e.getMessage(), e);
}
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
maybe we should revert #7410, because i debug ReferenceCountExchangeClientTest test_counter_error() as junit test,
it showed that:
```
client.close(); <== call HeaderExchangeChannel close() directly !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
```
close() method will be called directly, closed variable should not set true at close() function, otherwise close(int timeout) will not be called and the status of the invoker will keep available.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924867715
> Not completely solved
> sometimes no disconnect
> i will try to fix it
> @zrlw maybe you are right
> but i must fix it, because we have hotload case, and connect too much, mem oom
dubbo client only create one tcp connection to one dubbo service no matter how many dubbo references refer the same service.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924876443
intranet one is enough. it support set connects and share mode. unexport but have resource no gc i think is not allowed
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
maybe we should revert #7410, because i debug ReferenceCountExchangeClientTest test_counter_error() as junit test,
it showed that:
```
client.close(); <== call HeaderExchangeChannel close() , then call HeaderExchangeChannel close(int timeout) !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
```
close() method will be called firstly, closed variable should not set true at close() function, otherwise close(int timeout) will return directly and the status of the invoker will keep avaiable.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925444637
i created a PR #8881 for branch 3.0, #8882 for master
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924844824
after close(), reference count exchange client will be replaced with lazy client. so the client is still alive and the invoker is still available.
this is the reason why test_counter_error() failed because this pull destroys such mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925442834
@zrlw think you very much
i think we reuse the session in client pool or use share mode client is possible
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么每台引用这些服务的客户端与各服务端之间的总连接数就是10（前提：客户端和各个服务端之间的网络都是通的），无论是否发生invoker调用，这些连接会一直保持直到客户端结束运行或者服务端退出，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了3处bug：
第1处是HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
第2处是ReferenceCountExchangeClient的replaceWithLazyClient()判断是否创建LazyConnectExchangeClient的条件不应该包含client.isClose()，因为replaceWithLazyClient方法是在调用client的close()方法后执行的，这个条件好比1==1恒真，这样LazyConnectExchangeClient关闭几次就会创建几次LazyConnectExchangeClient，这样的效果应该不是设计LazyConnectExchangeClient之处的本意；
第3处是LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 closed pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 closed pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925019795
or add variable channelClosed only for close()
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw removed a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw removed a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924859547
maybe both close() and close(int timeout) in HeaderExchangeChannel should do nothing but return directly to support lazy client mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925462326
@zrlw 原谅英文不太好，用英文怕表达不清晰
我觉得原始的不回收的目的是invoke区分不出来ExchangeClient是share模式还是独占模式的
我的考虑是增加描述类型 或者 share模式采用池的方式实现（目前share模式是简单根据key获取）
底层的网络层封装我觉得还是根据上层的使用场景决定，如果我的思考是对的后面可以增加模式是否启动lazy等等
@zrlw @xiaoheng1 等待你们回复
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), and the closed variable value should keep false in HeaderExchangeChannel close(), then HeaderExchangeChannel close(int timeout) will be called at destroy() - the last statement in test_counter_error(). otherwise the status of the invoker will keep available because
the LazyConnectExchangeClient that was just closed will be recreated at replaceWithLazyClient() in ReferenceCountExchangeClient.java :
```
private void replaceWithLazyClient() {
...
if (!(client instanceof LazyConnectExchangeClient) || client.isClosed()) { <== wierd condition
... ...
client = new LazyConnectExchangeClient(lazyUrl, client.getExchangeHandler());
}
...
}
```
in my opinion, ```client.isClosed()``` should not exist at here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), and the closed variable value should keep false in HeaderExchangeChannel close(), then HeaderExchangeChannel close(int timeout) will be called at destroy() - the last statement in test_counter_error(). otherwise the status of the invoker will keep available because
the LazyConnectExchangeClient that was just closed will be recreated at replaceWithLazyClient() in ReferenceCountExchangeClient.java :
```
private void replaceWithLazyClient() {
...
if (!(client instanceof LazyConnectExchangeClient) || client.isClosed()) { <== wierd condition
... ...
client = new LazyConnectExchangeClient(lazyUrl, client.getExchangeHandler());
}
...
}
```
in my opinion:
1. client.isClosed() should not be conditional at replaceWithLazyClient();
2. helloServiceInvoker.isAvailable() should be true if the client has been replaced by lazy client;
3. client should be reset to null at close() and close(int timeout) in LazyConnectExchangeClient.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925600107
ReferenceCountExchangeClient 绑定 LazyConnectExchangeClient的实现， 不是LazyConnectExchangeClient本身，他本身功能lazy是没问题的
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on a change in pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on a change in pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#discussion_r713906773
##########
File path: dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeChannel.java
##########
@@ -168,7 +169,7 @@ public void close(int timeout) {
if (closed) {
return;
}
- closed = true;
+ close();
Review comment:
async clients will not receive any result if you move close() here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw removed a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw removed a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925019795
or add variable channelClosed only for close()
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925063354
@owen200008
your oom problem might have nothing to do with HeaderExchangeChannel.java because close(timeout) method might only be called by invoker.destroy(), it will not be called until your app exit.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925444637
i created a PR #8881 for branch 3.0
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), and the closed variable value should keep false in HeaderExchangeChannel close(), then HeaderExchangeChannel close(int timeout) will be called at destroy() - the last statement in test_counter_error(). otherwise the status of the invoker will keep available .
maybe HeaderExchangeChannel close() codes should like current branch 3.0 (just revert #7410 ):
```
@Override
public void close() {
try {
// graceful close
DefaultFuture.closeChannel(channel);
channel.close();
} catch (Throwable e) {
logger.warn(e.getMessage(), e);
}
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924861277
Not completely solved
sometimes no disconnect
i will try to fix it
@zrlw maybe you are right
but i must fix it, because we have hotload case, and connect too much, mem oom
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
maybe we should revert #7410, because i debug ReferenceCountExchangeClientTest test_counter_error() as junit test,
it showed that:
```
client.close(); <== call HeaderExchangeChannel close() , then call HeaderExchangeChannel close(int timeout) !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
```
close() method will be called first, closed variable should not set true at close() function, otherwise close(int timeout) will return directly and the status of the invoker will keep available.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
maybe we should revert #7410, because i debug ReferenceCountExchangeClientTest test_counter_error() as junit test,
it showed that:
```
client.close(); <== call HeaderExchangeChannel close() , then call HeaderExchangeChannel close(int timeout) !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
```
close() method will be called first, closed variable should not set true at close() function, otherwise close(int timeout) will return directly and the status of the invoker will keep avaiable.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么每台引用这些服务的客户端与各服务端之间的总连接数就是10（前提：客户端和各个服务端之间的网络都是通的），无论是否发生invoker调用，这些连接会一直保持直到客户端结束运行或者服务端退出，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了3处bug：
1. 是HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
2. ReferenceCountExchangeClient的replaceWithLazyClient()判断是否创建LazyConnectExchangeClient的条件不应该包含client.isClose()，因为replaceWithLazyClient方法是在调用client的close()方法后执行的，这个条件好比1==1恒真，这样LazyConnectExchangeClient关闭几次就会创建几次LazyConnectExchangeClient，这样的效果应该不是设计LazyConnectExchangeClient之处的本意；
3. LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw removed a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw removed a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924844824
after close(), reference count exchange client will be replaced with lazy client. so the client is still alive but the invoker is unavailable.
this is the reason why test_counter_error() failed because this pull destroys such mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924844824
after close(), reference count exchange client will be replaced with lazy client. so the client is still alive but the invoker is unavailable.
this is the reason why test_counter_error() failed because this pull destroys such mechanism.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924867715
> Not completely solved
> sometimes no disconnect
> i will try to fix it
> @zrlw maybe you are right
> but i must fix it, because we have hotload case, and connect too much, mem oom
dubbo client only create one tcp connection to one dubbo service no matter how many dubbo references existed in the app.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924867715
> Not completely solved
> sometimes no disconnect
> i will try to fix it
> @zrlw maybe you are right
> but i must fix it, because we have hotload case, and connect too much, mem oom
dubbo client only create one tcp connection to one dubbo service no matter how many dubbo references refer the same service existed in the app.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么所以使用这些服务的客户端的连接数就是10，无论有无请求，这些连接都会一直保持直到客户端app结束运行，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了两处bug：
第1处是HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
第2处是LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel（tcp长连接一直都在）的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么每台引用这些服务的客户端与各服务端之间的总连接数就是10（前提：客户端和各个服务端之间的网络都是通的），无论是否发生invoker调用，这些连接会一直保持直到客户端结束运行或者服务端退出，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了两处bug：
第1处是HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
第2处是LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] owen200008 commented on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
owen200008 commented on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925525039
说下原因哈：
原来我想的很简单 从网络层看 destroy调用了 close(timeout)，然后进close就没用了，后续不管怎么样都关闭不了了
后来测试没过我看了下 确实是想简单了
就是讨论下这块是否有更好的方式实现，目前感觉没有闭环，牵一发动全身
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), closed variable should not set true at HeaderExchangeChannel close(), otherwise the status of the invoker will keep available.
if closed value keep false, HeaderExchangeChannel close(int timeout) will be called at destroy(), the last statement in test_counter_error().
maybe HeaderExchangeChannel close() should like current branch 3.0:
```
@Override
public void close() {
try {
// graceful close
DefaultFuture.closeChannel(channel);
channel.close();
} catch (Throwable e) {
logger.warn(e.getMessage(), e);
}
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-924964900
i debug ReferenceCountExchangeClientTest test_counter_error() as junit test, it showed that:
```
client.close(); <== call HeaderExchangeChannel close() !!!
// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
destoy(); <== will call HeaderExchangeChannel close(int timeout)
```
HeaderExchangeChannel close() method will be called at the last client.close() in test_counter_error(), and the closed variable value should keep false in HeaderExchangeChannel close(), then HeaderExchangeChannel close(int timeout) will be called at destroy() - the last statement in test_counter_error(). otherwise the status of the invoker will keep available because
the LazyConnectExchangeClient that was just closed will be recreated at replaceWithLazyClient() in ReferenceCountExchangeClient.java :
```
private void replaceWithLazyClient() {
...
if (!(client instanceof LazyConnectExchangeClient) || client.isClosed()) { <== wierd condition
... ...
client = new LazyConnectExchangeClient(lazyUrl, client.getExchangeHandler());
}
...
}
```
in my opinion:
1. client.isClosed() should not be conditional at replaceWithLazyClient();
2. helloServiceInvoker.isAvailable() should be true if the client has been replaced by lazy client.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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] zrlw edited a comment on pull request #8872: Close status error noclose exception，i think #7410 have bug
Posted by GitBox <gi...@apache.org>.
zrlw edited a comment on pull request #8872:
URL: https://github.com/apache/dubbo/pull/8872#issuecomment-925517506
1. 无论客户端app里面有多少invoker，dubbo客户端与每个服务端之间只维持1条长连接，比如你有10个服务端在注册中心登记，那么每台引用这些服务的客户端与各服务端之间的总连接数就是10（前提：客户端和各个服务端之间的网络都是通的），无论是否发生invoker调用，这些连接会一直保持直到客户端结束运行或者服务端退出，所以你的连接数非常多甚至出现oom应该是其他原因引起的；
2. 你改了HeaderExchangeChannel.java的close方法，将当前LazyConnectExchangeClient的closed状态设为true，后面执行replaceWithLazyClient替换ReferenceCountExchangeClient内的ExchangeClient操作时，就会因为client.isClosed()条件为真而重新去创建一个LazyConnectExchangeClient，新建LazyConnectExchangeClient内的ExchangeClient为null，这样当测试方法执行断言helloServiceInvoker.isAvailable()为假时就会失败：原因在于此处isAvailable是DubboInvoker的isAvailable，调用LazyConnectExchangeClient的isConnected()因为ExchangeClient为null，所以返回LazyConnectExchangeClient的initialState，而这个initialState未做配置时默认是true，所以helloServiceInvoker.isAvailable()返回true。
你修改的代码暴露了3处bug:
1. HeaderExchangeChannel.java的close()方法应该将closed状态设置为true，因为跟踪发现关闭LazyConnectExchangeClient并不走close(timeout)方法，而是直接走close()方法，close()把channel连接关闭之后，应该将关闭状态设置为true；
2. ReferenceCountExchangeClient的replaceWithLazyClient()判断是否创建LazyConnectExchangeClient的条件不应该包含client.isClose()，因为replaceWithLazyClient方法是在调用client的close()方法后执行的，这个条件好比1==1恒真，这样LazyConnectExchangeClient关闭几次就会创建几次LazyConnectExchangeClient，这样的效果应该不是设计LazyConnectExchangeClient之处的本意；
3. LazyConnectExchangeClient.close方法没有将内部的ExchangeClient对象置为null，下次再使用已经close的LazyConnectExchangeClient时，就会复用已关闭channel的ExchangeClient导致invoker操作异常。
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
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