-
Notifications
You must be signed in to change notification settings - Fork 813
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
Correct log message for PG restart on replication config changes #1934
base: master
Are you sure you want to change the base?
Conversation
When some replication-related changes happen (e.g., restore_command is changed), Patroni restarts Postgres and the following can bee seen in logs: INFO: changing primary_conninfo and restarting in progress This message is misleading because it mentions one particular parameter but the change could happen with a different one (e.g., restore_command). This is a quick improvement that makes the messaging better. In the future, it would make sense to report the diff of changes (hence, particular root cause of the restart) that is analyzed in config.py:record_missmatch.
@CyberDem0n we've discussed this in person – what do you think to adjust the log message (maybe making it a bit shorter), while planning to implement config diff reported in logs explicitly? (The main idea I have is that we need to see what exactly has changed – but I guess it might be non-trivial and long, so let's have some simple improvement first). |
A shorter message could be just: The list of params being checked can be found here: patroni/patroni/postgresql/config.py Line 707 in 51cda9f
What I see there:
|
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.
@NikolayS if we go this way, I believe it would be also nice to log what parameters have been changed in the check_recovery_conf() method.
Only parameter names, without values (values could be sensitive!), and only if change results in a restart.
self._async_executor.try_run_async('replication-related parameters (such as primary_conninfo, restore_command) has recently changed, ' | ||
'restarting PostgreSQL to apply the changes', |
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.
self._async_executor.try_run_async('replication-related parameters (such as primary_conninfo, restore_command) has recently changed, ' | |
'restarting PostgreSQL to apply the changes', | |
self._async_executor.try_run_async('changing replication-related parameters and restarting', |
When some replication-related changes happen (e.g., restore_command
is changed), Patroni restarts Postgres and the following can bee seen in logs:
INFO: changing primary_conninfo and restarting in progress
This message is misleading because it mentions one particular parameter
but the change could happen with a different one (e.g., restore_command).
This is a quick improvement that makes the messaging better. In the future,
it would make sense to report the diff of changes (hence, particular root cause
of the restart) that is analyzed in config.py:record_missmatch.