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/01/23 04:35:57 UTC
[GitHub] [dubbo-go] wenxuwan opened a new pull request #1010: Fix:zk too many tcp conn
wenxuwan opened a new pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010
Now every registry have one connection with zk, this is a waste of resources. It is completely unnecessary for dubbo-go to maintain a complex reconnection mechanism, which will increase the number of links and memory of zk.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (3fdbb01) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.04%`.
> The diff coverage is `60.92%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.48% -0.05%
==========================================
Files 259 268 +9
Lines 12737 13317 +580
==========================================
+ Hits 7583 7922 +339
- Misses 4199 4409 +210
- Partials 955 986 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [173 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...3a05f71](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] fangyincheng commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
fangyincheng commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r588838738
##########
File path: remoting/getty/getty_client.go
##########
@@ -168,7 +168,7 @@ func (c *Client) Close() {
c.pool = nil
c.mux.Unlock()
if p != nil {
- p.close()
+ c.Close()
Review comment:
c.Close not p.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.
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
So now codes seems to implement the (2), not (1)? I'm not very sure.
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if active number is zero
----------------------------------------------------------------
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] AlexStocks commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584187352
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ client.DecreaseActiveNumber()
Review comment:
add a return value for this func.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (ae6a138) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.03%`.
> The diff coverage is `60.66%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.50% -0.04%
==========================================
Files 259 268 +9
Lines 12737 13321 +584
==========================================
+ Hits 7583 7926 +343
- Misses 4199 4410 +211
- Partials 955 985 +30
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [172 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...a8bc4e1](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (3fdbb01) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.04%`.
> The diff coverage is `60.92%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.48% -0.05%
==========================================
Files 259 268 +9
Lines 12737 13317 +580
==========================================
+ Hits 7583 7922 +339
- Misses 4199 4409 +210
- Partials 955 986 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [173 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...3fdbb01](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (ae6a138) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.03%`.
> The diff coverage is `60.66%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.50% -0.04%
==========================================
Files 259 268 +9
Lines 12737 13321 +584
==========================================
+ Hits 7583 7926 +343
- Misses 4199 4410 +211
- Partials 955 985 +30
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [172 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...ae6a138](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (a8bc4e1) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.23%`.
> The diff coverage is `60.66%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.29% -0.24%
==========================================
Files 259 268 +9
Lines 12737 13321 +584
==========================================
+ Hits 7583 7899 +316
- Misses 4199 4436 +237
- Partials 955 986 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [173 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...a8bc4e1](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] AlexStocks commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r563041821
##########
File path: registry/zookeeper/listener.go
##########
@@ -142,9 +138,6 @@ func (l *RegistryConfigurationListener) Process(configType *config_center.Config
func (l *RegistryConfigurationListener) Next() (*registry.ServiceEvent, error) {
for {
select {
- case <-l.client.Done():
Review comment:
为何删除？
##########
File path: config_center/zookeeper/impl.go
##########
@@ -209,8 +205,6 @@ func (c *zookeeperDynamicConfiguration) closeConfigs() {
c.cltLock.Lock()
defer c.cltLock.Unlock()
logger.Infof("begin to close provider zk client")
- // Close the old client first to close the tmp node
- c.client.Close()
Review comment:
1 把上面的 log 挪到 lock 上面，减小锁粒度；
2 为何把 client.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.
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587111107
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
So we only need to check that active number equals to zero (nonblocking), instead of waiting until active number becomes to zero (blocking)?
----------------------------------------------------------------
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] AlexStocks commented on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-770715069
pls change the target branch to 1.5
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r569392162
##########
File path: cluster/router/condition/app_router_test.go
##########
@@ -67,11 +67,11 @@ conditions:
_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
assert.NoError(t, err)
- defer func() {
- err = ts.Stop()
- assert.NoError(t, err)
- z.Close()
- }()
+ //defer func() {
Review comment:
yes, i will finish this when i have time
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
(1) close the conn until all the invokers (in one Node(VM)) destroyed (codes should be)
(2) close the conn if active number is zero
- so now codes seems to implement the (2), not (1)?
- or does (1) equal to (2)?
I am not sure.
----------------------------------------------------------------
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] AlexStocks commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584184754
##########
File path: remoting/exchange_client.go
##########
@@ -51,6 +52,8 @@ type ExchangeClient struct {
client Client
// the tag for init.
init bool
+ // the number of service using the exchangeClient
+ activeNum uint32
Review comment:
IMO, uber atomic pkg is more useful.
##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
return nil
}
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+ atomic.AddUint32(&client.activeNum, 1)
Review comment:
// increase number of service using client
func (client *ExchangeClient) IncreaseActiveNumber() uint32 {
return atomic.AddUint32(&client.activeNum, 1)
}
##########
File path: remoting/zookeeper/client.go
##########
@@ -285,6 +285,11 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
if state == (int)(zk.StateHasSession) {
continue
}
+ if event.State == zk.StateHasSession {
+ atomic.StoreUint32(&z.valid, 1)
+ close(z.reconnectCh)
+ z.reconnectCh = make(chan struct{})
Review comment:
pls add a lock to promise the following clause is executed sequentially .
```go
close(z.reconnectCh)
z.reconnectCh = make(chan struct{})
```
##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
return nil
}
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+ atomic.AddUint32(&client.activeNum, 1)
+}
+
+// decrease number of service using client
+func (client *ExchangeClient) DecreaseActiveNumber() {
Review comment:
return value
##########
File path: remoting/zookeeper/client.go
##########
@@ -210,7 +220,7 @@ func NewMockZookeeperClient(name string, timeout time.Duration, opts ...Option)
name: name,
ZkAddrs: []string{},
Timeout: timeout,
- exit: make(chan struct{}),
+ reconnectCh: make(chan struct{}),
Review comment:
pls add a 'reconnectChLock' to close it safely.
##########
File path: remoting/zookeeper/client.go
##########
@@ -285,6 +285,11 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
if state == (int)(zk.StateHasSession) {
continue
}
+ if event.State == zk.StateHasSession {
+ atomic.StoreUint32(&z.valid, 1)
+ close(z.reconnectCh)
+ z.reconnectCh = make(chan struct{})
Review comment:
I think u need to use a lock to promise that:
```go
close(z.reconnectCh)
z.reconnectCh = make(chan struct{})
```
----------------------------------------------------------------
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] fangyincheng commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
fangyincheng commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r582845439
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -37,11 +37,14 @@ func Test_Register(t *testing.T) {
regURL, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithParamsValue("serviceid", "soa.mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
- ts, reg, _ := newMockZkRegistry(regURL)
+ ts, reg, err := newMockZkRegistry(regURL)
+ if err != nil {
+ assert.NoError(t, err)
Review comment:
so err should be nil or nor？
----------------------------------------------------------------
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] codecov-io commented on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (7d7512e) into [develop](https://codecov.io/gh/apache/dubbo-go/commit/6c989b9d58032a4ff6c172c944921104c6c30739?el=desc) (6c989b9) will **decrease** coverage by `0.17%`.
> The diff coverage is `41.37%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #1010 +/- ##
===========================================
- Coverage 59.41% 59.24% -0.18%
===========================================
Files 261 261
Lines 12952 12888 -64
===========================================
- Hits 7696 7635 -61
+ Misses 4284 4283 -1
+ Partials 972 970 -2
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [config\_center/zookeeper/impl.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnX2NlbnRlci96b29rZWVwZXIvaW1wbC5nbw==) | `70.66% <0.00%> (+0.92%)` | :arrow_up: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `40.00% <14.28%> (-5.23%)` | :arrow_down: |
| [remoting/zookeeper/facade.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2ZhY2FkZS5nbw==) | `25.00% <16.66%> (-41.67%)` | :arrow_down: |
| [remoting/zookeeper/client.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2NsaWVudC5nbw==) | `50.96% <29.62%> (-9.86%)` | :arrow_down: |
| [registry/zookeeper/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3JlZ2lzdHJ5Lmdv) | `46.32% <60.00%> (-3.33%)` | :arrow_down: |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `64.15% <75.00%> (-0.77%)` | :arrow_down: |
| [registry/etcdv3/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL2xpc3RlbmVyLmdv) | `67.50% <100.00%> (+1.71%)` | :arrow_up: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `56.66% <100.00%> (-3.34%)` | :arrow_down: |
| [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `69.23% <100.00%> (+2.06%)` | :arrow_up: |
| [registry/zookeeper/service\_discovery.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3NlcnZpY2VfZGlzY292ZXJ5Lmdv) | `81.20% <100.00%> (ø)` | |
| ... and [10 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [6c989b9...0a215ac](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if active number is zero
- so now codes seems to implement the (2), not (1)?
- or does (1) equal to (2)?
I am not sure.
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587100225
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
Im my opinion,(1) also same with (2), and we need check the activeNumber when try to close the global(one VM) conn
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (9d8c910) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.24%`.
> The diff coverage is `60.80%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.28% -0.25%
==========================================
Files 259 268 +9
Lines 12737 13016 +279
==========================================
+ Hits 7583 7717 +134
- Misses 4199 4336 +137
- Partials 955 963 +8
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [170 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...9d8c910](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r588844623
##########
File path: remoting/getty/getty_client.go
##########
@@ -168,7 +168,7 @@ func (c *Client) Close() {
c.pool = nil
c.mux.Unlock()
if p != nil {
- p.close()
+ c.Close()
Review comment:
should be p.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.
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587060655
##########
File path: metadata/report/zookeeper/report.go
##########
@@ -22,14 +22,16 @@ import (
"time"
)
+import (
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
+)
import (
Review comment:
done
----------------------------------------------------------------
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] zouyx commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r569264774
##########
File path: cluster/router/condition/app_router_test.go
##########
@@ -67,11 +67,11 @@ conditions:
_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
assert.NoError(t, err)
- defer func() {
- err = ts.Stop()
- assert.NoError(t, err)
- z.Close()
- }()
+ //defer func() {
Review comment:
Detele them if useless
##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -308,9 +308,9 @@ tags:
}
func (suite *DynamicTagRouter) TearDownTest() {
- suite.zkClient.Close()
- err := suite.testCluster.Stop()
- suite.Nil(err)
+ //suite.zkClient.Close()
Review comment:
as above
##########
File path: cluster/router/condition/app_router_test.go
##########
@@ -118,11 +118,11 @@ conditions:
_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
assert.NoError(t, err)
- defer func() {
- err = ts.Stop()
- assert.NoError(t, err)
- z.Close()
- }()
+ //defer func() {
Review comment:
As above
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587073782
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
Good question, for the first question:
1.Should we close client in destroy method?
Yes, now the design is one node(VM)<=>one ExchangeClient. So we need close the tcp conn when all the invokers destroyed.
2.If we should, should we close client after active number check?
As the answer in the first question, we need close the conn until all the invokers(in one Node(VM)) destroyed.
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if active number is zero
So
- Now codes seems to implement the (2), not (1)? Or
- Does (1) equals to (2)?
I am not sure.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (991ef61) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.19%`.
> The diff coverage is `60.03%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.34% -0.20%
==========================================
Files 259 268 +9
Lines 12737 13316 +579
==========================================
+ Hits 7583 7902 +319
- Misses 4199 4433 +234
- Partials 955 981 +26
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [171 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...3fdbb01](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r582824652
##########
File path: registry/zookeeper/listener.go
##########
@@ -20,18 +20,14 @@ package zookeeper
import (
"strings"
"sync"
-)
-
-import (
- perrors "github.com/pkg/errors"
-)
-import (
"github.com/apache/dubbo-go/common"
"github.com/apache/dubbo-go/common/logger"
"github.com/apache/dubbo-go/config_center"
"github.com/apache/dubbo-go/registry"
"github.com/apache/dubbo-go/remoting"
+ perrors "github.com/pkg/errors"
Review comment:
done
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584231107
##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
return nil
}
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+ atomic.AddUint32(&client.activeNum, 1)
+}
+
+// decrease number of service using client
+func (client *ExchangeClient) DecreaseActiveNumber() {
Review comment:
done
##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
return nil
}
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+ atomic.AddUint32(&client.activeNum, 1)
Review comment:
done
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587059994
##########
File path: registry/zookeeper/registry.go
##########
@@ -129,7 +130,7 @@ func (r *zkRegistry) InitListeners() {
defer oldDataListener.mutex.Unlock()
r.dataListener.closed = true
recovered := r.dataListener.subscribed
- if len(recovered) > 0 {
+ if recovered != nil && len(recovered) > 0 {
Review comment:
done
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r588805190
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
I am not sure. If tests cover this case, I think there is no problem to some extend.
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587169812
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
now i think here is nonblocking, do you think it is blocking 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.
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r586977500
##########
File path: metadata/report/zookeeper/report.go
##########
@@ -22,14 +22,16 @@ import (
"time"
)
+import (
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
+)
import (
Review comment:
add blank line between two imports
##########
File path: registry/zookeeper/service_discovery.go
##########
@@ -19,15 +19,15 @@ package zookeeper
import (
"fmt"
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
Review comment:
move to sencond import block
##########
File path: remoting/exchange_client.go
##########
@@ -21,6 +21,9 @@ import (
"time"
)
+import (
+ uatomic "go.uber.org/atomic"
+)
import (
Review comment:
add blank line between two import
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
- If destroy method is responsible for client close, then we should close client whatever active number equals or not equals to zero.
- If destroy method is not responsible for client close, then we should not close client in destroy method.
So
- Should we close client in destroy method?
- If we should, should we close client after active number check?
##########
File path: registry/zookeeper/registry.go
##########
@@ -129,7 +130,7 @@ func (r *zkRegistry) InitListeners() {
defer oldDataListener.mutex.Unlock()
r.dataListener.closed = true
recovered := r.dataListener.subscribed
- if len(recovered) > 0 {
+ if recovered != nil && len(recovered) > 0 {
Review comment:
only need `len(recovered) > 0`
##########
File path: registry/zookeeper/registry.go
##########
@@ -297,9 +294,9 @@ func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationL
r.dataListener.mutex.Lock()
configurationListener := r.dataListener.subscribed[conf.ServiceKey()]
if configurationListener != nil {
- rcListener, _ := configurationListener.(*RegistryConfigurationListener)
- if rcListener != nil {
- if rcListener.isClosed {
+ zkListener, _ = configurationListener.(*RegistryConfigurationListener)
+ if zkListener != nil {
+ if zkListener.isClosed {
Review comment:
merge two lines to one line: `if zkListener != nil && zkListener.isClosed`
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587062958
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ client.DecreaseActiveNumber()
Review comment:
done
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (80dedeb) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.13%`.
> The diff coverage is `60.97%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.39% -0.14%
==========================================
Files 259 268 +9
Lines 12737 13017 +280
==========================================
+ Hits 7583 7732 +149
- Misses 4199 4327 +128
- Partials 955 958 +3
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [170 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...9d8c910](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
So now codes seems to implement the (2), not (1)? I'm not very sure.
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if activenumber is zero
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584505312
##########
File path: remoting/zookeeper/client.go
##########
@@ -285,6 +285,11 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
if state == (int)(zk.StateHasSession) {
continue
}
+ if event.State == zk.StateHasSession {
+ atomic.StoreUint32(&z.valid, 1)
+ close(z.reconnectCh)
+ z.reconnectCh = make(chan struct{})
Review comment:
There only have one goroutine run this function, don't need lock..
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (0a215ac) into [develop](https://codecov.io/gh/apache/dubbo-go/commit/6c989b9d58032a4ff6c172c944921104c6c30739?el=desc) (6c989b9) will **decrease** coverage by `0.19%`.
> The diff coverage is `33.96%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #1010 +/- ##
===========================================
- Coverage 59.41% 59.22% -0.20%
===========================================
Files 261 261
Lines 12952 12891 -61
===========================================
- Hits 7696 7635 -61
+ Misses 4284 4282 -2
- Partials 972 974 +2
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [config\_center/zookeeper/impl.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnX2NlbnRlci96b29rZWVwZXIvaW1wbC5nbw==) | `70.66% <0.00%> (+0.92%)` | :arrow_up: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `40.00% <14.28%> (-5.23%)` | :arrow_down: |
| [remoting/zookeeper/facade.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2ZhY2FkZS5nbw==) | `25.00% <16.66%> (-41.67%)` | :arrow_down: |
| [remoting/zookeeper/client.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2NsaWVudC5nbw==) | `50.96% <29.62%> (-9.86%)` | :arrow_down: |
| [...rotocol/protocolwrapper/protocol\_filter\_wrapper.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cHJvdG9jb2wvcHJvdG9jb2x3cmFwcGVyL3Byb3RvY29sX2ZpbHRlcl93cmFwcGVyLmdv) | `48.14% <50.00%> (-1.86%)` | :arrow_down: |
| [registry/zookeeper/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3JlZ2lzdHJ5Lmdv) | `46.32% <60.00%> (-3.33%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `60.00% <100.00%> (ø)` | |
| [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `69.23% <100.00%> (+2.06%)` | :arrow_up: |
| [registry/zookeeper/service\_discovery.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3NlcnZpY2VfZGlzY292ZXJ5Lmdv) | `81.20% <100.00%> (ø)` | |
| ... and [10 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [6c989b9...0a215ac](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] AlexStocks commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r563041392
##########
File path: config_center/zookeeper/impl.go
##########
@@ -20,20 +20,16 @@ package zookeeper
import (
"strings"
"sync"
-)
-import (
gxset "github.com/dubbogo/gost/container/set"
- perrors "github.com/pkg/errors"
-)
-import (
"github.com/apache/dubbo-go/common"
"github.com/apache/dubbo-go/common/constant"
"github.com/apache/dubbo-go/common/logger"
"github.com/apache/dubbo-go/config_center"
"github.com/apache/dubbo-go/config_center/parser"
"github.com/apache/dubbo-go/remoting/zookeeper"
+ perrors "github.com/pkg/errors"
Review comment:
pls move it to the 2rd import block.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (991ef61) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.19%`.
> The diff coverage is `60.03%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.34% -0.20%
==========================================
Files 259 268 +9
Lines 12737 13316 +579
==========================================
+ Hits 7583 7902 +319
- Misses 4199 4433 +234
- Partials 955 981 +26
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [171 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...991ef61](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] cityiron commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
cityiron commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r563023168
##########
File path: config_center/zookeeper/impl.go
##########
@@ -20,20 +20,16 @@ package zookeeper
import (
Review comment:
we split 3 import in dubbogo
##########
File path: registry/kubernetes/registry.go
##########
@@ -22,21 +22,18 @@ import (
"path"
"sync"
"time"
Review comment:
like above
##########
File path: registry/zookeeper/listener.go
##########
@@ -20,18 +20,14 @@ package zookeeper
import (
"strings"
"sync"
-)
-
-import (
- perrors "github.com/pkg/errors"
-)
-import (
"github.com/apache/dubbo-go/common"
"github.com/apache/dubbo-go/common/logger"
"github.com/apache/dubbo-go/config_center"
"github.com/apache/dubbo-go/registry"
"github.com/apache/dubbo-go/remoting"
+ perrors "github.com/pkg/errors"
Review comment:
like above
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r563057726
##########
File path: config_center/zookeeper/impl.go
##########
@@ -209,8 +205,6 @@ func (c *zookeeperDynamicConfiguration) closeConfigs() {
c.cltLock.Lock()
defer c.cltLock.Unlock()
logger.Infof("begin to close provider zk client")
- // Close the old client first to close the tmp node
- c.client.Close()
Review comment:
所有的registry公用一个zkClient， 不需要每个registry退出就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.
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r588844589
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -37,11 +37,14 @@ func Test_Register(t *testing.T) {
regURL, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithParamsValue("serviceid", "soa.mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
- ts, reg, _ := newMockZkRegistry(regURL)
+ ts, reg, err := newMockZkRegistry(regURL)
+ if err != nil {
+ assert.NoError(t, err)
Review comment:
bad code, changed now
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (3fdbb01) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.04%`.
> The diff coverage is `60.92%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.48% -0.05%
==========================================
Files 259 268 +9
Lines 12737 13317 +580
==========================================
+ Hits 7583 7922 +339
- Misses 4199 4409 +210
- Partials 955 986 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [173 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...ef8966c](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#issuecomment-767490394
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=h1) Report
> Merging [#1010](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=desc) (3fdbb01) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/968650f658b63c11bb0409897d29c57b91cfaf50?el=desc) (968650f) will **decrease** coverage by `0.04%`.
> The diff coverage is `60.92%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 1.5 #1010 +/- ##
==========================================
- Coverage 59.53% 59.48% -0.05%
==========================================
Files 259 268 +9
Lines 12737 13317 +580
==========================================
+ Hits 7583 7922 +339
- Misses 4199 4409 +210
- Partials 955 986 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/cluster\_impl/available\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYXZhaWxhYmxlX2NsdXN0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `64.28% <ø> (ø)` | |
| [cluster/router/healthcheck/factory.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvaGVhbHRoY2hlY2svZmFjdG9yeS5nbw==) | `66.66% <0.00%> (ø)` | |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `89.47% <ø> (+12.20%)` | :arrow_up: |
| [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `51.72% <ø> (ø)` | |
| [common/extension/config\_post\_processor.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcG9zdF9wcm9jZXNzb3IuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/conn\_checker.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25uX2NoZWNrZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9zZXJ2aWNlLmdv) | `0.00% <0.00%> (ø)` | |
| [common/proxy/proxy\_factory/default.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29tbW9uL3Byb3h5L3Byb3h5X2ZhY3RvcnkvZGVmYXVsdC5nbw==) | `13.55% <0.00%> (+0.22%)` | :arrow_up: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.02% <ø> (ø)` | |
| ... and [173 more](https://codecov.io/gh/apache/dubbo-go/pull/1010/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=footer). Last update [f1f81eb...ca647ab](https://codecov.io/gh/apache/dubbo-go/pull/1010?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584231113
##########
File path: remoting/exchange_client.go
##########
@@ -51,6 +52,8 @@ type ExchangeClient struct {
client Client
// the tag for init.
init bool
+ // the number of service using the exchangeClient
+ activeNum uint32
Review comment:
Why uber atomic is more useful?
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if active number is zero
- So now codes seems to implement the (2), not (1)? Or
- Does (1) equals to (2)?
I am not sure.
----------------------------------------------------------------
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 #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587096223
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
(1) close the conn until all the invokers (in one Node(VM)) destroyed
(2) close the conn if active number is zero
- So now codes seems to implement the (2), not (1)? Or
- Does (1) equal to (2)?
I am not sure.
----------------------------------------------------------------
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] wenxuwan commented on a change in pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r587060079
##########
File path: registry/zookeeper/registry.go
##########
@@ -297,9 +294,9 @@ func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationL
r.dataListener.mutex.Lock()
configurationListener := r.dataListener.subscribed[conf.ServiceKey()]
if configurationListener != nil {
- rcListener, _ := configurationListener.(*RegistryConfigurationListener)
- if rcListener != nil {
- if rcListener.isClosed {
+ zkListener, _ = configurationListener.(*RegistryConfigurationListener)
+ if zkListener != nil {
+ if zkListener.isClosed {
Review comment:
done
##########
File path: remoting/exchange_client.go
##########
@@ -21,6 +21,9 @@ import (
"time"
)
+import (
+ uatomic "go.uber.org/atomic"
+)
import (
Review comment:
done
##########
File path: registry/zookeeper/service_discovery.go
##########
@@ -19,15 +19,15 @@ package zookeeper
import (
"fmt"
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
Review comment:
done
----------------------------------------------------------------
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] AlexStocks merged pull request #1010: Fix:zk too many tcp conn
Posted by GitBox <gi...@apache.org>.
AlexStocks merged pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010
----------------------------------------------------------------
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