Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue-8695:DefaultFuture turn off logging optimization for Channel #8775

Merged
merged 2 commits into from Sep 12, 2021

Conversation

stone-98
Copy link
Contributor

What is the purpose of the change

fix #8695 on 3.0

Brief changelog

DefaultFuture turn off logging optimization for Channel.

Request newRequest = request.copy();
newRequest.setData(null);
return newRequest;
private static Request getRequestWithoutData(Request request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can be removed. We can use request. copyWithoutData() directly.

@@ -147,7 +147,7 @@ public static void closeChannel(Channel channel) {
disconnectResponse.setErrorMessage("Channel " +
channel +
" is inactive. Directly return the unFinished request : " +
future.getRequest());
getRequestWithoutData(future.getRequest()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here use logger.isDebugEnabled() to judge is print data also.

@stone-98
Copy link
Contributor Author

@horizonzy I have completed the optimization you mentioned. This is my first submission, and I want to know about the submission of the issue. How to determine which branch to merge into?

@horizonzy
Copy link
Member

@horizonzy I have completed the optimization you mentioned. This is my first submission, and I want to know about the submission of the issue. How to determine which branch to merge into?

Now we use brnach 3.0 as our default branch. And branch master also be maintained, you can pr to master again.

Copy link
Member

@horizonzy horizonzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2021

Codecov Report

Merging #8775 (4af675b) into 3.0 (2bd54ae) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.0    #8775      +/-   ##
============================================
+ Coverage     63.72%   63.74%   +0.01%     
  Complexity      314      314              
============================================
  Files          1149     1149              
  Lines         48347    48350       +3     
  Branches       7291     7292       +1     
============================================
+ Hits          30810    30819       +9     
+ Misses        14158    14156       -2     
+ Partials       3379     3375       -4     
Impacted Files Coverage Δ
...va/org/apache/dubbo/remoting/exchange/Request.java 77.19% <100.00%> (-11.05%) ⬇️
...dubbo/remoting/exchange/support/DefaultFuture.java 89.18% <100.00%> (-0.29%) ⬇️
.../client/metadata/ServiceInstanceMetadataUtils.java 84.61% <0.00%> (-2.89%) ⬇️
...va/org/apache/dubbo/registry/RegistryNotifier.java 84.61% <0.00%> (-2.57%) ⬇️
...apache/dubbo/common/extension/ExtensionLoader.java 80.89% <0.00%> (-0.20%) ⬇️
...n/java/org/apache/dubbo/common/utils/NetUtils.java 69.28% <0.00%> (+0.32%) ⬆️
...bo/registry/client/migration/MigrationInvoker.java 58.82% <0.00%> (+0.84%) ⬆️
...ting/zookeeper/curator/CuratorZookeeperClient.java 75.36% <0.00%> (+0.98%) ⬆️
.../src/main/java/org/apache/dubbo/rpc/RpcStatus.java 71.42% <0.00%> (+1.19%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bd54ae...4af675b. Read the comment docs.

@horizonzy horizonzy added this to the 3.0.3 milestone Sep 12, 2021
@horizonzy horizonzy merged commit 26152d8 into apache:3.0 Sep 12, 2021
@stone-98
Copy link
Contributor Author

@horizonzy I have completed the optimization you mentioned. This is my first submission, and I want to know about the submission of the issue. How to determine which branch to merge into?

Now we use brnach 3.0 as our default branch. And branch master also be maintained, you can pr to master again.

I've submitted to the Master branch again. link #8778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

org.apache.dubbo.remoting.exchange.support.DefaultFuture doWithCloseChannel will logRequestData
3 participants