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

Do not close connection by url when connection closed. #498

Merged
merged 6 commits into from Feb 20, 2019

Conversation

@leizhiyuan
Copy link
Contributor

commented Jan 28, 2019

Motivation:

do not close connection with connection.close.

please see #488

Modification:

Describe the idea and modifications you've done.

Result:

Fixes #.

If there is no issue then describe the changes introduced by this PR.

@leizhiyuan leizhiyuan requested review from ujjboy, NeGnail and JervyShi Jan 28, 2019
@leizhiyuan leizhiyuan added this to the 5.5.1 milestone Jan 28, 2019
@@ -126,7 +126,7 @@ public Connection getConnection(RpcClient rpcClient, ClientTransportConfig trans
if (LOGGER.isWarnEnabled()) {
LOGGER.warn("Multiple threads init ClientTransport with same key:" + url);
}
connection.close(); //如果同时有人插入,则使用第一个
rpcClient.closeStandaloneConnection(connection); //如果同时有人插入,则使用第一个

This comment has been minimized.

Copy link
@JervyShi

JervyShi Feb 14, 2019

Member

closeStandaloneConnection 对应的是 RpcClient#createStandaloneConnection 创建出的连接。我们这里用的是 RpcClient#getConnection 这个应该是自带 connectionManager 的。

This comment has been minimized.

Copy link
@leizhiyuan

leizhiyuan Feb 14, 2019

Author Contributor

是的,自带,我们这个是为了修复那个bug。

@@ -188,7 +188,7 @@ public void closeConnection(RpcClient rpcClient, ClientTransportConfig transport
}
}
if (needDestroy) {
rpcClient.closeConnection(url);
rpcClient.closeStandaloneConnection(connection);

This comment has been minimized.

Copy link
@JervyShi

JervyShi Feb 14, 2019

Member

同上

@codecov

This comment has been minimized.

Copy link

commented Feb 14, 2019

Codecov Report

Merging #498 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #498   +/-   ##
=========================================
  Coverage     71.35%   71.35%           
  Complexity     1144     1144           
=========================================
  Files           372      372           
  Lines         15711    15711           
  Branches       2546     2546           
=========================================
  Hits          11210    11210           
  Misses         3154     3154           
  Partials       1347     1347
Impacted Files Coverage Δ Complexity Δ
...pc/transport/bolt/BoltClientConnectionManager.java 76.71% <100%> (ø) 23 <0> (ø) ⬇️

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 83833b6...3211444. Read the comment docs.

leizhiyuan added 3 commits Feb 20, 2019
@ujjboy
ujjboy approved these changes Feb 20, 2019
@ujjboy ujjboy merged commit adf9544 into sofastack:master Feb 20, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
clahub All contributors have signed the Contributor License Agreement.
Details
@ujjboy ujjboy changed the title do not close connection with connection.close Do not close connection by url when connection closed. Feb 20, 2019
@ujjboy ujjboy added this to In progress in v5.5.x via automation Feb 20, 2019
@leizhiyuan leizhiyuan deleted the leizhiyuan:fix_bolt_connection_bug branch Feb 20, 2019
@ujjboy ujjboy added the optimization label Feb 20, 2019
@ujjboy ujjboy moved this from In progress to Done in v5.5.x Feb 20, 2019
treenewtreenew added a commit to treenewtreenew/sofa-rpc that referenced this pull request Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v5.5.x
  
Done
4 participants
You can’t perform that action at this time.