Fix socket connection reset

[HolyPrapor]

No description provided.

[shayhatsor]

Thank you for your interest and contribution. I have read you change, it seems that it should work but there's something that bothers me. The code must be non-blocking and there's a 1000ms wait for a response from the socket. I wonder if that's a scenario that will actually happen, given the previous conditions.

[HolyPrapor]

Actually that's not milliseconds, that's microseconds.
In other words, overall blocking time is 1ms.

I didn't notice any performance impact in my tests, and the problem disappeared.

You can do your own tests though.

[MatsKarlsson]

Been having the same problem, org.apache.zookeeper.KeeperException.ConnectionLossException started to appear more frequently in .netcore3, but when trying to upgrade to .NET5 I get it all the time.

Using MacOS and Big Sur.

[HolyPrapor]

It appears that provided fix works on .NET Core but doesn't work on .NET5 for some reason.

I decided to remove the connection reset check completely for now, because the client sends keep-alive pings and if there is real connection loss, we will fall in a short time.

[shayhatsor]

well, after a long time of not maintaining this project, i went to nuget to see if it's popular. I was surprise to see that it has over a million downloads, i think it's time to get back into it. I'm currently working full time, so it'll take me some time to refresh my memory. hopefully I'd be able to provide a new version that fixes all new issues (maybe also finally upgrade to the latest client spec). i hope i can count on you guys to test, thanks again for your interest

[kuskmen]

Hi, I think we might be experiencing the same problems with .netcore3.1 and linux containers, is there any progress on this?

[CrazyJinn]

Use socket.Poll before judge socket.Available equals 0
if (socketAsyncEventArgs.LastOperation == SocketAsyncOperation.Receive && _socket.Poll(100000, SelectMode.SelectRead) && _socket.Available == 0)
After doing that, the problem solved
However, I still don't know why the issue happen after upgrade .net 5.0 or 6.0

[HolyPrapor]

I am now closing this PR because provided solution with socket.Poll may have performance issues since this function call is synchronous.

For my use case it is completely fine to remove connection lost check completely. However, it might not be the case for all consumers.

You can use either of the solutions described in this thread as a workaround, but I suggest a more thorough analysis of the problem.

[kuskmen]

@HolyPrapor , what would be the workaround, I must have missed them.

[HolyPrapor]

@HolyPrapor , what would be the workaround, I must have missed them.

The first workaround is to use socket.Poll, as described earlier in this thread.
The second workaround is to remove the connection lost check completely.

Both of those require editing the source code and recompiling, unfortunately.

[kuskmen]

Which javac.target and javac.source versions you are using because when I try to compile the code it says

[javac] error: Source option 6 is no longer supported. Use 7 or later.                                                                                                                                                                [javac] error: Target option 6 is no longer supported. Use 7 or later.    

When I change value to >1.6 compilation starts throwing errors

...
javac] C:\source\github\global\zookeeper\src\java\generated\org\apache\zookeeper\proto\CreateRequest.java:93: error: incompatible types: ACL cannot be converted to Record
    [javac]     a_.readRecord(e1,"e1");
    [javac]                   ^
    [javac] C:\source\github\global\zookeeper\src\java\generated\org\apache\zookeeper\proto\CreateRequest.java:108: error: incompatible types: CreateRequest cannot be converted to Record
    [javac]       a_.startRecord(this,"");
    [javac]                      ^
    [javac] Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
    [javac] 100 errors
    [javac] 4 warnings
    [javac] only showing the first 100 errors, of 325 total; use -Xmaxerrs if you would like to see more

[HolyPrapor]

@kuskmen, sorry, I am unable to help you in this regard because the last time I compiled this was almost two years ago, so I'm not sure what to say here, but this is definitely doable.

I just followed this readme, nothing fancy.

[kuskmen]

Yes, I followed the readme and it got me there, anyway, I will try to figure it out myself.