From 6616acff58e51df9780c5d60b6405fd73fcc6a48 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 30 Jun 2021 09:11:12 +0200 Subject: [PATCH] Postpone writing postgresql.conf when joining running Postgres 12+ (#1956) When joining already running Postgres, Patroni ensures that config files are set according to expectations. With recovery parameters converted to GUCs in Postgres v12 it became a little problem, because when the `Postgresql` object is being created it is not yet known where the given replica is supposed to stream from. It resulted in postgresql.conf first being written without recovery parameters, and on the next run of HA loop Patroni noticing inconsistencies and updating the config one more time. For Postgres v12 it is not a big issue, but for v13+ it resulted in interruption of streaming replication. --- patroni/postgresql/__init__.py | 6 ++++-- patroni/postgresql/config.py | 4 +++- tests/test_postgresql.py | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index 2b0a7c2a..13055dbe 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -109,10 +109,12 @@ class Postgresql(object): # Last known running process self._postmaster_proc = None - if self.is_running(): + if self.is_running(): # we are "joining" already running postgres self.set_state('running') self.set_role('master' if self.is_leader() else 'replica') - self.config.write_postgresql_conf() # we are "joining" already running postgres + # postpone writing postgresql.conf for 12+ because recovery parameters are not yet known + if self.major_version < 120000 or self.is_leader(): + self.config.write_postgresql_conf() hba_saved = self.config.replace_pg_hba() ident_saved = self.config.replace_pg_ident() if hba_saved or ident_saved: diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index d0e5fc5b..c61a7695 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -702,7 +702,9 @@ class ConfigHandler(object): if wal_receiver_primary_slot_name is not None: self._current_recovery_params['primary_slot_name'][0] = wal_receiver_primary_slot_name - required = {'restart': 0, 'reload': 0} + # Increment the 'reload' to enforce write of postgresql.conf when joining the running postgres + required = {'restart': 0, + 'reload': int(not self._postgresql.cb_called and self._postgresql.major_version >= 120000)} def record_missmatch(mtype): required['restart' if mtype else 'reload'] += 1 diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 0fbc1b4d..54b5c674 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -224,6 +224,7 @@ class TestPostgresql(BaseTestPostgresql): @patch('patroni.postgresql.config.mtime', mock_mtime) @patch('patroni.postgresql.config.ConfigHandler._get_pg_settings') def test_check_recovery_conf(self, mock_get_pg_settings): + self.p.call_nowait('on_start') mock_get_pg_settings.return_value = { 'primary_conninfo': ['primary_conninfo', 'foo=', None, 'string', 'postmaster', self.p.config._auto_conf], 'recovery_min_apply_delay': ['recovery_min_apply_delay', '0', 'ms', 'integer', 'sighup', 'foo'] @@ -259,6 +260,7 @@ class TestPostgresql(BaseTestPostgresql): @patch.object(MockPostmaster, 'create_time', Mock(return_value=1234567), create=True) @patch('patroni.postgresql.config.ConfigHandler._get_pg_settings') def test__read_recovery_params(self, mock_get_pg_settings): + self.p.call_nowait('on_start') mock_get_pg_settings.return_value = {'primary_conninfo': ['primary_conninfo', '', None, 'string', 'postmaster', self.p.config._postgresql_conf]} self.p.config.write_recovery_conf({'standby_mode': 'on', 'primary_conninfo': {'password': 'foo'}})