- Notifications
You must be signed in to change notification settings - Fork514
improve is_closed effectiveness#1229
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
improve is_closed effectiveness#1229
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| self.connection.block_on(self.client.batch_execute(query)) | ||
| } | ||
| /// Check the connection is alive and wait for the confirmation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| /// Check the connection is alive and wait for the confirmation. | |
| /// Checkthatthe connection is alive and wait for the confirmation. |
| simple_query::batch_execute(self.inner(), query).await | ||
| } | ||
| /// Check the connection is alive and wait for the confirmation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| /// Check the connection is alive and wait for the confirmation. | |
| /// Checkthatthe connection is alive and wait for the confirmation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for the PR!
Hi. We noticed that
is_closed()is not always super effective, especially in thepostgrescrate. I've added a couple utilities to try and improve this state.Problem 1:
is_closed()checks if the channel is closed, not if the connection is closed. These can be the same, but currently the channel is only closed whenConnectionis dropped. Inpostgres::Client,Connectionnever gets dropped. Fix is to force close the receiver on terminal state.Problem 2: The connection will only turn if something happens. Sometimes closure can be silent, so even still it seems open. You only can check it's valid if you try to run a query or if the TCP keepalive triggers. Running a
SELECT 1;is a common workaround, but in my understanding, a singleSYNCmessage is cheaper and doesn't cause any logging - so it's a better liveness check than running a full query.Thanks for your consideration