Fix pg_rewind behaviour after pause (#2776)

On Slack user reported that Patroni didn't run pg_rewind on one of the nodes after coming out of maintenance mode.
Steps that were executed:
0. The initial state: node1 - primary, node2 - replica
1. `patronictl pause`
2. On node2: pg_ctl promote
3. On node1: pg_ctl stop
4. Patroni on node1 notice that Postgres isn't running and removes the leader lock
5. Patroni on node2 notice that Postgres is running as a primary and takes the leader lock.
6. `patronictl resume`.

After that node1 started saying in logs:
`Waiting for checkpoint on node2 before rewind`.

Repeating this steps may not necessarily reproduce the problem, because presumably pg_rewind failed earlier on node1.

Such situation was possible because promote wasn't executed by Patroni and therefore `Rewind._state` wasn't explicitly reset and the code that ensures that CHECKPOINT after promote was run wasn't triggered.

As a mitigation following changes have been made:
1. retrigger pg_rewind checks after coming out of maintenance mode
2. run ensure CHECKPOINT after promote checks using `Rewind._state != REWIND_STATUS.CHECKPOINT` condition. It allowed to remove useless hook from `Postgresql.promote()`.

patroni/ha.py

@@ -497,6 +497,7 @@ class Ha(object):
 
         role = 'replica'
         if self.has_lock() and not self.is_standby_cluster():
+            self._rewind.reset_state()  # we want to later trigger CHECKPOINT after promote
             msg = "starting as readonly because i had the session lock"
             node_to_follow = None
         else:
@@ -769,18 +770,14 @@ class Ha(object):
                 self.state_handler.sync_handler.set_synchronous_standby_names(
                     CaseInsensitiveSet('*') if self.global_config.is_synchronous_mode_strict else CaseInsensitiveSet())
             if self.state_handler.role not in ('master', 'promoted', 'primary'):
-                def on_success():
-                    self._rewind.reset_state()
-                    logger.info("cleared rewind state after becoming the leader")
-
                 def before_promote():
                     self.notify_citus_coordinator('before_promote')
 
                 with self._async_response:
                     self._async_response.reset()
+
                 self._async_executor.try_run_async('promote', self.state_handler.promote,
-                                                   args=(self.dcs.loop_wait, self._async_response,
-                                                         before_promote, on_success))
+                                                   args=(self.dcs.loop_wait, self._async_response, before_promote))
             return promote_message
 
     def fetch_node_status(self, member: Member) -> _MemberStatus:
@@ -1614,6 +1611,9 @@ class Ha(object):
             else:
                 if self._was_paused:
                     self.state_handler.schedule_sanity_checks_after_pause()
+                    # during pause people could manually do something with Postgres, therefore we want
+                    # to double check rewind conditions on replicas and maybe run CHECKPOINT on the primary
+                    self._rewind.reset_state()
                 self._was_paused = False
 
             if not self.cluster.has_member(self.state_handler.name):

patroni/postgresql/__init__.py

@@ -1125,8 +1125,8 @@ class Postgresql(object):
         except Exception as e:
             logger.error('Exception when calling `%s`: %r', cmd, e)
 
-    def promote(self, wait_seconds: int, task: CriticalTask, before_promote: Optional[Callable[..., Any]] = None,
-                on_success: Optional[Callable[..., Any]] = None) -> Optional[bool]:
+    def promote(self, wait_seconds: int, task: CriticalTask,
+                before_promote: Optional[Callable[..., Any]] = None) -> Optional[bool]:
         if self.role in ('promoted', 'master', 'primary'):
             return True
 
@@ -1152,8 +1152,6 @@ class Postgresql(object):
         ret = self.pg_ctl('promote', '-W')
         if ret:
             self.set_role('promoted')
-            if on_success is not None:
-                on_success()
             self.call_nowait(CallbackAction.ON_ROLE_CHANGE)
             ret = self._wait_promote(wait_seconds)
         return ret

patroni/postgresql/rewind.py

@@ -280,7 +280,7 @@ class Rewind(object):
         """After promote issue a CHECKPOINT from a new thread and asynchronously check the result.
         In case if CHECKPOINT failed, just check that timeline in pg_control was updated."""
 
-        if self._state == REWIND_STATUS.INITIAL and self._postgresql.is_leader():
+        if self._state != REWIND_STATUS.CHECKPOINT and self._postgresql.is_leader():
             with self._checkpoint_task_lock:
                 if self._checkpoint_task:
                     with self._checkpoint_task: