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

An issue with Established TCP connections after disposing of the client #40

Closed
OlyaKhruleva opened this issue Sep 13, 2019 · 14 comments
Closed
Assignees
Labels

Comments

@OlyaKhruleva
Copy link

OlyaKhruleva commented Sep 13, 2019

Hello,
I am using the csharp client 3.4.12.1 and zookeeper server 3.4.13 (tried to update to 3.5.5 also). I have the Restful API and creating clients per request, when request ended I disposing of the clients using the closeAsync() method synchronously. When it was disposed I see in code that connection state is closed, but in zookeeper, I see those connections were not closed and every time it increasing (see screenshot 1, I used the telnet with stat command) and count of established TCP ports increasing too (maximum was see screenshots 2,3 - perf monitor, 582 ports).
zoo connections
tcp perfmon
java resource monitor
My maxClientCnxns=500 in zoo.cfg, when I ran the stress test the count of connections was more than 500 and I got the LossException in my code, then count of connections was decreased to 319. In this case, when I shut down the Restful API the ports are release sometimes.
I tried to understand the problem. I tried to rewrite the code in ClientCnxn.close() and disconnect methods: instead of the calling the clientCnxnSocket.wakeupCnxn(); I used the following code:
private async Task disconnect() {
if (LOG.isDebugEnabled()) {
LOG.debug("Disconnecting client for session: 0x"
+ getSessionId().ToHexString());
}
await close();
queueEventOfDeath();
}
private async Task close()
{
state.Value = (int)ZooKeeper.States.CLOSED;
LOG.info("clientCnxnSocket cleanup() in close()");
await clientCnxnSocket.cleanup().ConfigureAwait(false);
LOG.info("clientCnxnSocket close()");
clientCnxnSocket.close();
}
but it was not resolved the issue.
Please advise

@shayhatsor shayhatsor self-assigned this Sep 29, 2019
@shayhatsor shayhatsor added the bug label Sep 29, 2019
@shayhatsor shayhatsor reopened this Sep 29, 2019
@shayhatsor
Copy link
Owner

@OlyaKhruleva, can you please verify on your end that the bug is fixed ?

@OlyaKhruleva
Copy link
Author

Hello, yes I can, but I need some time. I will build the new version and check. I'll let you know, please hold on

@shayhatsor
Copy link
Owner

great! thank you!

@OlyaKhruleva
Copy link
Author

Hello, I checked this with the new version 3.4.12.4, but the problem was not fixed. It still increases TCP connections and not close.
3 4 12 4 tcp

@shayhatsor
Copy link
Owner

I tried to make sense of your results and came up the following:

  • Connections Established is the number of TCP connections for which the current state is either ESTABLISHED or CLOSE-WAIT.
  • CLOSE_WAIT Indicates that the server has received the first FIN signal from the client and the connection is in the process of being closed.

So, the question shouldn't be whether Connections Established counter is going up, it should be whether it eventually goes down. Your test should include some time the code doesn't try to create new ZK clients, and see if the sockets get closed.

@OlyaKhruleva
Copy link
Author

Yes, of course, I was waiting. It is only one screenshot from several tests and monitoring. In the first comment, I have provided systematized information.

@OlyaKhruleva
Copy link
Author

The solution of the problem was: using the singleton instance of Zookeeper client in my API. It is working for me, but with Scoped lifetime (per request) it is not working. Of course, all established connections are closing after API shut down.

@OlyaKhruleva
Copy link
Author

OlyaKhruleva commented Oct 7, 2019

The general case is:

  • create API .Net Framework 4.6.1
  • use the Zookeeper client in the using block, for example:
    using (var ZookeeperClient = new APIZookeeperClient())
    {}

public async void Dispose()
{
await WriteRetryProtocol?.Client?.closeAsync();
await ReadRetryProtocol?.Client?.closeAsync();
}

  • in the using block do something (in my code i check node exists and create if needed)
  • create .bat file for emulate parallel queries (for example using curl)
  • run some monitoring programs in background (etc. perfmon, telnet with stat)
  • run several .bat files

@shayhatsor
Copy link
Owner

using (var ZookeeperClient = new APIZookeeperClient())

.net currently doesn't have syntactic sugar for a using statement that supports async code, that's why i added ZooKeeper.Using(..). your code does not actually await the closing of the zookeeper client, async void can't be awaited, it is fire and forget.

@OlyaKhruleva
Copy link
Author

OlyaKhruleva commented Oct 8, 2019

In my code I also tried to close clients with code bellow in sync controller action:
public void Dispose()
{
WriteRetryProtocol?.Client?.closeAsync().Wait();
ReadRetryProtocol?.Client?.closeAsync().Wait();
}
i got the connection state closed in the debug, but TCP connections were stay alive every time.

@OlyaKhruleva
Copy link
Author

OlyaKhruleva commented Oct 8, 2019

Do you mean that closeAsync() method can not be used in sync code at all?

@shayhatsor
Copy link
Owner

Do you mean that closeAsync() method can not be used in sync code at all?

No, as long as you Wait() or await.
In order for me to be able to fully understand your scenario, please provide a concise but complete code that reproduces the problem.

@OlyaKhruleva
Copy link
Author

Sorry for the delay, OK, when I will have enough time I will create an isolated project with my realization of work with the client and illustrate the bug. I'll provide it for you.

@shayhatsor
Copy link
Owner

Thank you!

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

No branches or pull requests

2 participants