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.

Sign up for GitHub

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

Jump to bottom

[Issue 6319][Pulsar client] connection leak fix #6524

Merged
merged 6 commits into from
Jun 11, 2020
Merged

[Issue 6319][Pulsar client] connection leak fix #6524

merged 6 commits into from
Jun 11, 2020

Conversation

ltamber
Copy link
Contributor

@ltamber ltamber commented Mar 11, 2020

Fixes #6319

@ltamber
Copy link
Contributor Author

ltamber commented Mar 11, 2020

@sijie @jiazhai

@ltamber ltamber changed the title connection leak fix [Pulsar client] connection leak fix Mar 14, 2020
@ltamber ltamber changed the title [Pulsar client] connection leak fix [Issue 6319][Pulsar client] connection leak fix Mar 14, 2020
@sijie sijie added this to the 2.6.0 milestone Mar 16, 2020
@sijie sijie added area/broker area/client type/bug The PR fixed a bug or issue reported a bug and removed area/broker labels Mar 16, 2020
codelipenghui
codelipenghui reviewed Mar 16, 2020
View reviewed changes
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Is it possible to add some tests for these changes?

@jiazhai
Copy link
Member

jiazhai commented Apr 4, 2020

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Apr 4, 2020

@ltamber Thanks a lot for the fix. As Penghui suggested, is it possible to add some tests to protect this part of change?

@ltamber
Copy link
Contributor Author

ltamber commented Apr 6, 2020

Sorry forgot this, I will add some tests as soon as possible

@ltamber
Copy link
Contributor Author

ltamber commented Apr 12, 2020

@jiazhai I added some unit tests, Is it possible to review this PR again ?

@sijie
Copy link
Member

sijie commented Apr 21, 2020

@codelipenghui @jiazhai can you review this pull request again?

codelipenghui
codelipenghui approved these changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Looks good to me, please fix the license header.

@ltamber
Copy link
Contributor Author

ltamber commented Apr 23, 2020

Looks good to me, please fix the license header.

sorry for that, have been fixed, please take a look @codelipenghui

@ltamber
Copy link
Contributor Author

ltamber commented Apr 26, 2020

/pulsarbot run-failure-checks

1 similar comment
@jiazhai
Copy link
Member

jiazhai commented May 10, 2020

/pulsarbot run-failure-checks

@sijie sijie mentioned this pull request May 20, 2020
Closed
@codelipenghui
Copy link
Contributor

@ltamber Could you please help resolve the conflicts?

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

2 similar comments
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor 

[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 1.845 s <<< FAILURE! - in org.apache.pulsar.client.api.PulsarMultiListenersWithoutInternalListenerNameTest
[ERROR] testFindBrokerWithListenerName(org.apache.pulsar.client.api.PulsarMultiListenersWithoutInternalListenerNameTest)  Time elapsed: 0.204 s  <<< FAILURE!
java.lang.AssertionError: expected [10.1.0.4:6650] but found [fv-az50.internal.cloudapp.net:45101]
	at org.testng.Assert.fail(Assert.java:96)
	at org.testng.Assert.failNotEquals(Assert.java:776)
	at org.testng.Assert.assertEqualsImpl(Assert.java:137)
	at org.testng.Assert.assertEquals(Assert.java:118)
	at org.testng.Assert.assertEquals(Assert.java:453)
	at org.testng.Assert.assertEquals(Assert.java:463)
	at org.apache.pulsar.client.api.PulsarMultiListenersWithoutInternalListenerNameTest.testFindBrokerWithListenerName(PulsarMultiListenersWithoutInternalListenerNameTest.java:83)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124)
	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:54)
	at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:44)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

@codelipenghui
Copy link
Contributor

@ltamber Looks the failed tests relate to this PR. I will move it to 2.7.0 first. If you have time, please take a look, thanks.

@codelipenghui codelipenghui modified the milestones: 2.6.0, 2.7.0 Jun 5, 2020
@ltamber
Copy link
Contributor Author

ltamber commented Jun 10, 2020

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 0c9c9fc into apache:master Jun 11, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
@ltamber @huangdx0726
[Issue 6319][Pulsar client] connection leak fix (apache#6524)
b2c4ad7
@codelipenghui
Copy link
Contributor

/pulsarbot cherry-pick to branch-2.6

github-actions bot pushed a commit that referenced this pull request Nov 20, 2020
@ltamber @codelipenghui
[Issue 6319][Pulsar client] connection leak fix (#6524)
079607a
codelipenghui added a commit that referenced this pull request Nov 20, 2020
@codelipenghui
[Issue 6319][Pulsar client] connection leak fix (#6524) (#8642)
30db702 
cherry-pick from 0c9c9fc #6524
codelipenghui added a commit to streamnative/pulsar-archived that referenced this pull request Nov 20, 2020
@codelipenghui
[Issue 6319][Pulsar client] connection leak fix (apache#6524) (apache…
11d25bc 
…#8642)

cherry-pick from 0c9c9fc apache#6524 

(cherry picked from commit 30db702)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiazhai jiazhai jiazhai approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@rdhabalia rdhabalia Awaiting requested review from rdhabalia

@merlimat merlimat Awaiting requested review from merlimat

Assignees

@ltamber ltamber

Labels
area/client release/2.6.3 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.

[pulsar client] partitioned topic cause connection leak when connection pool disable pooling
4 participants
@ltamber @jiazhai @sijie @codelipenghui