From 38aef484e81d68523b3c299acec6175f6a5517c8 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 31 Jan 2025 16:46:07 +0100 Subject: [PATCH] Fix a few little issues with 9.5 support (#3260) 1. pg_rewind error log format wasn't verbose 2. it doesn't support specifying num in synchronous_standby_names --- patroni/postgresql/rewind.py | 5 +++-- patroni/postgresql/sync.py | 2 +- tests/test_rewind.py | 8 ++++++++ tests/test_sync.py | 11 +++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/patroni/postgresql/rewind.py b/patroni/postgresql/rewind.py index ee8e801f..6505e853 100644 --- a/patroni/postgresql/rewind.py +++ b/patroni/postgresql/rewind.py @@ -135,9 +135,10 @@ class Rewind(object): # Message format depends on the major version: # * expected at least -- starting from v16 # * wanted -- before v16 + # * nothing (end of message) 9.5 and older # We will simply check all possible combinations. - for pattern in (': expected at least ', ': wanted '): - j = err[0].find(pattern, i) + for pattern in (': expected at least ', ': wanted ', '\n'): + j = (err[0] + '\n').find(pattern, i) if j > -1: try: return parse_lsn(err[0][i:j]) diff --git a/patroni/postgresql/sync.py b/patroni/postgresql/sync.py index 1dfe5728..ff435807 100644 --- a/patroni/postgresql/sync.py +++ b/patroni/postgresql/sync.py @@ -408,7 +408,7 @@ END;$$""") else: sync_param = next(iter(sync), None) - if global_config.is_quorum_commit_mode and sync or self._postgresql.supports_multiple_sync and len(sync) > 1: + if self._postgresql.supports_multiple_sync and (global_config.is_quorum_commit_mode and sync or len(sync) > 1): prefix = 'ANY ' if global_config.is_quorum_commit_mode and self._postgresql.supports_quorum_commit else '' sync_param = f'{prefix}{num} ({sync_param})' diff --git a/tests/test_rewind.py b/tests/test_rewind.py index 60702528..8b7f44c3 100644 --- a/tests/test_rewind.py +++ b/tests/test_rewind.py @@ -194,6 +194,14 @@ class TestRewind(BaseTestPostgresql): self.r.trigger_check_diverged_lsn() self.assertFalse(self.r.rewind_or_reinitialize_needed_and_possible(self.leader)) + mock_popen.return_value.communicate.return_value = ( + b'0, lsn: 0/040159C1, prev 0/\n', + b'pg_waldump: fatal: error in WAL record at 0/40159C1: invalid record length at 0/402DD98\n' + ) + self.r.reset_state() + self.r.trigger_check_diverged_lsn() + self.assertFalse(self.r.rewind_or_reinitialize_needed_and_possible(self.leader)) + self.r.reset_state() self.r.trigger_check_diverged_lsn() mock_popen.side_effect = Exception diff --git a/tests/test_sync.py b/tests/test_sync.py index 46e8275b..8df4e98f 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -155,6 +155,17 @@ class TestSync(BaseTestPostgresql): mock_reload.assert_called() self.assertEqual(value_in_conf(), "synchronous_standby_names = '3 (a,b)'") + self.p._major_version = 90501 + mock_reload.reset_mock() + self.s.set_synchronous_standby_names([], 1) + mock_reload.assert_called() + self.assertEqual(value_in_conf(), "synchronous_standby_names = '*'") + + mock_reload.reset_mock() + self.s.set_synchronous_standby_names(['a-1'], 1) + mock_reload.assert_called() + self.assertEqual(value_in_conf(), "synchronous_standby_names = '\"a-1\"'") + @patch.object(Postgresql, 'last_operation', Mock(return_value=1)) def test_do_not_prick_yourself(self): self.p.name = self.leadermem.name