Repository: zalando/patroni
Pull request #2787 (2023-07-28 13:27:41)

Don't allow stale primary to win the leader race
Consider a following situation:
1. node1 is stressed so much that Patroni heart-beat can't run regularly and the leader lock expires.
2. node2 notice that there is no leader, gets the lock, promotes, and gets to a situation like it is described in 1.
3. Patroni on node1 finally wakes up, notice that Postgres is running as a primary, but without a leader lock and "happily" acquires the lock.

That is, node1 discarded promoting of node2, and the node2 after that it will not be possible to join the node2 back to the cluster, because pg_rewind is not possible when two nodes are on the same timeline.

To partially mitigate the problem we introduce an additional timeline check. If postgres is running as primary Patroni will consider it as a perfect candidate only if timeline isn't behind the last known cluster timeline recorder in the `/history` key. If postgres timeline is behind the cluster timeline postgres will be demoted to read-only. Further behavior would depend on `maximum_lag_on_failover` and `check_timeline` settings.

Since the `/history` key isn't updated instantly after promotion, there is still a short period of time when the issue could happen, but it seems that it is close to impossible to make it more reliable.

Close https://github.com/zalando/patroni/issues/2779

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

* **7** of **7**   **(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.859%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/61641892/badge)](https://coveralls.io/builds/61641892) |
| :-- | --: |
| Change from base [Build 5680056308](https://coveralls.io/builds/61614054): |  0.0% |
| Covered Lines: | 12747 |
| Relevant Lines: | 12765 |

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

hughcapet: 👍 
CyberDem0n: :+1:

Patch:
diff --git a/patroni/ha.py b/patroni/ha.py
index 95ca6e05..2445aa02 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -996,9 +996,22 @@ class Ha(object):
                 return ret
 
         if self.state_handler.is_leader():
-            # in pause leader is the healthiest only when no initialize or sysid matches with initialize!
-            return not self.is_paused() or not self.cluster.initialize\
-                or self.state_handler.sysid == self.cluster.initialize
+            if self.is_paused():
+                # in pause leader is the healthiest only when no initialize or sysid matches with initialize!
+                return not self.cluster.initialize or self.state_handler.sysid == self.cluster.initialize
+
+            # We want to protect from the following scenario:
+            # 1. node1 is stressed so much that heart-beat isn't running regularly and the leader lock expires.
+            # 2. node2 promotes, gets heavy load and the situation described in 1 repeats.
+            # 3. Patroni on node1 comes back, notices that Postgres is running as primary but there is
+            #    no leader key and "happily" acquires the leader lock.
+            # That is, node1 discarded promotion of node2. To avoid it we want to detect timeline change.
+            my_timeline = self.state_handler.get_primary_timeline()
+            if my_timeline < self.cluster.timeline:
+                logger.warning('My timeline %s is behind last known cluster timeline %s',
+                               my_timeline, self.cluster.timeline)
+                return False
+            return True
 
         if self.is_paused():
             return False


Test patch:
diff --git a/tests/test_ha.py b/tests/test_ha.py
index 1f5f903d..ae6be3e2 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -375,6 +375,12 @@ class TestHa(PostgresInit):
     def test_acquire_lock_as_primary(self):
         self.assertEqual(self.ha.run_cycle(), 'acquired session lock as a leader')
 
+    def test_leader_race_stale_primary(self):
+        with patch.object(Postgresql, 'get_primary_timeline', Mock(return_value=1)), \
+                patch('patroni.ha.logger.warning') as mock_logger:
+            self.assertEqual(self.ha.run_cycle(), 'demoting self because i am not the healthiest node')
+            self.assertEqual(mock_logger.call_args[0][0], 'My timeline %s is behind last known cluster timeline %s')
+
     def test_promoted_by_acquiring_lock(self):
         self.ha.is_healthiest_node = true
         self.p.is_leader = false