Change the behavior in pause (#1687)

1. Don't call bootstrap if PGDATA is missing/empty, because it might be for purpose, and someone/something working on it.
2. Consider postgres running as a leader in pause not healthy if pg_control sysid doesn't match with the /initialize key (empty initialize key will allow the "race" and the leader will "restore" initialize key).
3. Don't exit on sysid mismatch in pause, only log a warning.
4. Cover corner cases when Patroni started in pause with empty PGDATA and it was restored by somebody else
5. Empty string is a valid `recovery_target`.

features/patroni_api.feature

@@ -61,9 +61,15 @@ Scenario: check the scheduled restart
 		And postgres0 role is the primary after 10 seconds
 
 Scenario: check API requests for the primary-replica pair in the pause mode
-	Given I run patronictl.py pause batman
+	Given I start postgres1
+	Then replication works from postgres0 to postgres1 after 20 seconds
+	When I run patronictl.py pause batman
+	Then I receive a response returncode 0
+	When I kill postmaster on postgres1
+	And I issue a GET request to http://127.0.0.1:8009/replica
+	Then I receive a response code 503
+	When I run patronictl.py restart batman postgres1 --force
 	Then I receive a response returncode 0
-	When I start postgres1
 	Then replication works from postgres0 to postgres1 after 20 seconds
 	When I issue a GET request to http://127.0.0.1:8009/replica
 	Then I receive a response code 200

patroni/ha.py

@@ -765,8 +765,10 @@ class Ha(object):
         if self.state_handler.is_starting():  # postgresql still starting up is unhealthy
             return False
 
-        if self.state_handler.is_leader():  # leader is always the healthiest
-            return True
+        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():
             return False
@@ -1352,6 +1354,8 @@ class Ha(object):
                     self.release_leader_key_voluntarily()
                     return 'released leader key voluntarily as data dir empty and currently leader'
 
+                if self.is_paused():
+                    return 'running with empty data directory'
                 return self.bootstrap()  # new node
             else:
                 # check if we are allowed to join
@@ -1363,9 +1367,16 @@ class Ha(object):
 
                 if self.sysid_valid(self.cluster.initialize):
                     if self.cluster.initialize != data_sysid:
-                        logger.fatal("system ID mismatch, node %s belongs to a different cluster: %s != %s",
-                                     self.state_handler.name, self.cluster.initialize, data_sysid)
-                        sys.exit(1)
+                        if self.is_paused():
+                            logger.warning('system ID has changed while in paused mode. Patroni will exit when resuming'
+                                           ' unless system ID is reset: %s != %s', self.cluster.initialize, data_sysid)
+                            if self.has_lock():
+                                self.release_leader_key_voluntarily()
+                                return 'released leader key voluntarily due to the system ID mismatch'
+                        else:
+                            logger.fatal('system ID mismatch, node %s belongs to a different cluster: %s != %s',
+                                         self.state_handler.name, self.cluster.initialize, data_sysid)
+                            sys.exit(1)
                 elif self.cluster.is_unlocked() and not self.is_paused():
                     # "bootstrap", but data directory is not empty
                     if not self.state_handler.cb_called and self.state_handler.is_running() \

patroni/postgresql/__init__.py

@@ -459,6 +459,8 @@ class Postgresql(object):
         self._pending_restart = False
 
         try:
+            if not self._major_version:
+                self.configure_server_parameters()
             configuration = self.config.effective_configuration
         except Exception:
             return None
@@ -1001,8 +1003,11 @@ class Postgresql(object):
     def schedule_sanity_checks_after_pause(self):
         """
             After coming out of pause we have to:
-            1. sync replication slots, because it might happen that slots were removed
-            2. get new 'Database system identifier' to make sure that it wasn't changed
+            1. configure server parameters if necessary
+            2. sync replication slots, because it might happen that slots were removed
+            3. get new 'Database system identifier' to make sure that it wasn't changed
         """
+        if not self._major_version:
+            self.configure_server_parameters()
         self.slots_handler.schedule()
         self._sysid = None

patroni/postgresql/validator.py

@@ -484,7 +484,7 @@ recovery_parameters = CaseInsensitiveDict({
     'promote_trigger_file': String(120000, None),
     'recovery_end_command': String(90300, None),
     'recovery_min_apply_delay': Integer(90400, None, 0, 2147483647, 'ms'),
-    'recovery_target': Enum(90400, None, ('immediate',)),
+    'recovery_target': Enum(90400, None, ('immediate', '')),
     'recovery_target_action': Enum(90500, None, ('pause', 'promote', 'shutdown')),
     'recovery_target_inclusive': Bool(90300, None),
     'recovery_target_lsn': String(100000, None),

tests/test_ha.py

@@ -1112,3 +1112,18 @@ class TestHa(PostgresInit):
         self.assertEqual(self.ha.run_cycle(), 'Unexpected exception raised, please report it as a BUG')
         self.ha.dcs.touch_member = Mock(side_effect=PatroniFatalException('foo'))
         self.assertRaises(PatroniFatalException, self.ha.run_cycle)
+
+    def test_empty_directory_in_pause(self):
+        self.ha.is_paused = true
+        self.p.data_directory_empty = true
+        self.assertEqual(self.ha.run_cycle(), 'PAUSE: running with empty data directory')
+        self.assertEqual(self.p.role, 'uninitialized')
+
+    @patch('patroni.ha.Ha.sysid_valid', MagicMock(return_value=True))
+    def test_sysid_no_match_in_pause(self):
+        self.ha.is_paused = true
+        self.p.controldata = lambda: {'Database cluster state': 'in recovery', 'Database system identifier': '123'}
+        self.assertEqual(self.ha.run_cycle(), 'PAUSE: continue to run as master without lock')
+
+        self.ha.has_lock = true
+        self.assertEqual(self.ha.run_cycle(), 'PAUSE: released leader key voluntarily due to the system ID mismatch')