Repository: zalando/patroni
Pull request #2383 (2022-08-15 08:50:56)

Don't drop replication slots in pause
If replication slots are enabled Patroni automatically creates them for any cluster member that is supposed to stream from a given node and for any permanent slot defined in the global configuration. If the member disappears from the DCS Patroni automatically removes the replication slot for it. The same behavior was in the maintenance mode (pause).

This commit disables removal of any replication slots that don't match Patroni's expectations in pause.

Close https://github.com/zalando/patroni/issues/2314

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

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

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/51656624/badge)](https://coveralls.io/builds/51656624) |
| :-- | --: |
| Change from base [Build 2797700302](https://coveralls.io/builds/51422778): |  0.0% |
| Covered Lines: | 11009 |
| Relevant Lines: | 11022 |

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

hughcapet: 👍 
CyberDem0n: :+1:

Patch:
diff --git a/docs/pause.rst b/docs/pause.rst
index a33b04b4..285ebac1 100644
--- a/docs/pause.rst
+++ b/docs/pause.rst
@@ -27,6 +27,8 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL,
 
 - When Postgres is stopped, Patroni does not try to start it. When Patroni is stopped, it does not try to stop the Postgres instance it is managing.
 
+- Patroni will not try to remove replication slots that don't represent the other cluster member or are not listed in the configuration of the permanent slots.
+
 User guide
 ----------
 
diff --git a/patroni/ha.py b/patroni/ha.py
index 9f84613f..54f2c0ab 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -1483,7 +1483,9 @@ class Ha(object):
                 # asynchronous processes are running (should be always the case for the master)
                 if not self._async_executor.busy and not self.state_handler.is_starting():
                     create_slots = self.state_handler.slots_handler.sync_replication_slots(self.cluster,
-                                                                                           self.patroni.nofailover)
+                                                                                           self.patroni.nofailover,
+                                                                                           self.patroni.replicatefrom,
+                                                                                           self.is_paused())
                     if not self.state_handler.cb_called:
                         if not self.state_handler.is_leader():
                             self._rewind.trigger_check_diverged_lsn()
diff --git a/patroni/postgresql/slots.py b/patroni/postgresql/slots.py
index 32cbc9b8..0d5051f7 100644
--- a/patroni/postgresql/slots.py
+++ b/patroni/postgresql/slots.py
@@ -112,10 +112,10 @@ class SlotsHandler(object):
         # In normal situation rowcount should be 1, otherwise either slot doesn't exists or it is still active
         return cursor.rowcount == 1
 
-    def _drop_incorrect_slots(self, cluster, slots):
+    def _drop_incorrect_slots(self, cluster, slots, paused):
         # drop old replication slots which are not presented in desired slots
         for name in set(self._replication_slots) - set(slots):
-            if not self.ignore_replication_slot(cluster, name) and not self.drop_replication_slot(name):
+            if not paused and not self.ignore_replication_slot(cluster, name) and not self.drop_replication_slot(name):
                 logger.error("Failed to drop replication slot '%s'", name)
                 self._schedule_load_slots = True
 
@@ -206,7 +206,7 @@ class SlotsHandler(object):
                         self._schedule_load_slots = True
         return create_slots
 
-    def sync_replication_slots(self, cluster, nofailover, replicatefrom=None):
+    def sync_replication_slots(self, cluster, nofailover, replicatefrom=None, paused=False):
         ret = None
         if self._postgresql.major_version >= 90400 and cluster.config:
             try:
@@ -215,7 +215,7 @@ class SlotsHandler(object):
                 slots = cluster.get_replication_slots(self._postgresql.name, self._postgresql.role,
                                                       nofailover, self._postgresql.major_version, True)
 
-                self._drop_incorrect_slots(cluster, slots)
+                self._drop_incorrect_slots(cluster, slots, paused)
 
                 self._ensure_physical_slots(slots)
 


Test patch:
diff --git a/tests/test_slots.py b/tests/test_slots.py
index 454fab8d..2301ae5f 100644
--- a/tests/test_slots.py
+++ b/tests/test_slots.py
@@ -42,8 +42,10 @@ class TestSlotsHandler(BaseTestPostgresql):
         self.p.set_role('standby_leader')
         self.s.sync_replication_slots(cluster, False)
         self.p.set_role('replica')
-        with patch.object(Postgresql, 'is_leader', Mock(return_value=False)):
-            self.s.sync_replication_slots(cluster, False)
+        with patch.object(Postgresql, 'is_leader', Mock(return_value=False)),\
+                patch.object(SlotsHandler, 'drop_replication_slot') as mock_drop:
+            self.s.sync_replication_slots(cluster, False, paused=True)
+            mock_drop.assert_not_called()
         self.p.set_role('master')
         with mock.patch('patroni.postgresql.Postgresql.role', new_callable=PropertyMock(return_value='replica')):
             self.s.sync_replication_slots(cluster, False)