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 #472] move GetPartitionedTopicMetadata to lookup service #478

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

freeznet
Copy link
Contributor

@freeznet freeznet commented Mar 2, 2021

Fixes #472

Motivation

#472 addressed a socket leakage in go client. with some debug works, a difference between java client and go client was found. one difference is go client send PartitionedTopicMetadata request with new connection, but java is using lookup service's connection. This pr added GetPartitionedTopicMetadata to lookup service to reuse the lookup service connection.

Through this pr, the leakage has been resolved with this sample deployment, the sockets are keep to 7 for 2 hours test. But still need to check with @KannarFr to see if there are any other possible leakages since I cannot have a cluster with 20 proxies/20 brokers/7 ZK locally.

Verifying this change

  • Make sure that the change passes the CI checks.

@freeznet
Copy link
Contributor Author

freeznet commented Mar 2, 2021

rerun failure checks

@wolfstudy wolfstudy requested review from jiazhai, zymap, merlimat and wolfstudy and removed request for jiazhai March 5, 2021 05:23
@wolfstudy wolfstudy added this to the 0.5.0 milestone Mar 5, 2021
@wolfstudy
Copy link
Member

rerun failure checks

Hello @freeznet , currently the rerun failure checks not work, please check the failure test case.

@wolfstudy wolfstudy merged commit f6754a0 into apache:master Mar 5, 2021
@michaeljmarshall
Copy link
Member

@freeznet and @wolfstudy - the original issue references an issue with the proxy. Was this bug limited to connections with the proxy or does it include connections to brokers as well? Thanks.

@freeznet
Copy link
Contributor Author

freeznet commented Mar 9, 2021

@freeznet and @wolfstudy - the original issue references an issue with the proxy. Was this bug limited to connections with the proxy or does it include connections to brokers as well? Thanks.

@michaeljmarshall according to my tests, the issue only appears on proxy, and I havnt noticed the similar socket leakage issue with standalone pulsar.

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

Successfully merging this pull request may close these issues.

TCP socket opened leak using proxy
4 participants