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

Adds option to log which parameter status is changed by the client #550

Merged

Conversation

zainkabani
Copy link
Contributor

This PR helps to debug what kind of parameters are being set by the client outside of the startup. This will only log for SET statements that respond with a ParameterStatus packet

set application_name to 'name';

will return a parameter status but

set statement_timeout to 10;

will not return a parameter status and will not be logged

@zainkabani zainkabani marked this pull request as ready for review August 16, 2023 17:01
@levkk
Copy link
Contributor

levkk commented Aug 16, 2023

I feel like you want to almost log all SET statements outside of startup because they are all pretty much considered application bugs by the Postgres community, outside of special circumstances. e.g. changing statement_timeout on a production database is a problem, because one client basically allows itself to take up a lot of resources, probably against the wishes of the DBA.

What do you think?

@zainkabani
Copy link
Contributor Author

zainkabani commented Aug 16, 2023

I'd love to have that visibility too, the issue is that the SET statements come in as queries so we'd have to parse those. For every set statement a SET command tag is returned with no other data. For a subset of SETs (ie. application_name) we also get back a parameter status packet, which gives you the key and value of the parameter. So unless we parse every query this is the only way we can get what the client is setting.

Copy link
Contributor

@levkk levkk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@levkk levkk merged commit 3255323 into postgresml:main Aug 16, 2023
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