You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "carryxyh (GitHub)" <gi...@apache.org> on 2018/12/19 12:17:45 UTC
[GitHub] [incubator-dubbo] carryxyh opened pull request #3017:
Refresh invocation's attachments in each invoke.
Refresh invocation's attachments in each invoke.
Fix bug:
https://github.com/apache/incubator-dubbo/issues/2981
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @carryxyh . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
https://github.com/apache/incubator-dubbo/pull/3331
@carryxyh please help to review.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
yes.
This problem does happen in the process of using Thrift.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Cannot we just remove or replace the INTERFACE_KEY with the right one?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
A proposal, not strictly related to this issue, only a thought.
There are [Invocation.attachments](), [RpcContext.attachments](), [RpcContext.server_attachments](), we should have someone to figure out the relations, scopes and possible problems each kind of attachment.
Some problems I already know
1.
```java
// `key1` will be passed though the RPC chain to the bottom: A -> B -> C -> D
RpcContext.getContext.setAttachment("key1", "value1");
```
2.
```java
// `server-key1` will be passed though the RPC chain to the top: A <- B <- C <- D
RpcContext.getServerContext.setAttachment("server-key1", "server-value1");
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3017: Refresh
invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@chickenlj
Already fix it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Good job, we can try to simplify the Dubbo Protocol further https://github.com/apache/incubator-dubbo/issues/3328 in the future
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
The only place to use INTERFACE_KEY is in ThriftProtocol:
https://github.com/apache/incubator-dubbo/blob/4d78772fc8c20c8404406444b32d6a9aed92204f/dubbo-rpc/dubbo-rpc-thrift/src/main/java/org/apache/dubbo/rpc/protocol/thrift/ThriftProtocol.java#L66
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Ok!
i haven't notice it yet...
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/5ea4fa531f9ec5ae446d4cb1ef2417b53712bd46?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
===========================================
- Coverage 63.43% 63.4% -0.03%
Complexity 75 75
===========================================
Files 653 653
Lines 28261 28263 +2
Branches 4820 4821 +1
===========================================
- Hits 17926 17919 -7
- Misses 8057 8066 +9
Partials 2278 2278
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `66.12% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...bo/rpc/cluster/support/AbstractClusterInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9BYnN0cmFjdENsdXN0ZXJJbnZva2VyLmphdmE=) | `76% <50%> (-0.54%)` | `0 <0> (ø)` | |
| [.../main/java/org/apache/dubbo/rpc/RpcInvocation.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNJbnZvY2F0aW9uLmphdmE=) | `55.81% <0%> (-6.98%)` | `0% <0%> (ø)` | |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `82.75% <0%> (-3.45%)` | `0% <0%> (ø)` | |
| [...he/dubbo/registry/multicast/MulticastRegistry.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktbXVsdGljYXN0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9tdWx0aWNhc3QvTXVsdGljYXN0UmVnaXN0cnkuamF2YQ==) | `63.79% <0%> (-1.73%)` | `0% <0%> (ø)` | |
| [...dubbo/metadata/support/AbstractMetadataReport.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tbWV0YWRhdGEtcmVwb3J0L2R1YmJvLW1ldGFkYXRhLXJlcG9ydC1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL21ldGFkYXRhL3N1cHBvcnQvQWJzdHJhY3RNZXRhZGF0YVJlcG9ydC5qYXZh) | `69.35% <0%> (-1.62%)` | `0% <0%> (ø)` | |
| [...pache/dubbo/remoting/transport/AbstractServer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RTZXJ2ZXIuamF2YQ==) | `47.91% <0%> (-1.05%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `63.04% <0%> (+2.17%)` | `0% <0%> (ø)` | :arrow_down: |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `62.35% <0%> (+4.7%)` | `0% <0%> (ø)` | :arrow_down: |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `85.36% <0%> (+4.87%)` | `0% <0%> (ø)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [5ea4fa5...a4febd7](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I will try to update the Thrift protocol with another PR.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
What if user has overridden the `timeout` or `token` value in RpcContext/Invocation, if you put replace `addAttachmentsIfAbsent ` with `addAttachments` and put it at the last place, the user will never has a chance to override the keys listed here https://github.com/apache/incubator-dubbo/blob/e07038b00e057bb3cd6989252534b581039916f9/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/protocol/AbstractInvoker.java#L78
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/859efeff747587c10cae4fdbfbf38719ec44e009?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
=========================================
+ Coverage 64.28% 64.3% +0.02%
=========================================
Files 584 584
Lines 26056 26058 +2
Branches 4562 4563 +1
=========================================
+ Hits 16749 16756 +7
- Misses 7124 7125 +1
+ Partials 2183 2177 -6
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `66.12% <100%> (ø)` | :arrow_up: |
| [...bo/rpc/cluster/support/AbstractClusterInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9BYnN0cmFjdENsdXN0ZXJJbnZva2VyLmphdmE=) | `76% <50%> (-0.54%)` | :arrow_down: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | :arrow_down: |
| [.../main/java/org/apache/dubbo/rpc/RpcInvocation.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNJbnZvY2F0aW9uLmphdmE=) | `55.81% <0%> (-6.98%)` | :arrow_down: |
| [.../apache/dubbo/qos/protocol/QosProtocolWrapper.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL3Byb3RvY29sL1Fvc1Byb3RvY29sV3JhcHBlci5qYXZh) | `64.1% <0%> (-5.13%)` | :arrow_down: |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `60.86% <0%> (-2.18%)` | :arrow_down: |
| [.../java/org/apache/dubbo/config/ReferenceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9SZWZlcmVuY2VDb25maWcuamF2YQ==) | `56.01% <0%> (-0.38%)` | :arrow_down: |
| [...pache/dubbo/registry/support/AbstractRegistry.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L0Fic3RyYWN0UmVnaXN0cnkuamF2YQ==) | `81.95% <0%> (+0.37%)` | :arrow_up: |
| [...pache/dubbo/remoting/transport/AbstractServer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RTZXJ2ZXIuamF2YQ==) | `48.95% <0%> (+1.04%)` | :arrow_up: |
| [.../src/main/java/org/apache/dubbo/rpc/RpcStatus.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNTdGF0dXMuamF2YQ==) | `70% <0%> (+2.22%)` | :arrow_up: |
| ... and [4 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [859efef...c73cce4](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/5ea4fa531f9ec5ae446d4cb1ef2417b53712bd46?src=pr&el=desc) will **increase** coverage by `0.43%`.
> The diff coverage is `62.41%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
============================================
+ Coverage 63.43% 63.86% +0.43%
Complexity 75 75
============================================
Files 653 661 +8
Lines 28261 28624 +363
Branches 4820 4826 +6
============================================
+ Hits 17926 18282 +356
- Misses 8057 8132 +75
+ Partials 2278 2210 -68
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...onfig/spring/schema/DubboBeanDefinitionParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc2NoZW1hL0R1YmJvQmVhbkRlZmluaXRpb25QYXJzZXIuamF2YQ==) | `67.27% <ø> (+0.9%)` | `0 <0> (ø)` | :arrow_down: |
| [...in/java/org/apache/dubbo/config/MonitorConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Nb25pdG9yQ29uZmlnLmphdmE=) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/context/properties/DefaultDubboConfigBinder.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvY29udGV4dC9wcm9wZXJ0aWVzL0RlZmF1bHREdWJib0NvbmZpZ0JpbmRlci5qYXZh) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...rg/apache/dubbo/config/spring/util/ClassUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9DbGFzc1V0aWxzLmphdmE=) | `75% <ø> (ø)` | `0 <0> (?)` | |
| [...pache/dubbo/common/config/SystemConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL1N5c3RlbUNvbmZpZ3VyYXRpb24uamF2YQ==) | `60% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/cluster/RouterChain.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvUm91dGVyQ2hhaW4uamF2YQ==) | `96.87% <ø> (-0.19%)` | `0 <0> (ø)` | |
| [...er/condition/config/model/ConditionRuleParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUnVsZVBhcnNlci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [.../config/spring/status/DataSourceStatusChecker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc3RhdHVzL0RhdGFTb3VyY2VTdGF0dXNDaGVja2VyLmphdmE=) | `74.35% <ø> (+2.56%)` | `0 <0> (ø)` | :arrow_down: |
| [...er/condition/config/model/ConditionRouterRule.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUm91dGVyUnVsZS5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...dubbo/config/spring/util/PropertySourcesUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9Qcm9wZXJ0eVNvdXJjZXNVdGlscy5qYXZh) | `86.95% <ø> (+1.24%)` | `0 <0> (ø)` | :arrow_down: |
| ... and [245 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [5ea4fa5...a4febd7](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3017: Refresh invocation's attachments in each invoke.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Just curious to know, as comment indicates refreshing timeout, group, and token. Do we need any place to be also refresh other than invocation?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Hey, maybe the description here is a bit problematic. We are not refreshing but covering. I will modify it later.
The global attachment here is information such as interface and timeout obtained from the URL. We want to make sure that the interface and timeout used for each call are in the url of the current invoker, not the other way. Other ways such as: RpcContext.getContext.setAttachment
So, before invoke, we should override these info.
:)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/4d78772fc8c20c8404406444b32d6a9aed92204f?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
============================================
- Coverage 63.82% 63.79% -0.03%
Complexity 75 75
============================================
Files 661 661
Lines 28624 28625 +1
Branches 4826 4826
============================================
- Hits 18268 18262 -6
- Misses 8144 8149 +5
- Partials 2212 2214 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `62.9% <ø> (-3.23%)` | `0 <0> (ø)` | |
| [...ava/org/apache/dubbo/rpc/filter/ContextFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvQ29udGV4dEZpbHRlci5qYXZh) | `93.33% <100%> (+0.22%)` | `0 <0> (ø)` | :arrow_down: |
| [...bo/rpc/cluster/support/AbstractClusterInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9BYnN0cmFjdENsdXN0ZXJJbnZva2VyLmphdmE=) | `76.53% <50%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-8.7%)` | `0% <0%> (ø)` | |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | `0% <0%> (ø)` | |
| [...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tbW9uaXRvci9kdWJiby1tb25pdG9yLWRlZmF1bHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL21vbml0b3IvZHViYm8vRHViYm9Nb25pdG9yLmphdmE=) | `87.85% <0%> (-1.87%)` | `0% <0%> (ø)` | |
| [...dubbo/remoting/exchange/support/DefaultFuture.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L0RlZmF1bHRGdXR1cmUuamF2YQ==) | `69.59% <0%> (-1.36%)` | `0% <0%> (ø)` | |
| [...rg/apache/dubbo/common/timer/HashedWheelTimer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdGltZXIvSGFzaGVkV2hlZWxUaW1lci5qYXZh) | `57.83% <0%> (-0.35%)` | `0% <0%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [4d78772...effefd4](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3017: Refresh
invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Ok, thx for your review.
:)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/5ea4fa531f9ec5ae446d4cb1ef2417b53712bd46?src=pr&el=desc) will **increase** coverage by `0.39%`.
> The diff coverage is `62.41%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
============================================
+ Coverage 63.43% 63.82% +0.39%
Complexity 75 75
============================================
Files 653 661 +8
Lines 28261 28624 +363
Branches 4820 4826 +6
============================================
+ Hits 17926 18268 +342
- Misses 8057 8144 +87
+ Partials 2278 2212 -66
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...onfig/spring/schema/DubboBeanDefinitionParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc2NoZW1hL0R1YmJvQmVhbkRlZmluaXRpb25QYXJzZXIuamF2YQ==) | `67.27% <ø> (+0.9%)` | `0 <0> (ø)` | :arrow_down: |
| [...in/java/org/apache/dubbo/config/MonitorConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Nb25pdG9yQ29uZmlnLmphdmE=) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/context/properties/DefaultDubboConfigBinder.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvY29udGV4dC9wcm9wZXJ0aWVzL0RlZmF1bHREdWJib0NvbmZpZ0JpbmRlci5qYXZh) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...rg/apache/dubbo/config/spring/util/ClassUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9DbGFzc1V0aWxzLmphdmE=) | `75% <ø> (ø)` | `0 <0> (?)` | |
| [...pache/dubbo/common/config/SystemConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL1N5c3RlbUNvbmZpZ3VyYXRpb24uamF2YQ==) | `60% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/cluster/RouterChain.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvUm91dGVyQ2hhaW4uamF2YQ==) | `96.87% <ø> (-0.19%)` | `0 <0> (ø)` | |
| [...er/condition/config/model/ConditionRuleParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUnVsZVBhcnNlci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [.../config/spring/status/DataSourceStatusChecker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc3RhdHVzL0RhdGFTb3VyY2VTdGF0dXNDaGVja2VyLmphdmE=) | `74.35% <ø> (+2.56%)` | `0 <0> (ø)` | :arrow_down: |
| [...er/condition/config/model/ConditionRouterRule.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUm91dGVyUnVsZS5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...dubbo/config/spring/util/PropertySourcesUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9Qcm9wZXJ0eVNvdXJjZXNVdGlscy5qYXZh) | `86.95% <ø> (+1.24%)` | `0 <0> (ø)` | :arrow_down: |
| ... and [245 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [5ea4fa5...a4febd7](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
> What if user has overridden the timeout or token value in RpcContext/Invocation, if you put replace addAttachmentsIfAbsent with addAttachments and put it at the last place, the user will never has a chance to override the keys listed here
I have previously considered using RpcContext to modify the timeout (group) scenario. I personally are not sure if there is a intention to leave a back door or a design flaw. But I rarely encounter the need to use RpcContext to modify the timeout, so the overlay is used directly.
I have previously considered using RpcContext to modify the timeout (token) scenario. I personally are not sure if there is a intention to leave a back door or a design flaw. But I rarely encounter the need to use RpcContext to modify the timeout, so the overlay is used directly.
> Cannot we just remove or replace the INTERFACE_KEY with the right one?
Yes, we can.
But it seems that `timeout` and `group` may have the same problem as interface. For example, in the link of `A -> B -> C`, `B` receives the `timeout` from `A`. At this time, in the link where `B` calls `C`, it is possible to use the timeout of `A -> B` instead of the timeout of `B -> C`. Should we try to avoid this risk?
> I even find that the INTERFACE_KEY useless on the wire.
I see that there are still many places that quote INTERFACE_KEY. Is it my understanding of what you mean wrong?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj closed pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
[ pull request closed by chickenlj ]
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/859efeff747587c10cae4fdbfbf38719ec44e009?src=pr&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
==========================================
- Coverage 64.28% 64.21% -0.08%
==========================================
Files 584 584
Lines 26056 26058 +2
Branches 4562 4563 +1
==========================================
- Hits 16751 16734 -17
- Misses 7123 7142 +19
Partials 2182 2182
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `66.12% <100%> (ø)` | :arrow_up: |
| [...bo/rpc/cluster/support/AbstractClusterInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9BYnN0cmFjdENsdXN0ZXJJbnZva2VyLmphdmE=) | `76% <50%> (-0.54%)` | :arrow_down: |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | :arrow_down: |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `75% <0%> (-11.12%)` | :arrow_down: |
| [.../main/java/org/apache/dubbo/rpc/RpcInvocation.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNJbnZvY2F0aW9uLmphdmE=) | `55.81% <0%> (-6.98%)` | :arrow_down: |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | :arrow_down: |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-4.35%)` | :arrow_down: |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `85.71% <0%> (-3.58%)` | :arrow_down: |
| [...rg/apache/dubbo/common/timer/HashedWheelTimer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdGltZXIvSGFzaGVkV2hlZWxUaW1lci5qYXZh) | `60.62% <0%> (-1.75%)` | :arrow_down: |
| [...pache/dubbo/registry/support/AbstractRegistry.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L0Fic3RyYWN0UmVnaXN0cnkuamF2YQ==) | `80.07% <0%> (-1.51%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [859efef...5b9b98b](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3017: Refresh
invocation's attachments in each invoke.
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
yep, I'd like open an issue to track this.
What is your opinion on this pr?
If we just use the correct interface, but timeout, group, and token may still have the same problem as the interface. Should we consider this scenario?
Or should we modify this issue after adjusting the issue of attachment?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
> I even find that the INTERFACE_KEY useless on the wire.
>
> I see that there are still many places that quote INTERFACE_KEY. Is it my understanding of what you mean wrong?
I mean the necessity of passing INTERFACE_KEY from consumer to provider. Does provider ever uses the INTERFACE_KEY received from consumer?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Good job, we can try to simplify the Dubbo Protocol further https://github.com/apache/incubator-dubbo/issues/3328
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
https://github.com/apache/incubator-dubbo/blob/33f1726713a4c5f2ce5880613acf2320ae2a4502/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ContextFilter.java#L46
I am simply thinking of remove INTERFACE_KEY in here. GROUP_KEY, TOKEN_KEY and TIMEOUT_KEY would not have this problem because they've already been removed.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3017:
Refresh invocation's attachments in each invoke.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=h1) Report
> Merging [#3017](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/5ea4fa531f9ec5ae446d4cb1ef2417b53712bd46?src=pr&el=desc) will **increase** coverage by `0.35%`.
> The diff coverage is `62.41%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3017 +/- ##
============================================
+ Coverage 63.43% 63.78% +0.35%
Complexity 75 75
============================================
Files 653 661 +8
Lines 28261 28624 +363
Branches 4820 4826 +6
============================================
+ Hits 17926 18259 +333
- Misses 8057 8156 +99
+ Partials 2278 2209 -69
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...onfig/spring/schema/DubboBeanDefinitionParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc2NoZW1hL0R1YmJvQmVhbkRlZmluaXRpb25QYXJzZXIuamF2YQ==) | `67.27% <ø> (+0.9%)` | `0 <0> (ø)` | :arrow_down: |
| [...in/java/org/apache/dubbo/config/MonitorConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Nb25pdG9yQ29uZmlnLmphdmE=) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/context/properties/DefaultDubboConfigBinder.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvY29udGV4dC9wcm9wZXJ0aWVzL0RlZmF1bHREdWJib0NvbmZpZ0JpbmRlci5qYXZh) | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...rg/apache/dubbo/config/spring/util/ClassUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9DbGFzc1V0aWxzLmphdmE=) | `75% <ø> (ø)` | `0 <0> (?)` | |
| [...pache/dubbo/common/config/SystemConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL1N5c3RlbUNvbmZpZ3VyYXRpb24uamF2YQ==) | `60% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...java/org/apache/dubbo/rpc/cluster/RouterChain.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvUm91dGVyQ2hhaW4uamF2YQ==) | `96.87% <ø> (-0.19%)` | `0 <0> (ø)` | |
| [...er/condition/config/model/ConditionRuleParser.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUnVsZVBhcnNlci5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [.../config/spring/status/DataSourceStatusChecker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvc3RhdHVzL0RhdGFTb3VyY2VTdGF0dXNDaGVja2VyLmphdmE=) | `74.35% <ø> (+2.56%)` | `0 <0> (ø)` | :arrow_down: |
| [...er/condition/config/model/ConditionRouterRule.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2NvbmRpdGlvbi9jb25maWcvbW9kZWwvQ29uZGl0aW9uUm91dGVyUnVsZS5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...dubbo/config/spring/util/PropertySourcesUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvdXRpbC9Qcm9wZXJ0eVNvdXJjZXNVdGlscy5qYXZh) | `86.95% <ø> (+1.24%)` | `0 <0> (ø)` | :arrow_down: |
| ... and [248 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3017/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?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/incubator-dubbo/pull/3017?src=pr&el=footer). Last update [5ea4fa5...a4febd7](https://codecov.io/gh/apache/incubator-dubbo/pull/3017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3017:
Refresh invocation's attachments in each invoke.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I even find that the INTERFACE_KEY useless on the wire. Instead `path` was used instead:
https://github.com/apache/incubator-dubbo/blob/e07038b00e057bb3cd6989252534b581039916f9/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java#L215
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3017: Refresh invocation's attachments in each invoke.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@carryxyh thanks for the info.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3017 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org