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

Ensure connection is usable after COPY stream is complete #1016

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

valeneiko
Copy link
Contributor

@valeneiko valeneiko commented Jan 21, 2025

Summary

Encountered this issue in a live environment.
Sometimes random queries were hanging indefinitely after using a COPY stream.

This PR likely fixes:

Cause

This was caused by receiving CopyData and CopyDone messages within the same chunk.
If CopyData triggers high water mark, handler pauses the socket, but still continues to process remaining messages, that were already received.
If CopyDone is handled before stream managed to process buffered messages, the connection becomes unusable - the socket is paused. Any query using that connection will hang indefinitely (with response data in the socket read buffer).

Fix

Make sure socket is flowing after COPY stream is done.
The test is a little complicated, but I couldn't find a simpler way to reliably reproduce the issue (now without the fix the test will timeout every time).

@porsager
Copy link
Owner

Nice work! Looks good!

You don't need the changes in the generated folders (cf, cjs, deno) so could you discard those changes?

Also perhaps rebase on your latest PR that fixed CI

@valeneiko

This comment was marked as resolved.

@valeneiko valeneiko marked this pull request as draft January 21, 2025 22:30
@valeneiko valeneiko marked this pull request as ready for review January 23, 2025 21:46
@valeneiko
Copy link
Contributor Author

@porsager PR is updated and CI is green.

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.

2 participants