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 receive issue of the client connection #245

Merged
merged 4 commits into from
May 20, 2023
Merged

Conversation

mostafa
Copy link
Member

@mostafa mostafa commented May 20, 2023

This PR partially fulfills the requirements and issues raised in #32, #209 and #219. I tried three solutions, and one partially worked:

  • The first solution is to try to read from the connection again inside the forever loop. The initial message, aka. startup message, didn't get through, as there is no metric to know when to run the second round of read. But if I check the size of the message, which in my tests was 105 bytes from an ErrorResponse message, and then run the second round of read, it'll process everything well. The issue with that is quite obvious, as the message size is definitely not a reliable metric.
  • The second solution is to try to set a read deadline on the connection to a very small value, like 100ms, and then try the second call to read. This works, but only on small messages. To process large messages, I needed to increase the read deadline, which caused significant delays in message receive time. The problem with this is that I don't know the size of the message in advance to set and reset the deadline, which practically makes this a 🐔 and 🥚 problem.
  • The last solution is to use channels, goroutines and select to wait on reads asynchronously, which didn't work as expected. That is, it doesn't necessarily solve the problem, it just complicates it.

I was finally able to solve this using select with 1ms of wait.

@mostafa mostafa merged commit b8491db into main May 20, 2023
@mostafa mostafa deleted the fix-client-connection-issue branch May 20, 2023 21:48
@mostafa mostafa mentioned this pull request Sep 29, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle SQL cancel command and errors properly Handle bi-directional communication between server and client
1 participant