Repository: zalando/patroni
Pull request #2863 (2023-09-14 15:10:39)

Don't rely on pg_stat_wal_receiver when deciding on pg_rewind
As was reported by @ants on Slack it could happen that `received_tli` is ahead of replayed timeline, therefore we should stop using it when deciding on pg_rewind if postgres is running and use only `IDENTIFY_SYSTEM` via replication connection.

coveralls: ## Pull Request Test Coverage Report for [Build 6187422569](https://coveralls.io/builds/62661265)

* **1** of **1**   **(100.0%)**  changed or added relevant line in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **99.862%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/62661265/badge)](https://coveralls.io/builds/62661265) |
| :-- | --: |
| Change from base [Build 6186153536](https://coveralls.io/builds/62658042): |  0.0% |
| Covered Lines: | 13068 |
| Relevant Lines: | 13086 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/.github/workflows/install_deps.py b/.github/workflows/install_deps.py
index 29acb701..ea0c9d1b 100644
--- a/.github/workflows/install_deps.py
+++ b/.github/workflows/install_deps.py
@@ -45,8 +45,8 @@ def install_packages(what):
     packages['exhibitor'] = packages['zookeeper']
     packages = packages.get(what, [])
     ver = versions.get(what)
-    if float(ver) >= 15:
-        packages += ['postgresql-{0}-citus-11.2'.format(ver)]
+    if float(ver) == 15:
+        packages += ['postgresql-{0}-citus-12.0'.format(ver)]
     subprocess.call(['sudo', 'apt-get', 'update', '-y'])
     return subprocess.call(['sudo', 'apt-get', 'install', '-y', 'postgresql-' + ver, 'expect-dev'] + packages)
 
diff --git a/.github/workflows/mapping.py b/.github/workflows/mapping.py
index 397a4cc7..f75efec4 100644
--- a/.github/workflows/mapping.py
+++ b/.github/workflows/mapping.py
@@ -1,1 +1,1 @@
-versions = {'etcd': '9.6', 'etcd3': '14', 'consul': '13', 'exhibitor': '12', 'raft': '11', 'kubernetes': '15'}
+versions = {'etcd': '9.6', 'etcd3': '16', 'consul': '13', 'exhibitor': '12', 'raft': '11', 'kubernetes': '15'}
diff --git a/README.rst b/README.rst
index f8f187e4..decc868f 100644
--- a/README.rst
+++ b/README.rst
@@ -12,7 +12,7 @@ Patroni is a template for high availability (HA) PostgreSQL solutions using Pyth
 
 We call Patroni a "template" because it is far from being a one-size-fits-all or plug-and-play replication system. It will have its own caveats. Use wisely.
 
-Currently supported PostgreSQL versions: 9.3 to 15.
+Currently supported PostgreSQL versions: 9.3 to 16.
 
 **Note to Citus users**: Starting from 3.0 Patroni nicely integrates with the `Citus <https://github.com/citusdata/citus>`__ database extension to Postgres. Please check the `Citus support page <https://github.com/zalando/patroni/blob/master/docs/citus.rst>`__ in the Patroni documentation for more info about how to use Patroni high availability together with a Citus distributed cluster.
 
diff --git a/docs/index.rst b/docs/index.rst
index 440f2278..c8f94aaf 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -10,7 +10,7 @@ Patroni is a template for high availability (HA) PostgreSQL solutions using Pyth
 
 We call Patroni a "template" because it is far from being a one-size-fits-all or plug-and-play replication system. It will have its own caveats. Use wisely. There are many ways to run high availability with PostgreSQL; for a list, see the `PostgreSQL Documentation <https://wiki.postgresql.org/wiki/Replication,_Clustering,_and_Connection_Pooling>`__.
 
-Currently supported PostgreSQL versions: 9.3 to 15.
+Currently supported PostgreSQL versions: 9.3 to 16.
 
 **Note to Citus users**: Starting from 3.0 Patroni nicely integrates with the `Citus <https://github.com/citusdata/citus>`__ database extension to Postgres. Please check the :ref:`Citus support page <citus>` in the Patroni documentation for more info about how to use Patroni high availability together with a Citus distributed cluster.
 
diff --git a/patroni/config.py b/patroni/config.py
index 65991b0e..1650934d 100644
--- a/patroni/config.py
+++ b/patroni/config.py
@@ -16,6 +16,7 @@ from .dcs import ClusterConfig, Cluster
 from .exceptions import ConfigParseError
 from .file_perm import pg_perm
 from .postgresql.config import ConfigHandler
+from .validator import IntValidator
 from .utils import deep_compare, parse_bool, parse_int, patch_config
 
 logger = logging.getLogger(__name__)
@@ -487,8 +488,9 @@ class Config(object):
             if name not in ConfigHandler.CMDLINE_OPTIONS:
                 pg_params[name] = value
             elif not is_local:
-                if ConfigHandler.CMDLINE_OPTIONS[name][1](value):
-                    pg_params[name] = value
+                validator = ConfigHandler.CMDLINE_OPTIONS[name][1]
+                if validator(value):
+                    pg_params[name] = int(value) if isinstance(validator, IntValidator) else value
                 else:
                     logger.warning("postgresql parameter %s=%s failed validation, defaulting to %s",
                                    name, value, ConfigHandler.CMDLINE_OPTIONS[name][0])
diff --git a/patroni/postgresql/citus.py b/patroni/postgresql/citus.py
index 09f77f2b..8ca2790e 100644
--- a/patroni/postgresql/citus.py
+++ b/patroni/postgresql/citus.py
@@ -397,7 +397,7 @@ class CitusHandler(Thread):
         parameters['shared_preload_libraries'] = ','.join(['citus'] + shared_preload_libraries)
 
         # if not explicitly set Citus overrides max_prepared_transactions to max_connections*2
-        if parameters.get('max_prepared_transactions') == 0:
+        if parameters['max_prepared_transactions'] == 0:
             parameters['max_prepared_transactions'] = parameters['max_connections'] * 2
 
         # Resharding in Citus implemented using logical replication
diff --git a/patroni/postgresql/rewind.py b/patroni/postgresql/rewind.py
index 73bccb44..6e1aab88 100644
--- a/patroni/postgresql/rewind.py
+++ b/patroni/postgresql/rewind.py
@@ -158,7 +158,7 @@ class Rewind(object):
     def _get_local_timeline_lsn(self) -> Tuple[Optional[bool], Optional[int], Optional[int]]:
         if self._postgresql.is_running():  # if postgres is running - get timeline from replication connection
             in_recovery = True
-            timeline = self._postgresql.received_timeline() or self._postgresql.get_replica_timeline()
+            timeline = self._postgresql.get_replica_timeline()
             lsn = self._postgresql.replayed_location()
         else:  # otherwise analyze pg_controldata output
             in_recovery, timeline, lsn = self._get_local_timeline_lsn_from_controldata()


Test patch:
diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py
index c922fcae..9ac6aa68 100644
--- a/tests/test_bootstrap.py
+++ b/tests/test_bootstrap.py
@@ -238,7 +238,8 @@ class TestBootstrap(BaseTestPostgresql):
         self.p.reload_config({'authentication': {'superuser': {'username': 'p', 'password': 'p'},
                                                  'replication': {'username': 'r', 'password': 'r'},
                                                  'rewind': {'username': 'rw', 'password': 'rw'}},
-                              'listen': '*', 'retry_timeout': 10, 'parameters': {'wal_level': '', 'hba_file': 'foo'}})
+                              'listen': '*', 'retry_timeout': 10,
+                              'parameters': {'wal_level': '', 'hba_file': 'foo', 'max_prepared_transactions': 10}})
         with patch.object(Postgresql, 'major_version', PropertyMock(return_value=110000)), \
                 patch.object(Postgresql, 'restart', Mock()) as mock_restart:
             self.b.post_bootstrap({}, task)
diff --git a/tests/test_config.py b/tests/test_config.py
index cf798d00..bd8d0a90 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -3,6 +3,7 @@ import sys
 import unittest
 import io
 
+from copy import deepcopy
 from mock import MagicMock, Mock, patch
 from patroni.config import Config, ConfigParseError
 
@@ -22,7 +23,7 @@ class TestConfig(unittest.TestCase):
             self.assertFalse(self.config.set_dynamic_configuration({'foo': 'bar'}))
         self.assertTrue(self.config.set_dynamic_configuration({'standby_cluster': {}, 'postgresql': {
             'parameters': {'cluster_name': 1, 'hot_standby': 1, 'wal_keep_size': 1,
-                           'track_commit_timestamp': 1, 'wal_level': 1}}}))
+                           'track_commit_timestamp': 1, 'wal_level': 1, 'max_connections': '100'}}}))
 
     def test_reload_local_configuration(self):
         os.environ.update({
@@ -149,3 +150,25 @@ class TestConfig(unittest.TestCase):
     @patch('os.path.isdir', Mock(return_value=False))
     def test_invalid_path(self):
         self.assertRaises(ConfigParseError, Config, 'postgres0')
+
+    def test__process_postgresql_parameters(self):
+        expected_params = {
+            'f.oo': 'bar',  # not in ConfigHandler.CMDLINE_OPTIONS
+            'max_connections': 100,  # IntValidator
+            'wal_level': 'hot_standby',  # EnumValidator
+        }
+        input_params = deepcopy(expected_params)
+
+        input_params['max_connections'] = '100'
+        self.assertEqual(self.config._process_postgresql_parameters(input_params), expected_params)
+
+        expected_params['f.oo'] = input_params['f.oo'] = '100'
+        self.assertEqual(self.config._process_postgresql_parameters(input_params), expected_params)
+
+        input_params['wal_level'] = 'cold_standby'
+        expected_params.pop('wal_level')
+        self.assertEqual(self.config._process_postgresql_parameters(input_params), expected_params)
+
+        input_params['max_connections'] = 10
+        expected_params.pop('max_connections')
+        self.assertEqual(self.config._process_postgresql_parameters(input_params), expected_params)
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9ab3f52d..e2fad6ba 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -688,7 +688,7 @@ class TestPostgresql(BaseTestPostgresql):
             self.assertIsNone(self.p.wait_for_startup())
 
     def test_get_server_parameters(self):
-        config = {'parameters': {'wal_level': 'hot_standby'}, 'listen': '0'}
+        config = {'parameters': {'wal_level': 'hot_standby', 'max_prepared_transactions': 100}, 'listen': '0'}
         self.p._global_config = GlobalConfig({'synchronous_mode': True})
         self.p.config.get_server_parameters(config)
         self.p._global_config = GlobalConfig({'synchronous_mode': True, 'synchronous_mode_strict': True})