fix: don't stop client connection when parameter status is updated#666
fix: don't stop client connection when parameter status is updated#666
Conversation
Just continue with the greeting process, with the new parameter status.
There was a problem hiding this comment.
With the changes, if the database version updates, only the server_version parameter is forwarded to clients in the status message. However, there are other important fields such as client_encoding, server_encoding, TimeZone, and others.
There is no need to send new_ps because all necessary information is already available in encoded_ps, which is already in iodata format.
Also, there is already a directory for e2e - test/integration
|
Thank you for the review, @abc3. Yeah, it felt wrong. I'll dive a bit deeper into it once I'm done with other stuff. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| changed_parameters -> | ||
| Logger.warning("Changed parameters: #{inspect(changed_parameters)}") | ||
|
|
||
| # TODO: should we update all? Previously we only updated server version |
Check warning
Code scanning / Credo
Found a TODO tag in a comment: # TODO: should we update all? Previously we only updated server version Warning
|
Hi, @abc3, I'll go ahead and merge this, but if you have any further feedback, let me know. Thank you again! I plan to extract the test script into a proper integration test, I'm working on this concurrently, but want to ship the code fix. |
|
hey @v0idpwn, looks good to me! |
3 participants
Instead of stopping the client connection, just continue with the greeting process, with the new parameter status.
Also adds a test script that performs a pseudo postgres update. In the original code, the query must be retried on Supavisor because the first connection attempt fails. After the patch, no retries are needed.
Discovered in #663.