Enable archive_mode=always in Postgres 9.5 and 9.6 #254
Conversation
| @@ -206,7 +206,7 @@ checkpoint_warning = {{postgresql_checkpoint_warning}} # 0 disables | |||
|
|
|||
| # - Archiving - | |||
|
|
|||
| archive_mode = {{'on' if postgresql_archive_mode else 'off'}} # enables archiving; off, on, or always | |||
| archive_mode = {{('always' if postgresql_archive_mode == 'always' else 'on') if postgresql_archive_mode else 'off'}} # enables archiving; off, on, or always | |||
There was a problem hiding this comment.
I prefer this simple syntax:
archive_mode = {{ postgresql_archive_mode }}
And document the different option values on/off/always in https://github.com/ANXS/postgresql/blob/master/defaults/main.yml#L261
There was a problem hiding this comment.
Yeah, that makes sense. There's no validation, but then we need to assume the users are sensible and in control. If it's not valid, then PostgreSQL will fail to startup and they will know quite quickly:
LOG: invalid value for parameter "archive_mode": "typo_in_param"
HINT: Available values: always, on, off.
< 2018-03-26 10:47:35.719 BST > FATAL: configuration file "/var/lib/pgsql/9.6/data/postgresql.conf" contains errors
There was a problem hiding this comment.
Oh... the only problem with the simple syntax, is that JINJA will always resolve "on/off" to "true/false", right?
We'll need to make sure it's handled just like synchronous_commit, as a string, not a boolean:
# Synchronization level:
# - off
# - local
# - remote_write
# - remote_apply (>= 9.6)
# - on
postgresql_synchronous_commit: "on"
There was a problem hiding this comment.
Ah... I remember a glitch about something like that yes. Well, we can accept this PR as-is, if someday PostgreSQL introduces a 4th value for this option, we will find another way to handle this :)
There was a problem hiding this comment.
Actually, I think your way is cleaner @sebalix. The true/on, false/off syntax is used exclusively for parameters that have two values:
[ansible@localhost templates]$ grep ' if postgresql_' postgresql.conf-9.6.j2 | cut -f2 -d =
{{'on' if postgresql_bonjour else 'off'}} # advertise server via Bonjour
{{'on' if postgresql_ssl else 'off'}} # (change requires restart)
{{ 'on' if postgresql_ssl_prefer_server_ciphers else 'off' }} # (change requires restart)
{{'on' if postgresql_password_encryption else 'off'}}
{{'on' if postgresql_db_user_namespace else 'off'}}
{{'on' if postgresql_row_security else 'off'}}
{{'on' if postgresql_db_user_namespace else 'off'}}
{{'on' if postgresql_fsync else 'off'}} # flush data to disk for crash safety
{{'on' if postgresql_full_page_writes else 'off'}} # recover from partial page writes
{{ 'on' if postgresql_wal_compression else 'off' }}
{{ 'on' if postgresql_wal_log_hints else 'off' }} # also do full page writes of non-critical updates
{{ postgresql_max_wal_size if postgresql_max_wal_size or '1G' }}
{{ postgresql_min_wal_size if postgresql_min_wal_size or '80MB' }}
{{'on' if postgresql_archive_mode else 'off'}} # enables archiving; off, on, or always
{{'on' if postgresql_track_commit_timestamp else 'off' }}
'{{postgresql_synchronous_standby_num_sync}}{% if postgresql_synchronous_standby_names !
{{'on' if postgresql_hot_standby else 'off'}} # "on" allows queries during recovery
{{'on' if postgresql_hot_standby_feedback or 'off'}} # send info from standby to prevent
{{'on' if postgresql_enable_bitmapscan else 'off'}}
{{'on' if postgresql_enable_hashagg else 'off'}}
{{'on' if postgresql_enable_hashjoin else 'off'}}
{{'on' if postgresql_enable_indexscan else 'off'}}
{{'on' if postgresql_enable_indexonlyscan else 'off'}}
{{'on' if postgresql_enable_material else 'off'}}
{{'on' if postgresql_enable_mergejoin else 'off'}}
{{'on' if postgresql_enable_nestloop else 'off'}}
{{'on' if postgresql_enable_seqscan else 'off'}}
{{'on' if postgresql_enable_sort else 'off'}}
{{'on' if postgresql_enable_tidscan else 'off'}}
{{'on' if postgresql_enable_tidscan else 'off'}}
{{'on' if postgresql_force_parallel_mode else 'off'}}
{{'on' if postgresql_logging_collector else 'off'}} # Enable capturing of stderr and csvlog
{{'on' if postgresql_log_truncate_on_rotation else 'off'}} # If on, an existing log file with the
{{'on' if postgresql_syslog_sequence_numbers else 'off'}}
{{'on' if postgresql_syslog_split_messages else 'off'}}
{{'on' if postgresql_debug_print_parse else 'off'}}
{{'on' if postgresql_debug_print_rewritten else 'off'}}
{{'on' if postgresql_debug_print_plan else 'off'}}
{{'on' if postgresql_debug_pretty_print else 'off'}}
{{'on' if postgresql_log_checkpoints else 'off'}}
{{'on' if postgresql_log_connections else 'off'}}
{{'on' if postgresql_log_disconnections else 'off'}}
{{'on' if postgresql_log_duration else 'off'}}
{{'on' if postgresql_log_duration else 'off'}}
{{'on' if postgresql_log_lock_waits else 'off'}} # log lock waits >
{{'on' if postgresql_track_activities else 'off'}}
{{'on' if postgresql_track_counts else 'off'}}
{{'on' if postgresql_track_io_timing else 'off'}}
{{'on' if postgresql_update_process_title else 'off'}}
{{'on' if postgresql_log_parser_stats else 'off'}}
{{'on' if postgresql_log_planner_stats else 'off'}}
{{'on' if postgresql_log_executor_stats else 'off'}}
{{'on' if postgresql_log_statement_stats else 'off'}}
{{'on' if postgresql_autovacuum else 'off'}} # Enable autovacuum subprocess? 'on'
{{'on' if postgresql_check_function_bodies else 'off'}}
{{'on' if postgresql_default_transaction_read_only else 'off'}}
{{'on' if postgresql_default_transaction_deferrable else 'off'}}
{{'on' if postgresql_array_nulls else 'off'}}
{{'on' if postgresql_default_with_oids else 'off'}}
{{'on' if postgresql_escape_string_warning else 'off'}}
{{'on' if postgresql_lo_compat_privileges else 'off'}}
{{'on' if postgresql_quote_all_identifiers else 'off'}}
{{'on' if postgresql_sql_inheritance else 'off'}}
{{'on' if postgresql_standard_conforming_strings else 'off'}}
{{'on' if postgresql_synchronize_seqscans else 'off'}}
{{'on' if postgresql_transform_null_equals else 'off'}}
{{'on' if postgresql_exit_on_error else 'off'}} # terminate session on any error?
{{'on' if postgresql_restart_after_crash else 'off'}} # reinitialize after backend crash?
I think it's sensible to keep this syntax for boolean postgresql.conf parameters, and to use the other syntax you proposed for items with multiple options (enum). In v9.6, the archive_mode changed from bool -> enum... so it's sensible for us to also change.
We just need to be a little more careful about how we list the defaults, to ensure they are most obviously strings. For parameters such as wal_level, there are no quotes... but that's only OK, because on/off/true/false/yes/no aren't options. We don't get that luxury with archive_mode.
postgresql_wal_level: minimal # minimal, archive (<= 9.5), hot_standby (<= 9.5), replica (>= 9.6), or logical
There was a problem hiding this comment.
I've raised #318, because I think we need to tidy up things a little to prevent errors.
|
@pjungwir , are you available to refresh this PR? |
|
It looks like you've removed that branch from your repo... as I can only find these: |
|
I made my own branch of this to get it done. #403 It's slightly different to your initial suggestion, but it works across all versions and defaults to the same behaviour as before. I hope you don't mind me closing this PR. |
|
Hi, thanks for adding support for this! I don't mind your making a separate implementation at all. My wife had a baby last week so I was a little busy. :-) (Also it looks like I must have replaced the ANXS postgres ansible repo with the postgres-itself repo. . . .) |
|
Congratulations @pjungwir ! I know just how busy it can get with a new baby in the house. Thanks again for your help, and please, if you have any more PR's then send them in. |
If the user sets
postgresql_archive_mode: alwaysthen we should honor that in Postgres 9.5 and 9.6 (where it is supported), rather than just forcing it toon. This is important if you want the standby to also archive WAL, e.g. in a cascading standby setup or when running WAL-E from the standby.