Merge branch 'master' of https://github.com/zalando/patroni

.travis.yml

@@ -110,5 +110,7 @@ script:
 
     set +e
 after_success:
+  # before_cache is executed earlier than after_success, so we need to restore one of virtualenv directories
+  - fpv=$(basename $(readlink $HOME/virtualenv/python3.5)) && mv $HOME/mycache/${fpv} $HOME/virtualenv/${fpv}
   - coveralls
   - if [[ $TEST_SUITE != "behave" ]]; then python-codacy-coverage -r coverage.xml; fi

docs/watchdog.rst

@@ -0,0 +1,34 @@
+================
+Watchdog support
+================
+
+Having multiple PostgreSQL servers running as master can result in transactions lost due to diverging timelines. This situation is also called a split-brain problem. To avoid split-brain Patroni needs to ensure PostgreSQL will not accept any transaction commits after leader key expires in the DCS. Under normal circumstances Patroni will try to achieve this by stopping PostgreSQL when leader lock update fails for any reason. However, this may fail to happen due to various reasons:
+
+- Patroni has crashed due to a bug, out-of-memory condition or by being accidentally killed by a system administrator.
+
+- Shutting down PostgreSQL is too slow.
+
+- Patroni does not get to run due to high load on the system, th VM being paused by the hypervisor, or other infrastructure issues.
+
+To guarantee correct behavior under these conditions Patroni supports watchdog devices. Watchdog devices are software or hardware mechanisms that will reset the whole system when they do not get a keepalive heartbeat within a specified timeframe.
+
+To be safe under all circumstances Patroni will set up the watchdog to expire after half of TTL. The watchdog will reset every time the high availability loop runs. This means that `ttl` must be at least twice `loop_wait` plus some safety margin. Default setup of `loop_wait=10` and `ttl=30` gives HA loop 5 seconds (ttl / 2 - loop_wait) to complete before the system gets forcefully reset. This is rather aggressive and you probably should increase `ttl` and/or reduce `loop_wait` if you decide to use a watchdog.
+
+Currently watchdogs are only supported using Linux watchdog device interface.
+
+Setting up software watchdog on Linux
+-------------------------------------
+
+Default Patroni configuration will try to use `/dev/watchdog` on Linux if it's accessible to Patroni. For most use cases using software watchdog built into the Linux kernel is secure enough.
+
+To enable software watchdog issue the following commands as root before starting Patroni:
+
+.. code-block:: bash
+
+    modprobe softdog
+    # Replace postgres with the user you will be running patroni under
+    chown postgres /dev/watchdog
+
+For testing it may be helpful to disable rebooting by adding `soft_noboot=1` to the modprobe command line. In this case the watchdog will just log a line in kernel ring buffer, visible via `dmesg`.
+
+Patroni will log information about the watchdog when it's successfully enabled.

extras/startup-scripts/README.md

@@ -11,3 +11,12 @@ Upstart job for Ubuntu 12.04 or 14.04.  Requires Upstart > 1.4. Intended for sys
 
 ### patroni.service
 Systemd service file, to be copied to /etc/systemd/system/patroni.service, tested on Centos 7.1 with Patroni installed from pip. 
+
+### patroni
+Init.d service file for Debian-like distributions. Copy it to /etc/init.d/, make executable:
+```chmod 755 /etc/init.d/patroni``` and run with ```service patroni start```, or make it starting on boot with ```update-rc.d patroni defaults```. Also you might edit some configuration variables in it:
+PATRONI for patroni.py location
+CONF for configuration file
+LOGFILE for log (script creates it if does not exist)
+
+Note. If you have several versions of Postgres installed, please add to POSTGRES_VERSION the release number which you wish to run. Script uses this value to append PATH environment with correct path to Postgres bin.

extras/startup-scripts/patroni

@@ -0,0 +1,144 @@
+#!/bin/sh
+#
+### BEGIN INIT INFO
+# Provides:          patroni
+# Required-Start:    $remote_fs $syslog
+# Required-Stop:     $remote_fs $syslog
+# Default-Start:     2 3 4 5
+# Default-Stop:      0 1 6
+# Short-Description: Patroni init script
+# Description:       Runners to orchestrate a high-availability PostgreSQL
+### END INIT INFO
+
+### BEGIN USER CONFIGURATION
+
+CONF="/etc/patroni/postgres.yml"
+LOGFILE="/var/log/patroni.log"
+USER="postgres"
+GROUP="postgres"
+
+NAME=patroni
+PATRONI="/opt/patroni/$NAME.py"
+PIDFILE="/var/run/$NAME.pid"
+
+# Set this parameter, if you have several Postgres versions installed
+# POSTGRES_VERSION="9.4"
+POSTGRES_VERSION=""
+
+### END USER CONFIGURATION
+
+. /lib/lsb/init-functions
+
+# Loading this library for get_versions() function
+if test ! -e /usr/share/postgresql-common/init.d-functions; then
+  log_failure_msg "Probably postgresql-common does not installed."
+  exit 1
+else
+  . /usr/share/postgresql-common/init.d-functions
+fi
+
+# Is there Patroni executable?
+if test ! -e $PATRONI; then
+  log_failure_msg "Patroni executable $PATRONI does not exist."
+  exit 1
+fi
+
+# Is there Patroni configuration file?
+if test ! -e $CONF; then
+  log_failure_msg "Patroni configuration file $CONF does not exist."
+  exit 1
+fi
+
+# Create logfile if doesn't exist
+if test ! -e $LOGFILE; then
+  log_action_msg "Creating logfile for Patroni..."
+  touch $LOGFILE
+  chown $USER:$GROUP $LOGFILE
+fi
+
+prepare_pgpath() {
+  if [ "$POSTGRES_VERSION" != "" ]; then
+    if [ -x /usr/lib/postgresql/$POSTGRES_VERSION/bin/pg_ctl ]; then
+      PGPATH="/usr/lib/postgresql/$POSTGRES_VERSION/bin"
+    else
+      log_failure_msg "Postgres version incorrect, check POSTGRES_VERSION variable."
+      exit 0
+    fi
+  else
+    get_versions
+    if echo $versions | grep -q -e "\s"; then
+      log_warning_msg "You have several Postgres versions installed. Please, use POSTGRES_VERSION to define correct environment."
+    else
+      versions=`echo $versions | sed -e 's/^[ \t]*//'`
+      PGPATH="/usr/lib/postgresql/$versions/bin"
+    fi
+  fi
+}
+
+get_pid() {
+  if test -e $PIDFILE; then
+    PID=`cat $PIDFILE`
+    CHILDPID=`ps --ppid $PID -o %p --no-headers`
+  else
+    log_failure_msg "Could not find PID file. Patroni probably down."
+    exit 1
+  fi
+}
+
+
+case "$1" in
+  start)
+    prepare_pgpath
+    PGPATH=$PATH:$PGPATH
+    log_success_msg "Starting Patroni\n"
+    exec start-stop-daemon --start --quiet \
+        --background \
+        --pidfile $PIDFILE --make-pidfile \
+        --chuid $USER:$GROUP \
+        --chdir `eval echo ~$USER` \
+        --exec $PATRONI \
+        --startas /bin/sh -- \
+        -c "/usr/bin/env PATH=$PGPATH /usr/bin/python $PATRONI $CONF >> $LOGFILE 2>&1"
+    ;;
+
+  stop)
+    log_success_msg "Stopping Patroni"
+    get_pid
+    start-stop-daemon --stop --pid $CHILDPID
+    start-stop-daemon --stop --pidfile $PIDFILE --remove-pidfile --quiet
+    ;;
+
+  reload)
+    log_success_msg "Reloading Patroni configuration"
+    get_pid
+    kill -HUP $CHILDPID
+    ;;
+
+  status)
+    get_pid
+    if start-stop-daemon -T --pid $CHILDPID; then
+      log_success_msg "Patroni is running\n"
+      exit 0
+    else
+      log_warning_msg "Patroni in not running\n"
+    fi
+    ;;
+
+  restart)
+    $0 stop
+    $0 start
+    ;;
+
+  *)
+    echo "Usage: /etc/init.d/$NAME {start|stop|restart|reload|status}"
+    exit 1
+    ;;
+esac
+
+if [ $? -eq 0 ]; then
+        echo .
+        exit 0
+else
+        echo " failed"
+        exit 1
+fi

extras/startup-scripts/patroni.service

@@ -21,7 +21,7 @@ ExecStart=/bin/patroni /etc/patroni.yml
 KillMode=process
 
 # Give a reasonable amount of time for the server to start up/shut down
-TimeoutSec=10
+TimeoutSec=30
 
 # Do not restart the service if it crashes, we want to manually inspect database on failure
 Restart=no

features/basic_replication.feature

@@ -34,7 +34,8 @@ Feature: basic replication
     And postgres1 role is the primary after 10 seconds
 
   Scenario: check rejoin of the former master with pg_rewind
-    Given I start postgres0
+    Given I add the table splitbrain to postgres0
+    And I start postgres0
     Then postgres0 role is the secondary after 20 seconds
     When I add the table buz to postgres1
     Then table buz is present on postgres0 after 20 seconds

features/environment.py

@@ -1,14 +1,18 @@
 import abc
 import consul
+import datetime
 import etcd
 import kazoo.client
 import kazoo.exceptions
 import os
+import psutil
 import psycopg2
 import shutil
+import signal
 import six
 import subprocess
 import tempfile
+import threading
 import time
 import yaml
 
@@ -74,18 +78,28 @@ class AbstractController(object):
         if self._log:
             self._log.close()
 
+    def cancel_background(self):
+        pass
 
 class PatroniController(AbstractController):
     __PORT = 5440
     PATRONI_CONFIG = '{}.yml'
     """ starts and stops individual patronis"""
 
-    def __init__(self, context, name, work_directory, output_dir, tags=None):
+    def __init__(self, context, name, work_directory, output_dir, tags=None, with_watchdog=False):
         super(PatroniController, self).__init__(context, 'patroni_' + name, work_directory, output_dir)
         PatroniController.__PORT += 1
         self._data_dir = os.path.join(work_directory, 'data', name)
         self._connstring = None
-        self._config = self._make_patroni_test_config(name, tags)
+        if with_watchdog:
+            self.watchdog = WatchdogMonitor(name, work_directory, output_dir)
+            custom_config = {'watchdog': {'driver': 'testing', 'device': self.watchdog.fifo_path, 'mode': 'required'}}
+        else:
+            self.watchdog = None
+            custom_config = None
+
+        self._config = self._make_patroni_test_config(name, tags, custom_config)
+        self._closables = []
 
         self._conn = None
         self._curs = None
@@ -109,6 +123,8 @@ class PatroniController(AbstractController):
                 yaml.safe_dump(config, w, default_flow_style=False)
 
     def _start(self):
+        if self.watchdog:
+            self.watchdog.start()
         return subprocess.Popen(['coverage', 'run', '--source=patroni', '-p', 'patroni.py', self._config],
                                 stdout=self._log, stderr=subprocess.STDOUT, cwd=self._work_directory)
 
@@ -116,11 +132,16 @@ class PatroniController(AbstractController):
         if postgres:
             return subprocess.call(['pg_ctl', '-D', self._data_dir, 'stop', '-mi', '-w'])
         super(PatroniController, self).stop(kill, timeout)
+        if self.watchdog:
+            self.watchdog.stop()
 
     def _is_accessible(self):
-        return self.query("SELECT 1", fail_ok=True) is not None
+        cursor = self.query("SELECT 1", fail_ok=True)
+        if cursor is not None:
+            cursor.execute("SET synchronous_commit TO 'local'")
+            return True
 
-    def _make_patroni_test_config(self, name, tags):
+    def _make_patroni_test_config(self, name, tags, custom_config):
         patroni_config_name = self.PATRONI_CONFIG.format(name)
         patroni_config_path = os.path.join(self._output_dir, patroni_config_name)
 
@@ -148,6 +169,15 @@ class PatroniController(AbstractController):
         if tags:
             config['tags'] = tags
 
+        if custom_config is not None:
+            def recursive_update(dst, src):
+                for k, v in src.items():
+                    if k in dst and isinstance(dst[k], dict):
+                        recursive_update(dst[k], v)
+                    else:
+                        dst[k] = v
+            recursive_update(config, custom_config)
+
         with open(patroni_config_path, 'w') as f:
             yaml.safe_dump(config, f, default_flow_style=False)
 
@@ -185,6 +215,87 @@ class PatroniController(AbstractController):
             time.sleep(1)
         return False
 
+    def get_watchdog(self):
+        return self.watchdog
+
+    def _get_pid(self):
+        try:
+            pidfile = os.path.join(self._data_dir, 'postmaster.pid')
+            if not os.path.exists(pidfile):
+                return None
+            return int(open(pidfile).readline().strip())
+        except:
+            return None
+
+    def database_is_running(self):
+        pid = self._get_pid()
+        if not pid:
+            return False
+        try:
+            os.kill(pid, 0)
+        except OSError:
+            return False
+        return True
+
+    def postmaster_hang(self, timeout):
+        hang = ProcessHang(self._get_pid(), timeout)
+        self._closables.append(hang)
+        hang.start()
+
+    def checkpoint_hang(self, timeout):
+        pid = self._get_pid()
+        if not pid:
+            return False
+        proc = psutil.Process(pid)
+        for child in proc.children():
+            if 'checkpoint' in child.cmdline()[0]:
+                checkpointer = child
+                break
+        else:
+            return False
+        hang = ProcessHang(checkpointer.pid, timeout)
+        self._closables.append(hang)
+        hang.start()
+        return True
+
+    def cancel_background(self):
+        for obj in self._closables:
+            obj.close()
+        self._closables = []
+
+    def terminate_backends(self):
+        pid = self._get_pid()
+        if not pid:
+            return False
+        proc = psutil.Process(pid)
+        for p in proc.children():
+            if 'process' not in p.cmdline()[0]:
+                p.terminate()
+
+class ProcessHang(object):
+
+    """A background thread implementing a cancelable process hang via SIGSTOP."""
+
+    def __init__(self, pid, timeout):
+        self._cancelled = threading.Event()
+        self._thread = threading.Thread(target=self.run)
+        self.pid = pid
+        self.timeout = timeout
+
+    def start(self):
+        self._thread.start()
+
+    def run(self):
+        os.kill(self.pid, signal.SIGSTOP)
+        try:
+            self._cancelled.wait(self.timeout)
+        finally:
+            os.kill(self.pid, signal.SIGCONT)
+
+    def close(self):
+        self._cancelled.set()
+        self._thread.join()
+
 
 class AbstractDcsController(AbstractController):
 
@@ -383,13 +494,15 @@ class PatroniPoolController(object):
     def output_dir(self):
         return self._output_dir
 
-    def start(self, name, max_wait_limit=20, tags=None):
+    def start(self, name, max_wait_limit=20, tags=None, with_watchdog=False):
         if name not in self._processes:
-            self._processes[name] = PatroniController(self._context, name, self.patroni_path, self._output_dir, tags)
+            self._processes[name] = PatroniController(self._context, name, self.patroni_path, self._output_dir, tags, with_watchdog=with_watchdog)
         self._processes[name].start(max_wait_limit)
 
     def __getattr__(self, func):
-        if func not in ['stop', 'query', 'write_label', 'read_label', 'check_role_has_changed_to', 'add_tag_to_config']:
+        if func not in ['stop', 'query', 'write_label', 'read_label', 'check_role_has_changed_to', 'add_tag_to_config',
+                        'get_watchdog', 'database_is_running', 'checkpoint_hang', 'postmaster_hang',
+                        'terminate_backends']:
             raise AttributeError("PatroniPoolController instance has no attribute '{0}'".format(func))
 
         def wrapper(name, *args, **kwargs):
@@ -398,6 +511,7 @@ class PatroniPoolController(object):
 
     def stop_all(self):
         for ctl in self._processes.values():
+            ctl.cancel_background()
             ctl.stop()
         self._processes.clear()
 
@@ -416,6 +530,123 @@ class PatroniPoolController(object):
         return self._dcs
 
 
+class WatchdogMonitor(object):
+    """Testing harness for emulating a watchdog device as a named pipe. Because we can't easily emulate ioctl's we
+    require a custom driver on Patroni side. The device takes no action, only notes if it was pinged and/or triggered.
+    """
+    def __init__(self, name, work_directory, output_dir):
+        self.fifo_path = os.path.join(work_directory, 'data', 'watchdog.{0}.fifo'.format(name))
+        self.fifo_file = None
+        self._stop_requested = False # Relying on bool setting being atomic
+        self._thread = None
+        self.last_ping = None
+        self.was_pinged = False
+        self.was_closed = False
+        self._was_triggered = False
+        self.timeout = 60
+        self._log_file = open(os.path.join(output_dir, 'watchdog.{0}.log'.format(name)), 'w')
+        self._log("watchdog {0} initialized".format(name))
+
+    def _log(self, msg):
+        tstamp = datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S,%f")
+        self._log_file.write("{0}: {1}\n".format(tstamp, msg))
+
+    def start(self):
+        assert self._thread is None
+        self._stop_requested = False
+        self._log("starting fifo {0}".format(self.fifo_path))
+        fifo_dir = os.path.dirname(self.fifo_path)
+        if os.path.exists(self.fifo_path):
+            os.unlink(self.fifo_path)
+        elif not os.path.exists(fifo_dir):
+            os.mkdir(fifo_dir)
+        os.mkfifo(self.fifo_path)
+        self.last_ping = time.time()
+
+        self._thread = threading.Thread(target=self.run)
+        self._thread.start()
+
+    def run(self):
+        try:
+            while not self._stop_requested:
+                self._log("opening")
+                self.fifo_file = os.open(self.fifo_path, os.O_RDONLY)
+                try:
+                    self._log("Fifo {0} connected".format(self.fifo_path))
+                    self.was_closed = False
+                    while not self._stop_requested:
+                        c = os.read(self.fifo_file, 1)
+
+                        if c == b'X':
+                            self._log("Stop requested")
+                            return
+                        elif c == b'':
+                            self._log("Pipe closed")
+                            break
+                        elif c == b'C':
+                            command = b''
+                            c = os.read(self.fifo_file, 1)
+                            while c != b'\n' and c != b'':
+                                command += c
+                                c = os.read(self.fifo_file, 1)
+                            command = command.decode('utf8')
+
+                            if command.startswith('timeout='):
+                                self.timeout = int(command.split('=')[1])
+                                self._log("timeout={0}".format(self.timeout))
+                        elif c in [b'V', b'1']:
+                            cur_time = time.time()
+                            if cur_time - self.last_ping > self.timeout:
+                                self._log("Triggered")
+                                self._was_triggered = True
+                            if c == b'V':
+                                self._log("magic close")
+                                self.was_closed = True
+                            elif c == b'1':
+                                self.was_pinged = True
+                                self._log("ping after {0} seconds".format(cur_time - (self.last_ping or cur_time)))
+                                self.last_ping = cur_time
+                        else:
+                            self._log('Unknown command {0} received from fifo'.format(c))
+                finally:
+                    self.was_closed = True
+                    self._log("closing")
+                    os.close(self.fifo_file)
+        except Exception as e:
+            self._log("Error {0}".format(e))
+        finally:
+            self._log("stopping")
+            self._log_file.flush()
+            if os.path.exists(self.fifo_path):
+                os.unlink(self.fifo_path)
+
+    def stop(self):
+        self._log("Monitor stop")
+        self._stop_requested = True
+        try:
+            if os.path.exists(self.fifo_path):
+                fd = os.open(self.fifo_path, os.O_WRONLY)
+                os.write(fd, b'X')
+                os.close(fd)
+        except Exception as e:
+            self._log("err while closing: {0}".format(str(e)))
+        if self._thread:
+            self._thread.join()
+            self._thread = None
+
+
+    def reset(self):
+        self._log("reset")
+        self.was_pinged = self.was_closed = self._was_triggered = False
+
+    @property
+    def was_triggered(self):
+        delta = time.time() - self.last_ping
+        triggered = self._was_triggered or not self.was_closed and delta > self.timeout
+        self._log("triggered={0}, {1}s left".format(triggered, self.timeout - delta))
+        return triggered
+
+
 # actions to execute on start/stop of the tests and before running invidual features
 def before_all(context):
     context.ci = 'TRAVIS_BUILD_NUMBER' in os.environ or 'BUILD_NUMBER' in os.environ

features/steps/basic_replication.py

@@ -11,7 +11,7 @@ def start_patroni(context, name):
 
 @step('I shut down {name:w}')
 def stop_patroni(context, name):
-    return context.pctl.stop(name)
+    return context.pctl.stop(name, timeout=60)
 
 
 @step('I kill {name:w}')

features/steps/patroni_api.py

@@ -111,7 +111,7 @@ def check_response(context, component, data):
         assert context.status_code == int(data),\
             "status code {0} != {1}, response: {2}".format(context.status_code, data, context.response)
     elif component == 'returncode':
-        assert context.status_code == int(data), "return code {0} != {1}".format(context.status_code, data)
+        assert context.status_code == int(data), "return code {0} != {1}, {2}".format(context.status_code, data, context.response)
     elif component == 'text':
         assert context.response == data.strip('"'), "response {0} does not contain {1}".format(context.response, data)
     elif component == 'output':

features/steps/watchdog.py

@@ -0,0 +1,74 @@
+from behave import step, then
+import time
+
+def polling_loop(timeout, interval=1):
+    """Returns an iterator that returns values until timeout has passed. Timeout is measured from start of iteration."""
+    start_time = time.time()
+    iteration = 0
+    end_time = start_time + timeout
+    while time.time() < end_time:
+        yield iteration
+        iteration += 1
+        time.sleep(interval)
+
+
+
+@step('I start {name:w} with watchdog')
+def start_patroni_with_watchdog(context, name):
+    return context.pctl.start(name, with_watchdog=True)
+
+
+@step('{name:w} watchdog has been pinged after {timeout:d} seconds')
+def watchdog_was_pinged(context, name, timeout):
+    for _ in polling_loop(timeout):
+        if context.pctl.get_watchdog(name).was_pinged:
+            return True
+    return False
+
+
+@then('{name:w} watchdog has been closed')
+def watchdog_was_closed(context, name):
+    assert context.pctl.get_watchdog(name).was_closed
+
+
+@step('I wait for next {name:w} watchdog ping')
+def watchdog_reset_pinged(context, name):
+    context.pctl.get_watchdog(name).reset()
+
+
+@then('{name:w} watchdog is triggered after {timeout:d} seconds')
+def watchdog_was_triggered(context, name, timeout):
+    for _ in polling_loop(timeout):
+        if context.pctl.get_watchdog(name).was_triggered:
+            return True
+    assert False
+
+
+@then('{name:w} watchdog was not triggered')
+def watchdog_was_not_triggered(context, name):
+    assert not context.pctl.get_watchdog(name).was_triggered
+
+
+@step('{name:w} checkpoint takes {timeout:d} seconds')
+def checkpoint_hang(context, name, timeout):
+    assert context.pctl.checkpoint_hang(name, timeout)
+
+
+@step('{name:w} hangs for {timeout:d} seconds')
+def postmaster_hang(context, name, timeout):
+    return context.pctl.postmaster_hang(name, timeout)
+
+
+@step('I terminate {name:w} user processes')
+def terminate_backends(context, name):
+    return context.pctl.terminate_backends(name)
+
+
+@step('Sleep for {timeout:d} seconds')
+def dcs_connection_lost(context, timeout):
+    time.sleep(timeout)
+
+
+@then('{name:w} database is running')
+def database_is_running(context, name):
+    assert context.pctl.database_is_running(name)

features/watchdog.feature

@@ -0,0 +1,43 @@
+Feature: watchdog
+  Verify that watchdog gets pinged and triggered under appropriate circumstances.
+
+  Scenario: watchdog is opened, pinged and closed
+    Given I start postgres0 with watchdog
+    Then postgres0 is a leader after 10 seconds
+    And postgres0 role is the primary after 10 seconds
+    And postgres0 watchdog has been pinged after 10 seconds
+    When I shut down postgres0
+    Then postgres0 watchdog has been closed
+
+  Scenario: watchdog is updated during pause
+    Given I start postgres0 with watchdog
+    Then postgres0 role is the primary after 10 seconds
+    When I run patronictl.py pause batman
+    And I wait for next postgres0 watchdog ping
+    Then I receive a response returncode 0
+    And postgres0 watchdog has been pinged after 10 seconds
+    When I shut down postgres0
+    Then postgres0 watchdog has been closed
+    And postgres0 database is running
+
+  Scenario: watchdog is updated during shutdown checkpoint
+    Given I start postgres0 with watchdog
+    Then postgres0 role is the primary after 10 seconds
+    And Sleep for 10 seconds
+    Given I run patronictl.py resume batman
+    Then I receive a response returncode 0
+    When I start postgres1
+    Then postgres1 role is the secondary after 10 seconds
+    When postgres0 checkpoint takes 30 seconds
+    And I shut down postgres0
+    Then postgres0 watchdog was not triggered
+    And postgres1 role is the primary after 10 seconds
+
+  Scenario: watchdog is triggered if postgres stops responding
+    Given I start postgres0 with watchdog
+    Then postgres0 role is the secondary after 10 seconds
+    When I shut down postgres1
+    Then postgres0 role is the primary after 10 seconds
+    When postgres0 hangs for 30 seconds
+    And I terminate postgres0 user processes
+    Then postgres0 watchdog is triggered after 30 seconds

patroni/__init__.py

@@ -16,6 +16,7 @@ class Patroni(object):
         from patroni.ha import Ha
         from patroni.postgresql import Postgresql
         from patroni.version import __version__
+        from patroni.watchdog import Watchdog
 
         self.setup_signal_handlers()
 
@@ -26,6 +27,7 @@ class Patroni(object):
 
         self.postgresql = Postgresql(self.config['postgresql'])
         self.api = RestApiServer(self, self.config['restapi'])
+        self.watchdog = Watchdog(self.config)
         self.ha = Ha(self)
 
         self.tags = self.get_tags()
@@ -98,6 +100,7 @@ class Patroni(object):
             self.next_run = time.time()
 
     def run(self):
+        self.ha.start()
         self.api.start()
         self.next_run = time.time()
 
@@ -124,6 +127,10 @@ class Patroni(object):
         signal.signal(signal.SIGHUP, self.sighup_handler)
         signal.signal(signal.SIGTERM, self.sigterm_handler)
 
+    def shutdown(self):
+        self.api.shutdown()
+        self.ha.shutdown()
+
 
 def patroni_main():
     logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO)
@@ -135,12 +142,7 @@ def patroni_main():
     except KeyboardInterrupt:
         pass
     finally:
-        patroni.api.shutdown()
-        if patroni.ha.is_paused():
-            logger.info('Leader key is not deleted and Postgresql is not stopped due paused state')
-        else:
-            patroni.ha.while_not_sync_standby(lambda: patroni.postgresql.stop(checkpoint=False))
-            patroni.dcs.delete_leader()
+        patroni.shutdown()
 
 
 def pg_ctl_start(args):

patroni/api.py

@@ -382,14 +382,16 @@ class RestApiHandler(BaseHTTPRequestHandler):
                                        pg_is_in_recovery(),
                                        CASE WHEN pg_is_in_recovery()
                                             THEN 0
-                                            ELSE pg_xlog_location_diff(pg_current_xlog_location(), '0/0')::bigint
+                                            ELSE pg_{0}_{1}_diff(pg_current_{0}_{1}(), '0/0')::bigint
                                        END,
-                                       pg_xlog_location_diff(COALESCE(pg_last_xlog_receive_location(),
-                                                                      pg_last_xlog_replay_location()), '0/0')::bigint,
-                                       pg_xlog_location_diff(pg_last_xlog_replay_location(), '0/0')::bigint,
+                                       pg_{0}_{1}_diff(COALESCE(pg_last_{0}_receive_{1}(),
+                                                                      pg_last_{0}_replay_{1}()), '0/0')::bigint,
+                                       pg_{0}_{1}_diff(pg_last_{0}_replay_{1}(), '0/0')::bigint,
                                        to_char(pg_last_xact_replay_timestamp(), 'YYYY-MM-DD HH24:MI:SS.MS TZ'),
-                                       pg_is_in_recovery() AND pg_is_xlog_replay_paused(),
-                                       (SELECT array_to_json(array_agg(row_to_json(ri))) FROM replication_info ri)""",
+                                       pg_is_in_recovery() AND pg_is_{0}_replay_paused(),
+                                       (SELECT array_to_json(array_agg(row_to_json(ri)))
+                                          FROM replication_info ri)""".format(self.server.patroni.postgresql.wal_name,
+                                                                              self.server.patroni.postgresql.lsn_name),
                              retry=retry)[0]
 
             result = {

patroni/async_executor.py

@@ -1,9 +1,55 @@
 import logging
-from threading import RLock, Thread
+from threading import Lock, RLock, Thread
 
 logger = logging.getLogger(__name__)
 
 
+class CriticalTask(object):
+    """Represents a critical task in a background process that we either need to cancel or get the result of.
+
+    Fields of this object may be accessed only when holding a lock on it. To perform the critical task the background
+    thread must, while holding lock on this object, check `is_cancelled` flag, run the task and mark the task as
+    complete using `complete()`.
+
+    The main thread must hold async lock to prevent the task from completing, hold lock on critical task object,
+    call cancel. If the task has completed `cancel()` will return False and `result` field will contain the result of
+    the task. When cancel returns True it is guaranteed that the background task will notice the `is_cancelled` flag.
+    """
+    def __init__(self):
+        self._lock = Lock()
+        self.is_cancelled = False
+        self.result = None
+
+    def reset(self):
+        """Must be called every time the background task is finished.
+
+        Must be called from async thread. Caller must hold lock on async executor when calling."""
+        self.is_cancelled = False
+        self.result = None
+
+    def cancel(self):
+        """Tries to cancel the task, returns True if the task has already run.
+
+        Caller must hold lock on async executor and the task when calling."""
+        if self.result is not None:
+            return False
+        self.is_cancelled = True
+        return True
+
+    def complete(self, result):
+        """Mark task as completed along with a result.
+
+        Must be called from async thread. Caller must hold lock on task when calling."""
+        self.result = result
+
+    def __enter__(self):
+        self._lock.acquire()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self._lock.release()
+
+
 class AsyncExecutor(object):
 
     def __init__(self, ha_wakeup):
@@ -11,6 +57,7 @@ class AsyncExecutor(object):
         self._thread_lock = RLock()
         self._scheduled_action = None
         self._scheduled_action_lock = RLock()
+        self.critical_task = CriticalTask()
 
     @property
     def busy(self):
@@ -43,6 +90,8 @@ class AsyncExecutor(object):
         finally:
             with self:
                 self.reset_scheduled_action()
+                with self.critical_task:
+                    self.critical_task.reset()
             if wakeup is not None:
                 self._ha_wakeup()
 

patroni/config.py

@@ -43,10 +43,14 @@ class Config(object):
         'maximum_lag_on_failover': 1048576,
         'master_start_timeout': 300,
         'synchronous_mode': False,
+        'synchronous_mode_strict': False,
         'postgresql': {
             'bin_dir': '',
             'use_slots': True,
             'parameters': {p: v[0] for p, v in Postgresql.CMDLINE_OPTIONS.items()}
+        },
+        'watchdog': {
+            'mode': 'automatic',
         }
     }
 
@@ -174,7 +178,7 @@ class Config(object):
                     elif name not in ('connect_address', 'listen', 'data_dir', 'pgpass', 'authentication'):
                         config['postgresql'][name] = deepcopy(value)
             elif name in config:  # only variables present in __DEFAULT_CONFIG allowed to be overriden from DCS
-                if name == 'synchronous_mode':
+                if name in ('synchronous_mode', 'synchronous_mode_strict'):
                     config[name] = value
                 else:
                     config[name] = int(value)
@@ -271,7 +275,7 @@ class Config(object):
                         config['postgresql'][name].update(self._process_postgresql_parameters(value, True))
                     elif name != 'use_slots':  # replication slots must be enabled/disabled globally
                         config['postgresql'][name] = deepcopy(value)
-            elif name not in config:
+            elif name not in config or name in ['watchdog']:
                 config[name] = deepcopy(value) if value else {}
 
         # restapi server expects to get restapi.auth = 'username:password'

patroni/ctl.py

@@ -4,27 +4,36 @@ Patroni Control
 
 import base64
 import click
+import codecs
 import datetime
 import dateutil.parser
+import cdiff
+import copy
+import difflib
+import io
 import json
 import logging
 import os
 import psycopg2
 import random
 import requests
+import subprocess
 import sys
+import tempfile
 import time
 import tzlocal
 import yaml
 
 from click import ClickException
+from contextlib import contextmanager
 from patroni.config import Config
 from patroni.dcs import get_dcs as _get_dcs
 from patroni.exceptions import PatroniException
 from patroni.postgresql import Postgresql
-from patroni.utils import is_valid_pg_version
+from patroni.utils import is_valid_pg_version, patch_config
 from prettytable import PrettyTable
 from six.moves.urllib_parse import urlparse
+from six import text_type
 
 CONFIG_DIR_PATH = click.get_app_dir('patroni')
 CONFIG_FILE_PATH = os.path.join(CONFIG_DIR_PATH, 'patronictl.yaml')
@@ -819,3 +828,204 @@ def pause(obj, cluster_name):
 @click.pass_obj
 def resume(obj, cluster_name):
     return toggle_pause(obj, cluster_name, False)
+
+
+@contextmanager
+def temporary_file(contents, suffix='', prefix='tmp'):
+    """Creates a temporary file with specified contents that persists for the context.
+
+    :param contents: binary string that will be written to the file.
+    :param prefix: will be prefixed to the filename.
+    :param suffix: will be appended to the filename.
+    :returns path of the created file.
+    """
+    tmp = tempfile.NamedTemporaryFile(suffix=suffix, prefix=prefix, delete=False)
+    with tmp:
+        tmp.write(contents)
+
+    try:
+        yield tmp.name
+    finally:
+        os.unlink(tmp.name)
+
+
+def show_diff(before_editing, after_editing):
+    """Shows a diff between two strings.
+
+    If the output is to a tty the diff will be colored. Inputs are expected to be unicode strings.
+    """
+    def listify(string):
+        return [l+'\n' for l in string.rstrip('\n').split('\n')]
+
+    unified_diff = difflib.unified_diff(listify(before_editing), listify(after_editing))
+
+    if sys.stdout.isatty():
+        buf = io.StringIO()
+        for line in unified_diff:
+            # Force cast to unicode as difflib on Python 2.7 returns a mix of unicode and str.
+            buf.write(text_type(line))
+        buf.seek(0)
+
+        class opts:
+            side_by_side = False
+            width = 80
+            tab_width = 8
+        cdiff.markup_to_pager(cdiff.PatchStream(buf), opts)
+    else:
+        for line in unified_diff:
+            click.echo(line.rstrip('\n'))
+
+
+def format_config_for_editing(data):
+    """Formats configuration as YAML for human consumption.
+
+    :param data: configuration as nested dictionaries
+    :returns unicode YAML of the configuration"""
+    return yaml.safe_dump(data, default_flow_style=False, encoding=None, allow_unicode=True)
+
+
+def apply_config_changes(before_editing, data, kvpairs):
+    """Applies config changes specified as a list of key-value pairs.
+
+    Keys are interpreted as dotted paths into the configuration data structure. Except for paths beginning with
+    `postgresql.parameters` where rest of the path is used directly to allow for PostgreSQL GUCs containing dots.
+    Values are interpreted as YAML values.
+
+    :param before_editing: human representation before editing
+    :param data: configuration datastructure
+    :param kvpairs: list of strings containing key value pairs separated by =
+    :returns tuple of human readable and parsed datastructure after changes
+    """
+    changed_data = copy.deepcopy(data)
+
+    def set_path_value(config, path, value, prefix=()):
+        # Postgresql GUCs can't be nested, but can contain dots so we re-flatten the structure for this case
+        if prefix == ('postgresql', 'parameters'):
+            path = ['.'.join(path)]
+
+        if len(path) == 1:
+            if value is None:
+                config.pop(path[0], None)
+            else:
+                config[path[0]] = value
+        else:
+            key = path[0]
+            if key not in config:
+                config[key] = {}
+            set_path_value(config[key], path[1:], value, prefix + (key,))
+            if config[key] == {}:
+                del config[key]
+
+    for pair in kvpairs:
+        if not pair or "=" not in pair:
+            raise PatroniCtlException("Invalid parameter setting {0}".format(pair))
+        key_path, value = pair.split("=", 1)
+        set_path_value(changed_data, key_path.strip().split("."), yaml.safe_load(value))
+
+    return format_config_for_editing(changed_data), changed_data
+
+
+def apply_yaml_file(data, filename):
+    """Applies changes from a YAML file to configuration
+
+    :param data: configuration datastructure
+    :param filename: name of the YAML file, - is taken to mean standard input
+    :returns tuple of human readable and parsed datastructure after changes
+    """
+    changed_data = copy.deepcopy(data)
+
+    if filename == '-':
+        new_options = yaml.safe_load(sys.stdin)
+    else:
+        with open(filename) as fd:
+            new_options = yaml.safe_load(fd)
+
+    patch_config(changed_data, new_options)
+
+    return format_config_for_editing(changed_data), changed_data
+
+
+def invoke_editor(before_editing, cluster_name):
+    """Starts editor command to edit configuration in human readable format
+
+    :param before_editing: human representation before editing
+    :returns tuple of human readable and parsed datastructure after changes
+    """
+    editor_cmd = os.environ.get('EDITOR')
+    if not editor_cmd:
+        raise PatroniCtlException('EDITOR environment variable is not set')
+
+    with temporary_file(contents=before_editing.encode('utf-8'),
+                        suffix='.yaml',
+                        prefix='{0}-config-'.format(cluster_name)) as tmpfile:
+        ret = subprocess.call([editor_cmd, tmpfile])
+        if ret:
+            raise PatroniCtlException("Editor exited with return code {0}".format(ret))
+
+        with codecs.open(tmpfile, encoding='utf-8') as fd:
+            after_editing = fd.read()
+
+        return after_editing, yaml.safe_load(after_editing)
+
+
+@ctl.command('edit-config', help="Edit cluster configuration")
+@click.argument('cluster_name')
+@click.option('--quiet', '-q', is_flag=True, help='Do not show changes')
+@click.option('--set', '-s', 'kvpairs', multiple=True,
+              help='Set specific configuration value. Can be specified multiple times')
+@click.option('--pg', '-p', 'pgkvpairs', multiple=True,
+              help='Set specific PostgreSQL parameter value. Shorthand for -s postgresql.parameters. '
+                   'Can be specified multiple times')
+@click.option('--apply', 'apply_filename', help='Apply configuration from file. Use - for stdin.')
+@click.option('--replace', 'replace_filename', help='Apply configuration from file, replacing existing configuration.'
+              ' Use - for stdin.')
+@option_force
+@click.pass_obj
+def edit_config(obj, cluster_name, force, quiet, kvpairs, pgkvpairs, apply_filename, replace_filename):
+    dcs = get_dcs(obj, cluster_name)
+    cluster = dcs.get_cluster()
+
+    before_editing = format_config_for_editing(cluster.config.data)
+
+    after_editing = None  # Serves as a flag if any changes were requested
+    changed_data = cluster.config.data
+
+    if replace_filename:
+        after_editing, changed_data = apply_yaml_file({}, replace_filename)
+
+    if apply_filename:
+        after_editing, changed_data = apply_yaml_file(changed_data, apply_filename)
+
+    if kvpairs or pgkvpairs:
+        all_pairs = list(kvpairs) + ['postgresql.parameters.'+v.lstrip() for v in pgkvpairs]
+        after_editing, changed_data = apply_config_changes(before_editing, changed_data, all_pairs)
+
+    # If no changes were specified on the command line invoke editor
+    if after_editing is None:
+        after_editing, changed_data = invoke_editor(before_editing, cluster_name)
+
+    if cluster.config.data == changed_data:
+        if not quiet:
+            click.echo("Not changed")
+        return
+
+    if not quiet:
+        show_diff(before_editing, after_editing)
+
+    if (apply_filename == '-' or replace_filename == '-') and not force:
+        click.echo("Use --force option to apply changes")
+        return
+
+    if force or click.confirm('Apply these changes?'):
+        if not dcs.set_config_value(json.dumps(changed_data), cluster.config.modify_index):
+            raise PatroniCtlException("Config modification aborted due to concurrent changes")
+        click.echo("Configuration changed")
+
+
+@ctl.command('show-config', help="Show cluster configuration")
+@click.argument('cluster_name')
+@click.pass_obj
+def show_config(obj, cluster_name):
+    cluster = get_dcs(obj, cluster_name).get_cluster()
+
+    click.echo(format_config_for_editing(cluster.config.data))

patroni/exceptions.py

@@ -23,3 +23,7 @@ class DCSError(PatroniException):
 
 class PostgresConnectionException(PostgresException):
     pass
+
+
+class WatchdogError(PatroniException):
+    pass

patroni/ha.py

@@ -12,26 +12,26 @@ from multiprocessing.pool import ThreadPool
 from patroni.async_executor import AsyncExecutor
 from patroni.exceptions import DCSError, PostgresConnectionException
 from patroni.postgresql import ACTION_ON_START
-from patroni.utils import polling_loop, tzutc
-from threading import RLock
+from patroni.utils import polling_loop, null_context, tzutc
+from threading import RLock, Event, Thread
 
 logger = logging.getLogger(__name__)
 
 
-class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,xlog_location,tags')):
+class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,wal_position,tags')):
     """Node status distilled from API response:
 
         member - dcs.Member object of the node
         reachable - `!False` if the node is not reachable or is not responding with correct JSON
         in_recovery - `!True` if pg_is_in_recovery() == true
-        xlog_location - value of `replayed_location` or `location` from JSON, dependin on its role.
+        wal_position - value of `replayed_location` or `location` from JSON, dependin on its role.
         tags - dictionary with values of different tags (i.e. nofailover)
     """
     @classmethod
     def from_api_response(cls, member, json):
         is_master = json['role'] == 'master'
-        xlog = not is_master and max(json['xlog'].get('received_location', 0), json['xlog'].get('replayed_location', 0))
-        return cls(member, True, not is_master, xlog, json.get('tags', {}))
+        wal = not is_master and max(json['xlog'].get('received_location', 0), json['xlog'].get('replayed_location', 0))
+        return cls(member, True, not is_master, wal, json.get('tags', {}))
 
     @classmethod
     def unknown(cls, member):
@@ -46,6 +46,53 @@ class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,xl
         return None
 
 
+class BackgroundKeepaliveSender(object):
+    """A context manager that sends keepalives every loop_wait seconds in a background thread while the context is
+    running, but only after a safepoint has been reached. After the safepoint PostgreSQL must not be allowed to
+    transition to master before the context has ended. Intended use is for long operations that run in main HA loop.
+
+    If safe event is given it must be triggered when no client can be accessing PostgreSQL as master. If this condition
+    is already guaranteed before entering the context the safe event can be omitted.
+    """
+    def __init__(self, ha, safe_event=None):
+        """
+        :param safe_event: None or threading.Event that is cleared when context is entered.
+        """
+        self.ha = ha
+        self.safe_event = safe_event
+        self._stop_event = Event()
+        self._bg_thread = Thread(target=self.run)
+        self.loop_wait = ha.dcs.loop_wait
+
+    def __enter__(self):
+        if self.safe_event is not None:
+            self.safe_event.clear()
+        self._bg_thread.start()
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        # FIXME: Do we want to handle the case where the safe event was not set?
+        # e.g. stop failed with an exception, looks like witholding keepalives is ok then
+        # We do want to avoid it when we don't have keepalives enabled, but maybe we can
+        # avoid creating the thread in the first place.
+        # if not self.safe_event.is_set():
+        #     self.safe_event.set()????
+        self._stop_event.set()
+        self._bg_thread.join()
+        # Always send at least one keepalive
+        self.ha.keepalive()
+
+    def run(self):
+        if self.safe_event is not None:
+            self.safe_event.wait()
+            logger.debug("Background keepalive safe event reached")
+        while not self._stop_event.is_set():
+            logger.debug("Sending background keepalive")
+            self.ha.keepalive()
+            if not self._stop_event.wait(self.loop_wait):
+                self.ha.keepalive_sent = False
+        logger.debug("Stopping background keepalive")
+
+
 class Ha(object):
 
     def __init__(self, patroni):
@@ -57,6 +104,7 @@ class Ha(object):
         self.recovering = False
         self._start_timeout = None
         self._async_executor = AsyncExecutor(self.wakeup)
+        self.watchdog = patroni.watchdog
 
         # Each member publishes various pieces of information to the DCS using touch_member. This lock protects
         # the state and publishing procedure to have consistent ordering and avoid publishing stale values.
@@ -64,6 +112,10 @@ class Ha(object):
         # Count of concurrent sync disabling requests. Value above zero means that we don't want to be synchronous
         # standby. Changes protected by _member_state_lock.
         self._disable_sync = 0
+        # We need to send keepalives at most once per lock update so it is guaranteed that keepalive expires before
+        # lock TTL runs out. However we want to do it as soon as we determine that it is safe to do so. This flag
+        # keeps track whether a keepalive has been sent in the current cycle.
+        self.keepalive_sent = False
 
     def is_paused(self):
         return self.cluster and self.cluster.is_paused()
@@ -77,15 +129,20 @@ class Ha(object):
         self.cluster = cluster
 
     def acquire_lock(self):
-        return self.dcs.attempt_to_acquire_leader()
+        ret = self.dcs.attempt_to_acquire_leader()
+        if ret:
+            self.keepalive()
+        return ret
 
     def update_lock(self, write_leader_optime=False):
         ret = self.dcs.update_leader()
-        if ret and write_leader_optime:
-            try:
-                self.dcs.write_leader_optime(self.state_handler.last_operation())
-            except:
-                pass
+        if ret:
+            self.keepalive()
+            if write_leader_optime:
+                try:
+                    self.dcs.write_leader_optime(self.state_handler.last_operation())
+                except:
+                    pass
         return ret
 
     def has_lock(self):
@@ -116,7 +173,7 @@ class Ha(object):
                 data['pending_restart'] = True
             if not self._async_executor.busy and data['state'] in ['running', 'restarting', 'starting']:
                 try:
-                    data['xlog_location'] = self.state_handler.xlog_position(retry=False)
+                    data['xlog_location'] = self.state_handler.wal_position(retry=False)
                 except:
                     pass
             if self.patroni.scheduled_restart:
@@ -131,7 +188,7 @@ class Ha(object):
             logger.info('bootstrapped %s', msg)
             cluster = self.dcs.get_cluster()
             node_to_follow = self._get_node_to_follow(cluster)
-            return self.state_handler.follow(node_to_follow, cluster.leader, True)
+            return self.state_handler.follow(node_to_follow)
         else:
             logger.error('failed to bootstrap %s', msg)
             self.state_handler.remove_data_directory()
@@ -147,28 +204,38 @@ class Ha(object):
         # no initialize key and node is allowed to be master and has 'bootstrap' section in a configuration file
         elif self.cluster.initialize is None and not self.patroni.nofailover and 'bootstrap' in self.patroni.config:
             if self.dcs.initialize(create_new=True):  # race for initialization
-                try:
-                    self.state_handler.bootstrap(self.patroni.config['bootstrap'])
-                    self.dcs.initialize(create_new=False, sysid=self.state_handler.sysid)
-                except:  # initdb or start failed
-                    # remove initialization key and give a chance to other members
-                    logger.info("removing initialize key after failed attempt to initialize the cluster")
-                    self.dcs.cancel_initialization()
-                    self.state_handler.stop('immediate')
-                    self.state_handler.move_data_directory()
-                    raise
-                self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration, separators=(',', ':')))
-                self.dcs.take_leader()
-                self.load_cluster_from_dcs()
-                return 'initialized a new cluster'
+                with self._background_keepalive_context(wait_for_safepoint=False):
+                    try:
+                        self.state_handler.bootstrap(self.patroni.config['bootstrap'])
+                        self.dcs.initialize(create_new=False, sysid=self.state_handler.sysid)
+                    except:  # initdb or start failed
+                        # remove initialization key and give a chance to other members
+                        logger.info("removing initialize key after failed attempt to initialize the cluster")
+                        self.dcs.cancel_initialization()
+                        self.state_handler.stop('immediate')
+                        self.state_handler.move_data_directory()
+                        raise
+                    self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration,
+                                                         separators=(',', ':')))
+                    self.dcs.take_leader()
+                    self.load_cluster_from_dcs()
+                    return 'initialized a new cluster'
             else:
                 return 'failed to acquire initialize lock'
         else:
             if self.state_handler.can_create_replica_without_replication_connection():
+                msg = 'bootstrap (without leader)'
+                self._async_executor.schedule(msg)
                 self._async_executor.run_async(self.clone)
-                return "trying to bootstrap (without leader)"
+                return 'trying to ' + msg
             return 'waiting for leader to bootstrap'
 
+    def _handle_rewind(self):
+        if self.state_handler.rewind_needed_and_possible(self.cluster.leader):
+            self._async_executor.schedule('running pg_rewind from ' + self.cluster.leader.name)
+            self._async_executor.run_async(self.state_handler.rewind, (self.cluster.leader,))
+            return True
+
     def recover(self):
         if self.has_lock() and self.update_lock():
             timeout = self.patroni.config['master_start_timeout']
@@ -182,9 +249,24 @@ class Ha(object):
         else:
             timeout = None
 
+        self.load_cluster_from_dcs()
+
+        if self.has_lock():
+            msg = "starting as readonly because i had the session lock"
+            node_to_follow = None
+        else:
+            if not self.state_handler.rewind_executed:
+                self.state_handler.trigger_check_diverged_lsn()
+            if self._handle_rewind():
+                return self._async_executor.scheduled_action
+            msg = "starting as a secondary"
+            node_to_follow = self._get_node_to_follow(self.cluster)
+
         self.recovering = True
-        return self.follow("starting as readonly because i had the session lock",
-                           "starting as a secondary", True, True, None, timeout)
+
+        self._async_executor.schedule('restarting after failure')
+        self._async_executor.run_async(self.state_handler.follow, (node_to_follow, timeout))
+        return msg
 
     def _get_node_to_follow(self, cluster):
         # determine the node to follow. If replicatefrom tag is set,
@@ -196,33 +278,43 @@ class Ha(object):
 
         return node_to_follow if node_to_follow and node_to_follow.name != self.state_handler.name else None
 
-    def follow(self, demote_reason, follow_reason, refresh=True, recovery=False, need_rewind=None, timeout=None):
+    def follow(self, demote_reason, follow_reason, refresh=True):
         if refresh:
             self.load_cluster_from_dcs()
 
-        if recovery:
-            ret = demote_reason if self.has_lock() else follow_reason
-        else:
-            is_leader = self.state_handler.is_leader()
-            ret = demote_reason if is_leader else follow_reason
+        is_leader = self.state_handler.is_leader()
 
         node_to_follow = self._get_node_to_follow(self.cluster)
 
-        if self.is_paused() and not (self.state_handler.need_rewind and self.state_handler.can_rewind):
-            self.state_handler.set_role('master' if is_leader else 'replica')
-            if is_leader:
-                return 'continue to run as master without lock'
-            elif not node_to_follow:
-                return 'no action'
+        if self.is_paused():
+            self.keepalive()
+            if not (self.state_handler.need_rewind and self.state_handler.can_rewind) or self.cluster.is_unlocked():
+                self.state_handler.set_role('master' if is_leader else 'replica')
+                if is_leader:
+                    return 'continue to run as master without lock'
+                elif not node_to_follow:
+                    return 'no action'
+        elif is_leader:
+            self.demote('immediate-nolock')
+            return demote_reason
+        else:
+            self.keepalive()
 
-        self.state_handler.follow(node_to_follow, self.cluster.leader, recovery,
-                                  self._async_executor, need_rewind, timeout)
+        if self._handle_rewind():
+            return self._async_executor.scheduled_action
 
-        return ret
+        if not self.state_handler.check_recovery_conf(node_to_follow):
+            self._async_executor.schedule('changing primary_conninfo and restarting')
+            self._async_executor.run_async(self.state_handler.follow, (node_to_follow,))
+
+        return follow_reason
 
     def is_synchronous_mode(self):
         return bool(self.cluster and self.cluster.config and self.cluster.config.data.get('synchronous_mode'))
 
+    def is_synchronous_mode_strict(self):
+        return bool(self.cluster and self.cluster.config and self.cluster.config.data.get('synchronous_mode_strict'))
+
     def process_sync_replication(self):
         """Process synchronous standby beahvior.
 
@@ -242,10 +334,15 @@ class Ha(object):
                     if not self.dcs.write_sync_state(self.state_handler.name, None, index=self.cluster.sync.index):
                         logger.info('Synchronous replication key updated by someone else.')
                         return
+
+                if self.is_synchronous_mode_strict() and picked is None:
+                    picked = '*'
+                    logger.warning("No standbys available!")
+
                 logger.info("Assigning synchronous standby status to %s", picked)
                 self.state_handler.set_synchronous_standby(picked)
 
-                if picked and not allow_promote:
+                if picked and picked != '*' and not allow_promote:
                     # Wait for PostgreSQL to enable synchronous mode and see if we can immediately set sync_standby
                     time.sleep(2)
                     picked, allow_promote = self.state_handler.pick_synchronous_standby(self.cluster)
@@ -344,21 +441,21 @@ class Ha(object):
         pool.join()
         return results
 
-    def is_lagging(self, xlog_location):
-        """Returns if instance with an xlog should consider itself unhealthy to be promoted due to replication lag.
+    def is_lagging(self, wal_position):
+        """Returns if instance with an wal should consider itself unhealthy to be promoted due to replication lag.
 
-        :param xlog_location: Current xlog location.
+        :param wal_position: Current wal position.
         :returns True when node is lagging
         """
-        lag = (self.cluster.last_leader_operation or 0) - xlog_location
+        lag = (self.cluster.last_leader_operation or 0) - wal_position
         return lag > self.state_handler.config.get('maximum_lag_on_failover', 0)
 
     def _is_healthiest_node(self, members, check_replication_lag=True):
         """This method tries to determine whether I am healthy enough to became a new leader candidate or not."""
 
-        my_xlog_location = self.state_handler.xlog_position()
-        if check_replication_lag and self.is_lagging(my_xlog_location):
-            return False  # Too far behind last reported xlog location on master
+        my_wal_position = self.state_handler.wal_position()
+        if check_replication_lag and self.is_lagging(my_wal_position):
+            return False  # Too far behind last reported wal position on master
 
         # Prepare list of nodes to run check against
         members = [m for m in members if m.name != self.state_handler.name and not m.nofailover and m.api_url]
@@ -369,7 +466,7 @@ class Ha(object):
                     if not st.in_recovery:
                         logger.warning('Master (%s) is still alive', st.member.name)
                         return False
-                    if my_xlog_location < st.xlog_location:
+                    if my_wal_position < st.wal_position:
                         return False
         return True
 
@@ -381,7 +478,7 @@ class Ha(object):
                 not_allowed_reason = st.failover_limitation()
                 if not_allowed_reason:
                     logger.info('Member %s is %s', st.member.name, not_allowed_reason)
-                elif self.is_lagging(st.xlog_location):
+                elif self.is_lagging(st.wal_position):
                     logger.info('Member %s exceeds maximum replication lag', st.member.name)
                 else:
                     ret = True
@@ -485,31 +582,47 @@ class Ha(object):
             graceful is used when failing over to another node due to user request. May only be called running async.
             immediate is used when we determine that we are not suitable for master and want to failover quickly
                 without regard for data durability. May only be called synchronously.
+            immediate-nolock is used when find out that we have lost the lock to be master. Need to bring down
+                PostgreSQL as quickly as possible without regard for data durability. May only be called synchronously.
         """
-        assert mode in ['offline', 'graceful', 'immediate']
-        if mode != 'offline':
-            if mode == 'immediate':
-                self.state_handler.stop('immediate', checkpoint=False)
-            else:
-                self.state_handler.stop()
+        mode_control = {
+            'offline':          dict(stop='fast', checkpoint=False, release=False, offline=True, async=False),
+            'graceful':         dict(stop='fast', checkpoint=True, release=True, offline=False, async=False),
+            'immediate':        dict(stop='immediate', checkpoint=False, release=True, offline=False, async=True),
+            'immediate-nolock': dict(stop='immediate', checkpoint=False, release=False, offline=False, async=True),
+        }[mode]
+
+        with self._background_keepalive_context() if mode != 'graceful' else null_context():
+            self.state_handler.trigger_check_diverged_lsn()
+            self.state_handler.stop(mode_control['stop'], checkpoint=mode_control['checkpoint'])
             self.state_handler.set_role('demoted')
-            self.release_leader_key_voluntarily()
-            time.sleep(2)  # Give a time to somebody to take the leader lock
-            cluster = self.dcs.get_cluster()
-            node_to_follow = self._get_node_to_follow(cluster)
-            if mode == 'immediate':
-                # We will try to start up as a standby now. If no one takes the leader lock before we finish
-                # recovery we will try to promote ourselves.
-                self._async_executor.schedule('waiting for failover to complete')
-                self._async_executor.run_async(self.state_handler.follow,
-                                               (node_to_follow, cluster.leader, True, None, True))
+
+            if mode_control['release']:
+                    self.release_leader_key_voluntarily()
+                    time.sleep(2)  # Give a time to somebody to take the leader lock
+            if mode_control['offline']:
+                node_to_follow, leader = None, None
             else:
-                return self.state_handler.follow(node_to_follow, cluster.leader, recovery=True, need_rewind=True)
+                cluster = self.dcs.get_cluster()
+                node_to_follow, leader = self._get_node_to_follow(cluster), cluster.leader
+
+            # FIXME: with mode offline called from DCS exception handler and handle_long_action_in_progress
+            # there could be an async action already running, calling follow from here will lead
+            # to racy state handler state updates.
+            if mode_control['async']:
+                self._async_executor.schedule('starting after demotion')
+                self._async_executor.run_async(self.state_handler.follow, (node_to_follow,))
+            else:
+                if self.state_handler.rewind_needed_and_possible(leader):
+                    return False  # do not start postgres, but run pg_rewind on the next iteration
+                self.state_handler.follow(node_to_follow)
+
+    def _background_keepalive_context(self, wait_for_safepoint=True):
+        if self.watchdog.is_running:
+            safe_event = self.state_handler.stop_safepoint_reached if wait_for_safepoint else None
+            return BackgroundKeepaliveSender(self, safe_event)
         else:
-            # Need to become unavailable as soon as possible, so initiate a stop here. However as we can't release
-            # the leader key we don't care about confirming the shutdown quickly and can use a regular stop.
-            self.state_handler.stop(checkpoint=False)
-            self.state_handler.follow(None, None, recovery=True)
+            return null_context()
 
     def should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn):
         if scheduled_at and not self.is_paused():
@@ -604,21 +717,21 @@ class Ha(object):
         else:
             # when we are doing manual failover there is no guaranty that new leader is ahead of any other node
             # node tagged as nofailover can be ahead of the new leader either, but it is always excluded from elections
-            need_rewind = bool(self.cluster.failover) or self.patroni.nofailover
-            if need_rewind:
+            if bool(self.cluster.failover) or self.patroni.nofailover:
+                self.state_handler.trigger_check_diverged_lsn()
                 time.sleep(2)  # Give a time to somebody to take the leader lock
 
             if self.patroni.nofailover:
                 return self.follow('demoting self because I am not allowed to become master',
-                                   'following a different leader because I am not allowed to promote',
-                                   need_rewind=need_rewind)
+                                   'following a different leader because I am not allowed to promote')
             return self.follow('demoting self because i am not the healthiest node',
-                               'following a different leader because i am not the healthiest node',
-                               need_rewind=need_rewind)
+                               'following a different leader because i am not the healthiest node')
 
     def process_healthy_cluster(self):
         if self.has_lock():
             if self.is_paused() and not self.state_handler.is_leader():
+                # Not a master
+                self.keepalive()
                 if self.cluster.failover and self.cluster.failover.candidate == self.state_handler.name:
                     return 'waiting to become master after promote...'
 
@@ -637,14 +750,14 @@ class Ha(object):
                 # Either there is no connection to DCS or someone else acquired the lock
                 logger.error('failed to update leader lock')
                 if self.state_handler.is_leader():
-                    self.demote('offline')
+                    self.demote('immediate-nolock')
                     return 'demoted self because failed to update leader lock in DCS'
                 else:
                     return 'not promoting because failed to update leader lock in DCS'
         else:
             logger.info('does not have lock')
         return self.follow('demoting self because i do not have the lock and i was a leader',
-                           'no action.  i am a secondary and i am following a leader', False)
+                           'no action.  i am a secondary and i am following a leader', refresh=False)
 
     def evaluate_scheduled_restart(self):
         if self._async_executor.busy:  # Restart already in progress
@@ -736,13 +849,13 @@ class Ha(object):
             # leader key (if it belong to us) rather than trying to start postgres once again.
             self.recovering = True
 
-        # No that restart is scheduled we can set timeout for startup, it will get reset
+        # Now that restart is scheduled we can set timeout for startup, it will get reset
         # once async executor runs and main loop notices PostgreSQL as up.
         timeout = restart_data.get('timeout', self.patroni.config['master_start_timeout'])
         self.set_start_timeout(timeout)
 
         # For non async cases we want to wait for restart to complete or timeout before returning.
-        do_restart = functools.partial(self.state_handler.restart, timeout)
+        do_restart = functools.partial(self.state_handler.restart, timeout, self._async_executor.critical_task)
         if self.is_synchronous_mode() and not self.has_lock():
             do_restart = functools.partial(self.while_not_sync_standby, do_restart)
 
@@ -783,15 +896,28 @@ class Ha(object):
         self._async_executor.run_async(self._do_reinitialize, args=(self.cluster, ))
 
     def handle_long_action_in_progress(self):
-        if self.has_lock():
-            if self.update_lock():
+        try:
+            if self.has_lock() and self.update_lock():
                 return 'updated leader lock during ' + self._async_executor.scheduled_action
             else:
-                return 'failed to update leader lock during ' + self._async_executor.scheduled_action
-        elif self.cluster.is_unlocked():
-            return 'not healthy enough for leader race'
-        else:
-            return self._async_executor.scheduled_action + ' in progress'
+                # Don't have lock, make sure we are not starting up a master in the background
+                if self.state_handler.role == 'master':
+                    logger.info("Demoting master during " + self._async_executor.scheduled_action)
+                    if self._async_executor.scheduled_action == 'restart':
+                        # Restart needs a special interlocking cancel because postmaster may be just started in a
+                        # background thread and has not even written a pid file yet.
+                        with self._async_executor.critical_task as task:
+                            if not task.cancel():
+                                self.state_handler.terminate_starting_postmaster(pid=task.result)
+                    self.demote('immediate-nolock')
+                    return 'lost leader lock during ' + self._async_executor.scheduled_action
+        finally:
+            self.keepalive()
+
+        if self.cluster.is_unlocked():
+            logger.info('not healthy enough for leader race')
+
+        return self._async_executor.scheduled_action + ' in progress'
 
     @staticmethod
     def sysid_valid(sysid):
@@ -802,6 +928,7 @@ class Ha(object):
 
     def post_recover(self):
         if not self.state_handler.is_running():
+            self.keepalive()
             if self.has_lock():
                 self.state_handler.set_role('demoted')
                 self.dcs.delete_leader()
@@ -823,7 +950,7 @@ class Ha(object):
         if self.has_lock():
             if not self.update_lock():
                 logger.info("Lost lock while starting up. Demoting self.")
-                self.demote('immediate')
+                self.demote('immediate-nolock')
                 return 'stopped PostgreSQL while starting up because leader key was lost'
 
             timeout = self._start_timeout or self.patroni.config['master_start_timeout']
@@ -853,8 +980,14 @@ class Ha(object):
         Must be called when async_executor is busy or in the main thread."""
         self._start_timeout = value
 
+    def keepalive(self):
+        if not self.keepalive_sent:
+            self.watchdog.keepalive()
+        self.keepalive_sent = True
+
     def _run_cycle(self):
         dcs_failed = False
+        self.keepalive_sent = False
         try:
             self.load_cluster_from_dcs()
 
@@ -886,6 +1019,10 @@ class Ha(object):
 
             # is data directory empty?
             if self.state_handler.data_directory_empty():
+                # PostgreSQL is assumed to not be running if data dir is empty.
+                # TODO: detect the datadir going away (e.g. unmounted ) while PostgreSQL is running
+                self.keepalive()
+
                 # is this instance the leader?
                 if self.has_lock():
                     self.release_leader_key_voluntarily()
@@ -903,6 +1040,8 @@ class Ha(object):
                     sys.exit(1)
 
             if not self.state_handler.is_healthy():
+                # We are not running, so it's safe to send the keepalive
+                self.keepalive()
                 if self.is_paused():
                     if self.has_lock():
                         self.dcs.delete_leader()
@@ -926,6 +1065,8 @@ 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():
                     if not self.state_handler.cb_called:
+                        if not self.state_handler.is_leader():
+                            self.state_handler.trigger_check_diverged_lsn()
                         self.state_handler.call_nowait(ACTION_ON_START)
                     self.state_handler.sync_replication_slots(self.cluster)
         except DCSError:
@@ -940,12 +1081,38 @@ class Ha(object):
         finally:
             if not dcs_failed:
                 self.touch_member()
+            if not self.keepalive_sent:
+                logger.error("End of HA loop reached without sending keepalive")
 
     def run_cycle(self):
         with self._async_executor:
             info = self._run_cycle()
             return (self.is_paused() and 'PAUSE: ' or '') + info
 
+    def start(self):
+        self.watchdog.activate()
+
+    def shutdown(self):
+        if self.is_paused():
+            logger.info('Leader key is not deleted and Postgresql is not stopped due paused state')
+            self.watchdog.disable()
+        else:
+            # FIXME: If stop doesn't reach safepoint quickly enough keepalive is triggered. If shutdown checkpoint
+            # takes longer than ttl, then leader key is lost and replication might not have sent out all xlog.
+            # This might not be the desired behavior of users, as a graceful shutdown of the host can mean lost data.
+            # We probably need to something smarter here.
+            with self._background_keepalive_context(wait_for_safepoint=self.state_handler.is_leader):
+                self.while_not_sync_standby(lambda: self.state_handler.stop(checkpoint=False))
+            if not self.state_handler.is_running():
+                self.dcs.delete_leader()
+                self.watchdog.disable()
+            else:
+                # XXX: what about when Patroni is started as the wrong user that has access to the watchdog device
+                # but cannot shut down PostgreSQL. Root would be the obvious example. Would be nice to not kill the
+                # system due to a bad config.
+                logger.error("PostgreSQL shutdown failed, leader key not removed." +
+                             (" Leaving watchdog running." if self.watchdog.is_running else ""))
+
     def watch(self, timeout):
         cluster = self.cluster
         # watch on leader key changes if the postgres is running and leader is known and current node is not lock owner

patroni/postgresql.py

@@ -1,20 +1,24 @@
 import logging
+import errno
 import os
 import psycopg2
+import psutil
 import re
 import shlex
 import shutil
+import signal
 import subprocess
 import tempfile
 import time
 
 from collections import defaultdict
+from contextlib import contextmanager
 from patroni import call_self
 from patroni.callback_executor import CallbackExecutor
 from patroni.exceptions import PostgresConnectionException, PostgresException
-from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError, polling_loop
+from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError, polling_loop, null_context
 from six import string_types
-from threading import current_thread, Lock
+from threading import current_thread, Lock, Event
 
 logger = logging.getLogger(__name__)
 
@@ -29,6 +33,14 @@ STATE_REJECT = 'rejecting connections'
 STATE_NO_RESPONSE = 'not responding'
 STATE_UNKNOWN = 'unknown'
 
+STOP_SIGNALS = {
+    'smart': signal.SIGTERM,
+    'fast': signal.SIGINT,
+    'immediate': signal.SIGQUIT,
+}
+STOP_POLLING_INTERVAL = 1
+REWIND_STATUS = type('Enum', (), {'INITIAL': 0, 'CHECK': 1, 'NEED': 2, 'NOT_NEED': 3, 'SUCCESS': 4, 'FAILED': 5})
+
 
 def slot_name_from_member_name(member_name):
     """Translate member name to valid PostgreSQL slot name.
@@ -63,20 +75,20 @@ class Postgresql(object):
     #    check_function -- if the new value is not correct must return `!False`
     #    min_version -- major version of PostgreSQL when parameter was introduced
     CMDLINE_OPTIONS = {
-        'listen_addresses': (None, lambda _: False, 9.1),
-        'port': (None, lambda _: False, 9.1),
-        'cluster_name': (None, lambda _: False, 9.5),
-        'wal_level': ('hot_standby', lambda v: v.lower() in ('hot_standby', 'replica', 'logical'), 9.1),
-        'hot_standby': ('on', lambda _: False, 9.1),
-        'max_connections': (100, lambda v: int(v) >= 100, 9.1),
-        'max_wal_senders': (5, lambda v: int(v) >= 5, 9.1),
-        'wal_keep_segments': (8, lambda v: int(v) >= 8, 9.1),
-        'max_prepared_transactions': (0, lambda v: int(v) >= 0, 9.1),
-        'max_locks_per_transaction': (64, lambda v: int(v) >= 64, 9.1),
-        'track_commit_timestamp': ('off', lambda v: parse_bool(v) is not None, 9.5),
-        'max_replication_slots': (5, lambda v: int(v) >= 5, 9.4),
-        'max_worker_processes': (8, lambda v: int(v) >= 8, 9.4),
-        'wal_log_hints': ('on', lambda _: False, 9.4)
+        'listen_addresses': (None, lambda _: False, 90100),
+        'port': (None, lambda _: False, 90100),
+        'cluster_name': (None, lambda _: False, 90500),
+        'wal_level': ('hot_standby', lambda v: v.lower() in ('hot_standby', 'replica', 'logical'), 90100),
+        'hot_standby': ('on', lambda _: False, 90100),
+        'max_connections': (100, lambda v: int(v) >= 100, 90100),
+        'max_wal_senders': (5, lambda v: int(v) >= 5, 90100),
+        'wal_keep_segments': (8, lambda v: int(v) >= 8, 90100),
+        'max_prepared_transactions': (0, lambda v: int(v) >= 0, 90100),
+        'max_locks_per_transaction': (64, lambda v: int(v) >= 64, 90100),
+        'track_commit_timestamp': ('off', lambda v: parse_bool(v) is not None, 90500),
+        'max_replication_slots': (5, lambda v: int(v) >= 5, 90400),
+        'max_worker_processes': (8, lambda v: int(v) >= 8, 90400),
+        'wal_log_hints': ('on', lambda _: False, 90400)
     }
 
     def __init__(self, config):
@@ -96,10 +108,9 @@ class Postgresql(object):
 
         self._connect_address = config.get('connect_address')
         self._superuser = config['authentication'].get('superuser', {})
-        self._replication = config['authentication']['replication']
         self.resolve_connection_addresses()
 
-        self._need_rewind = False
+        self._rewind_state = REWIND_STATUS.INITIAL
         self._use_slots = config.get('use_slots', True)
         self._schedule_load_slots = self.use_slots
 
@@ -135,6 +146,12 @@ class Postgresql(object):
 
         self._state_entry_timestamp = None
 
+        # This event is set to true when no backends are running. Could be set in parallel by
+        # multiple processes, like when demote is racing with async restart. Needs to be cleared
+        # before invoking stop if wait for this event is desired.
+        self.stop_safepoint_reached = Event()
+        self.stop_safepoint_reached.set()
+
         if self.is_running():
             self.set_state('running')
             self.set_role('master' if self.is_leader() else 'replica')
@@ -151,12 +168,28 @@ class Postgresql(object):
 
     @property
     def use_slots(self):
-        return self._use_slots and self._major_version >= 9.4
+        return self._use_slots and self._major_version >= 90400
+
+    @property
+    def _replication(self):
+        return self.config['authentication']['replication']
 
     @property
     def callback(self):
         return self.config.get('callbacks') or {}
 
+    @staticmethod
+    def _wal_name(version):
+        return 'wal' if version >= 100000 else 'xlog'
+
+    @property
+    def wal_name(self):
+        return self._wal_name(self._major_version)
+
+    @property
+    def lsn_name(self):
+        return 'lsn' if self._major_version >= 100000 else 'location'
+
     def _version_file_exists(self):
         return not self.data_directory_empty() and os.path.isfile(self._version_file)
 
@@ -164,10 +197,10 @@ class Postgresql(object):
         if self._version_file_exists():
             try:
                 with open(self._version_file) as f:
-                    return float(f.read())
+                    return self.postgres_major_version_to_int(f.read().strip())
             except Exception:
                 logger.exception('Failed to read PG_VERSION from %s', self._data_dir)
-        return 0.0
+        return 0
 
     def get_server_parameters(self, config):
         parameters = config['parameters'].copy()
@@ -175,13 +208,16 @@ class Postgresql(object):
         parameters.update({'cluster_name': self.scope, 'listen_addresses': listen_addresses, 'port': port})
         if config.get('synchronous_mode', False):
             if self._synchronous_standby_names is None:
-                parameters.pop('synchronous_standby_names', None)
+                if config.get('synchronous_mode_strict', False):
+                    parameters['synchronous_standby_names'] = '*'
+                else:
+                    parameters.pop('synchronous_standby_names', None)
             else:
                 parameters['synchronous_standby_names'] = self._synchronous_standby_names
-        if self._major_version >= 9.6 and parameters['wal_level'] == 'hot_standby':
+        if self._major_version >= 90600 and parameters['wal_level'] == 'hot_standby':
             parameters['wal_level'] = 'replica'
         return {k: v for k, v in parameters.items() if not self._major_version or
-                self._major_version >= self.CMDLINE_OPTIONS.get(k, (0, 1, 9.1))[2]}
+                self._major_version >= self.CMDLINE_OPTIONS.get(k, (0, 1, 90100))[2]}
 
     def resolve_connection_addresses(self):
         self._local_address = self.get_local_address()
@@ -198,14 +234,6 @@ class Postgresql(object):
         :returns: `!True` when return_code == 0, otherwise `!False`"""
 
         pg_ctl = [self._pgcommand('pg_ctl'), cmd]
-        if cmd == 'stop':
-            pg_ctl += ['-w']
-            timeout = self.config.get('pg_ctl_timeout')
-            if timeout:
-                try:
-                    pg_ctl += ['-t', str(int(timeout))]
-                except Exception:
-                    logger.error('Bad value of pg_ctl_timeout: %s', timeout)
         return subprocess.call(pg_ctl + ['-D', self._data_dir] + list(args), **kwargs) == 0
 
     def pg_isready(self):
@@ -229,6 +257,7 @@ class Postgresql(object):
         return return_codes.get(ret, STATE_UNKNOWN)
 
     def reload_config(self, config):
+        self._superuser = config['authentication'].get('superuser', {})
         server_parameters = self.get_server_parameters(config)
 
         listen_address_changed = pending_reload = pending_restart = False
@@ -293,6 +322,11 @@ class Postgresql(object):
     def pending_restart(self):
         return self._pending_restart
 
+    @staticmethod
+    def configuration_allows_rewind(data):
+        return data.get('Current wal_log_hints setting', 'off') == 'on' \
+            or data.get('Data page checksum version', '0') != '0'
+
     @property
     def can_rewind(self):
         """ check if pg_rewind executable is there and that pg_controldata indicates
@@ -309,9 +343,7 @@ class Postgresql(object):
                 return False
         except OSError:
             return False
-        # check if the cluster's configuration permits pg_rewind
-        data = self.controldata()
-        return data.get('wal_log_hints setting', 'off') == 'on' or data.get('Data page checksum version', '0') != '0'
+        return self.configuration_allows_rewind(self.controldata())
 
     @property
     def sysid(self):
@@ -578,8 +610,9 @@ class Postgresql(object):
 
     def is_running(self):
         if not (self._version_file_exists() and os.path.isfile(self._postmaster_pid)):
+            # XXX: This is dangerous in case somebody deletes the data directory while PostgreSQL is still running.
             return False
-        return self.is_pid_running(self.read_pid_file().get('pid', 0))
+        return self.is_pid_running(self.get_pid())
 
     def read_pid_file(self):
         """Reads and parses postmaster.pid from the data directory
@@ -593,10 +626,21 @@ class Postgresql(object):
         except IOError:
             return {}
 
+    def get_pid(self):
+        """Fetches pid value from postmaster.pid using read_pid_file
+
+        :returns pid if successful, 0 if pid file is not present"""
+        # TODO: figure out what to do on permission errors
+        pid = self.read_pid_file().get('pid', 0)
+        try:
+            return int(pid)
+        except ValueError:
+            logger.warning("Garbage pid in postmaster.pid: {0!r}".format(pid))
+            return 0
+
     @staticmethod
     def is_pid_running(pid):
         try:
-            pid = int(pid)
             if pid < 0:
                 pid = -pid
             return pid > 0 and pid != os.getpid() and pid != os.getppid() and (os.kill(pid, 0) or True)
@@ -672,7 +716,7 @@ class Postgresql(object):
         logger.warning("Timed out waiting for PostgreSQL to start")
         return False
 
-    def start(self, timeout=None, block_callbacks=False):
+    def start(self, timeout=None, block_callbacks=False, task=None):
         """Start PostgreSQL
 
         Waits for postmaster to open ports or terminate so pg_isready can be used to check startup completion
@@ -716,11 +760,21 @@ class Postgresql(object):
         # of init process to take care about postmaster.
         # In order to make everything portable we can't use fork&exec approach here, so  we will call
         # ourselves and pass list of arguments which must be used to start postgres.
-        proc = call_self(['pg_ctl_start', self._pgcommand('postgres'), '-D', self._data_dir] + options, close_fds=True,
-                         preexec_fn=os.setsid, stdout=subprocess.PIPE, env={'PATH': os.environ.get('PATH')})
-        pid = int(proc.stdout.readline().strip())
-        proc.wait()
-        logger.info('postmaster pid=%s', pid)
+        with task or null_context():
+            if task and task.is_cancelled:
+                logger.info("PostgreSQL start cancelled.")
+                return False
+
+            start_initiated = time.time()
+            proc = call_self(['pg_ctl_start', self._pgcommand('postgres'), '-D', self._data_dir] + options,
+                             close_fds=True, preexec_fn=os.setsid, stdout=subprocess.PIPE,
+                             env={p: os.environ[p] for p in ('PATH', 'LC_ALL', 'LANG') if p in os.environ})
+            pid = int(proc.stdout.readline().strip())
+            proc.wait()
+            logger.info('postmaster pid=%s', pid)
+
+            if task:
+                task.complete(pid)
 
         start_timeout = timeout
         if not start_timeout:
@@ -747,24 +801,35 @@ class Postgresql(object):
         for p in ['connect_timeout', 'options']:
             connect_kwargs.pop(p, None)
         try:
-            with psycopg2.connect(**connect_kwargs) as conn:
-                conn.autocommit = True
-                with conn.cursor() as cur:
-                    cur.execute("SET statement_timeout = 0")
-                    if check_not_is_in_recovery:
-                        cur.execute('SELECT pg_is_in_recovery()')
-                        if cur.fetchone()[0]:
-                            return 'is_in_recovery=true'
-                    return cur.execute('CHECKPOINT')
+            with self._get_connection_cursor(**connect_kwargs) as cur:
+                cur.execute("SET statement_timeout = 0")
+                if check_not_is_in_recovery:
+                    cur.execute('SELECT pg_is_in_recovery()')
+                    if cur.fetchone()[0]:
+                        return 'is_in_recovery=true'
+                return cur.execute('CHECKPOINT')
         except psycopg2.Error:
             logging.exception('Exception during CHECKPOINT')
             return 'not accessible or not healty'
 
     def stop(self, mode='fast', block_callbacks=False, checkpoint=True):
-        if not self.is_running():
+        success, pg_signaled = self._do_stop(mode, block_callbacks, checkpoint)
+        if success:
+            self.stop_safepoint_reached.set()  # In case we exited early. Setting twice is not a problem.
+            # block_callbacks is used during restart to avoid
+            # running start/stop callbacks in addition to restart ones
             if not block_callbacks:
                 self.set_state('stopped')
-            return True
+                if pg_signaled:
+                    self.call_nowait(ACTION_ON_STOP)
+        else:
+            logger.warning('pg_ctl stop failed')
+            self.set_state('stop failed')
+        return success
+
+    def _do_stop(self, mode, block_callbacks, checkpoint):
+        if not self.is_running():
+            return True, False
 
         if checkpoint and not self.is_starting():
             self.checkpoint()
@@ -772,16 +837,87 @@ class Postgresql(object):
         if not block_callbacks:
             self.set_state('stopping')
 
-        ret = self.pg_ctl('stop', '-m', mode)
-        # block_callbacks is used during restart to avoid
-        # running start/stop callbacks in addition to restart ones
-        if not ret:
-            logger.warning('pg_ctl stop failed')
-            self.set_state('stop failed')
-        elif not block_callbacks:
-            self.set_state('stopped')
-            self.call_nowait(ACTION_ON_STOP)
-        return ret
+        # Send signal to postmaster to stop
+        pid, result = self._signal_postmaster_stop(mode)
+        if result is not None:
+            return result, True
+
+        # We can skip safepoint detection if nobody is waiting for it.
+        if not self.stop_safepoint_reached.is_set():
+            # Wait for our connection to terminate so we can be sure that no new connections are being initiated
+            self._wait_for_connection_close(pid)
+            self._wait_for_user_backends_to_close(pid)
+            self.stop_safepoint_reached.set()
+
+        self._wait_for_postmaster_stop(pid)
+
+        return True, True
+
+    def _wait_for_postmaster_stop(self, pid):
+        # This wait loop differs subtly from pg_ctl as we check for both the pid file going
+        # away and if the pid is running. This seems safer.
+        while pid == self.get_pid() and self.is_pid_running(pid):
+            time.sleep(STOP_POLLING_INTERVAL)
+
+    def _signal_postmaster_stop(self, mode):
+        pid = self.get_pid()
+        if pid == 0:
+            return None, True
+        elif pid < 0:
+            logger.warning("Cannot stop server; single-user server is running (PID: {0})".format(-pid))
+            return None, False
+        try:
+            os.kill(pid, STOP_SIGNALS[mode])
+        except OSError as e:
+            if e.errno == errno.ESRCH:
+                return None, True
+            else:
+                logger.warning("Could not send stop signal to PostgreSQL (error: {0})".format(e.errno))
+                return None, False
+        return pid, None
+
+    def terminate_starting_postmaster(self, pid):
+        """Terminates a postmaster that has not yet opened ports or possibly even written a pid file. Blocks
+        until the process goes away."""
+        try:
+            os.kill(pid, STOP_SIGNALS['immediate'])
+        except OSError as e:
+            if e.errno == errno.ESRCH:
+                return
+            logger.warning("Could not send stop signal to PostgreSQL (error: {0})".format(e.errno))
+
+        while self.is_pid_running(pid):
+            time.sleep(STOP_POLLING_INTERVAL)
+
+    def _wait_for_connection_close(self, pid):
+        try:
+            with self.connection().cursor() as cur:
+                while True:  # Need a timeout here?
+                    if pid == self.get_pid() and self.is_pid_running(pid):
+                        cur.execute("SELECT 1")
+                        time.sleep(STOP_POLLING_INTERVAL)
+                        continue
+                    else:
+                        break
+        except psycopg2.Error:
+            pass
+
+    @staticmethod
+    def _wait_for_user_backends_to_close(postmaster_pid):
+        # These regexps are cross checked against versions PostgreSQL 9.1 .. 9.6
+        aux_proc_re = re.compile("(?:postgres:)( .*:)? (?:""(?:startup|logger|checkpointer|writer|wal writer|"
+                                 "autovacuum launcher|autovacuum worker|stats collector|wal receiver|archiver|"
+                                 "wal sender) process|bgworker: )")
+
+        try:
+            postmaster = psutil.Process(postmaster_pid)
+            user_backends = [p for p in postmaster.children() if not aux_proc_re.match(p.cmdline()[0])]
+            logger.debug("Waiting for user backends {0} to close".format(
+                ",".join(p.cmdline()[0] for p in user_backends)))
+            psutil.wait_procs(user_backends)
+            logger.debug("Backends closed")
+        except psutil.NoSuchProcess:
+            return
 
     def reload(self):
         ret = self.pg_ctl('reload')
@@ -841,7 +977,7 @@ class Postgresql(object):
 
         return self.state == 'running'
 
-    def restart(self, timeout=None):
+    def restart(self, timeout=None, task=None):
         """Restarts PostgreSQL.
 
         When timeout parameter is set the call will block either until PostgreSQL has started, failed to start or
@@ -851,7 +987,7 @@ class Postgresql(object):
         """
         self.set_state('restarting')
         self.__cb_pending = ACTION_ON_RESTART
-        ret = self.stop(block_callbacks=True) and self.start(timeout=timeout, block_callbacks=True)
+        ret = self.stop(block_callbacks=True) and self.start(timeout=timeout, block_callbacks=True, task=task)
         if not ret and not self.is_starting():
             self.set_state('restart failed ({0})'.format(self.state))
         return ret
@@ -878,14 +1014,17 @@ class Postgresql(object):
             f.write('\n{}\n'.format('\n'.join(config)))
 
     def primary_conninfo(self, member):
-        if not (member and member.conn_url):
+        if not (member and member.conn_url) or member.name == self.name:
             return None
         r = member.conn_kwargs(self._replication)
         r.update({'application_name': self.name, 'sslmode': 'prefer', 'sslcompression': '1'})
         keywords = 'user password host port sslmode sslcompression application_name'.split()
         return ' '.join('{0}={{{0}}}'.format(kw) for kw in keywords).format(**r)
 
-    def check_recovery_conf(self, primary_conninfo):
+    def check_recovery_conf(self, member):
+        # TODO: recovery.conf could be stale, would be nice to detect that.
+        primary_conninfo = self.primary_conninfo(member)
+
         if not os.path.isfile(self._recovery_conf):
             return False
 
@@ -906,7 +1045,7 @@ class Postgresql(object):
                 if name not in ('standby_mode', 'recovery_target_timeline', 'primary_conninfo', 'primary_slot_name'):
                     f.write("{0} = '{1}'\n".format(name, value))
 
-    def rewind(self, r):
+    def pg_rewind(self, r):
         # prepare pg_rewind connection
         env = self.write_pgpass(r)
         dsn_attrs = [
@@ -933,145 +1072,217 @@ class Postgresql(object):
         # Don't try to call pg_controldata during backup restore
         if self._version_file_exists() and self.state != 'creating replica':
             try:
-                data = subprocess.check_output([self._pgcommand('pg_controldata'), self._data_dir])
+                data = subprocess.check_output([self._pgcommand('pg_controldata'), self._data_dir],
+                                               env={'LANG': 'C', 'LC_ALL': 'C', 'PATH': os.environ['PATH']})
                 if data:
                     data = data.decode('utf-8').splitlines()
-                    result = {l.split(':')[0].replace('Current ', '', 1): l.split(':')[1].strip() for l in data if l}
+                    result = {l.split(':', 1)[0]: l.split(':', 1)[1].strip() for l in data if l}
             except subprocess.CalledProcessError:
                 logger.exception("Error when calling pg_controldata")
         return result
 
-    def read_postmaster_opts(self):
-        """ returns the list of option names/values from postgres.opts, Empty dict if read failed or no file """
-        result = {}
-        try:
-            with open(os.path.join(self._data_dir, "postmaster.opts")) as f:
-                data = f.read()
-                opts = [opt.strip('"\n') for opt in data.split(' "')]
-                for opt in opts:
-                    if '=' in opt and opt.startswith('--'):
-                        name, val = opt.split('=', 1)
-                        name = name.strip('-')
-                        result[name] = val
-        except IOError:
-            logger.exception('Error when reading postmaster.opts')
-        return result
-
-    def single_user_mode(self, command=None, options=None):
-        """ run a given command in a single-user mode. If the command is empty - then just start and stop """
-        cmd = [self._pgcommand('postgres'), '--single', '-D', self._data_dir]
-        for opt, val in sorted((options or {}).items()):
-            cmd.extend(['-c', '{0}={1}'.format(opt, val)])
-        # need a database name to connect
-        cmd.append(self._database)
-        p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=open(os.devnull, 'w'), stderr=subprocess.STDOUT)
-        if p:
-            if command:
-                p.communicate('{0}\n'.format(command))
-            p.stdin.close()
-            return p.wait()
-        return 1
-
-    def cleanup_archive_status(self):
-        status_dir = os.path.join(self._data_dir, 'pg_xlog', 'archive_status')
-        try:
-            for f in os.listdir(status_dir):
-                path = os.path.join(status_dir, f)
-                try:
-                    if os.path.islink(path):
-                        os.unlink(path)
-                    elif os.path.isfile(path):
-                        os.remove(path)
-                except OSError:
-                    logger.exception("Unable to remove %s", path)
-        except OSError:
-            logger.exception("Unable to list %s", status_dir)
-
     @property
     def need_rewind(self):
-        return self._need_rewind
+        return self._rewind_state in (REWIND_STATUS.CHECK, REWIND_STATUS.NEED)
 
-    def follow(self, member, leader, recovery=False, async_executor=None, need_rewind=None, timeout=None):
-        if need_rewind is not None:
-            self._need_rewind = need_rewind
+    @staticmethod
+    @contextmanager
+    def _get_connection_cursor(**kwargs):
+        with psycopg2.connect(**kwargs) as conn:
+            conn.autocommit = True
+            with conn.cursor() as cur:
+                yield cur
 
-        primary_conninfo = self.primary_conninfo(member)
+    @contextmanager
+    def _get_replication_connection_cursor(self, host='localhost', port=5432, **kwargs):
+        with self._get_connection_cursor(host=host, port=int(port), database=self._database, replication=1,
+                                         user=self._replication['username'], password=self._replication['password'],
+                                         connect_timeout=3, options='-c statement_timeout=2000') as cur:
+            yield cur
 
-        if self.check_recovery_conf(primary_conninfo) and not recovery:
-            return True
+    def check_leader_is_not_in_recovery(self, **kwargs):
+        try:
+            with self._get_connection_cursor(connect_timeout=3, options='-c statement_timeout=2000', **kwargs) as cur:
+                cur.execute('SELECT pg_is_in_recovery()')
+                if not cur.fetchone()[0]:
+                    return True
+                logger.info('Leader is still in_recovery and therefore can\'t be used for rewind')
+        except Exception:
+            return logger.exception('Exception when working with leader')
 
-        if async_executor:
-            async_executor.schedule('changing primary_conninfo and restarting')
-            async_executor.run_async(self._do_follow, (primary_conninfo, leader, recovery, timeout))
+    def _get_local_timeline_lsn(self):
+        timeline = lsn = None
+        if self.is_running():  # if postgres is running - get timeline and lsn from replication connection
+            try:
+                with self._get_replication_connection_cursor(**self._local_address) as cur:
+                    cur.execute('IDENTIFY_SYSTEM')
+                    timeline, lsn = cur.fetchone()[1:3]
+            except Exception:
+                logger.exception('Can not fetch local timeline and lsn from replication connection')
+        else:  # otherwise analyze pg_controldata output
+            data = self.controldata()
+            try:
+                if data.get('Database cluster state') == 'shut down':
+                    lsn = data.get('Latest checkpoint location')
+                    timeline = int(data.get("Latest checkpoint's TimeLineID"))
+                elif data.get('Database cluster state') == 'shut down in recovery':
+                    lsn = data.get('Minimum recovery ending location')
+                    timeline = int(data.get("Min recovery ending loc's timeline"))
+            except (TypeError, ValueError):
+                logger.exception('Failed to get local timeline and lsn from pg_controldata output')
+        logger.info('Local timeline=%s lsn=%s', timeline, lsn)
+        return timeline, lsn
+
+    def _check_timeline_and_lsn(self, leader):
+        local_timeline, local_lsn = self._get_local_timeline_lsn()
+        if local_timeline is None or local_lsn is None:
+            return
+
+        if not self.check_leader_is_not_in_recovery(**leader.conn_kwargs(self._superuser)):
+            return
+
+        history = need_rewind = None
+        try:
+            with self._get_replication_connection_cursor(**leader.conn_kwargs()) as cur:
+                cur.execute('IDENTIFY_SYSTEM')
+                master_timeline = cur.fetchone()[1]
+                logger.info('master_timeline=%s', master_timeline)
+                if local_timeline > master_timeline:  # Not always supported by pg_rewind
+                    need_rewind = True
+                elif master_timeline > 1:
+                    cur.execute('TIMELINE_HISTORY %s', (master_timeline,))
+                    history = bytes(cur.fetchone()[1]).decode('utf-8')
+                    logger.info('master: history=%s', history)
+                else:  # local_timeline == master_timeline == 1
+                    need_rewind = False
+        except Exception:
+            return logger.exception('Exception when working with master via replication connection')
+
+        if history is not None:
+            def parse_lsn(lsn):
+                t = lsn.split('/')
+                return int(t[0], 16) * 0x100000000 + int(t[1], 16)
+
+            for line in history.split('\n'):
+                line = line.strip().split('\t')
+                if len(line) == 3:
+                    try:
+                        timeline = int(line[0])
+                        if timeline == local_timeline:
+                            try:
+                                need_rewind = parse_lsn(local_lsn) >= parse_lsn(line[1])
+                            except ValueError:
+                                logger.exception('Exception when parsing lsn')
+                            break
+                        elif timeline > local_timeline:
+                            break
+                    except ValueError:
+                        continue
+
+        self._rewind_state = need_rewind and REWIND_STATUS.NEED or REWIND_STATUS.NOT_NEED
+
+    def rewind(self, leader):
+        if self.is_running() and not self.stop(checkpoint=False):
+            return logger.warning('Can not run pg_rewind because postgres is still running')
+
+        # prepare pg_rewind connection
+        r = leader.conn_kwargs(self._superuser)
+
+        # first make sure that we are really trying to rewind
+        # from the master and run a checkpoint on it in order to
+        # make it store the new timeline ([email protected])
+        leader_status = self.checkpoint(r)
+        if leader_status:
+            return logger.warning('Can not use %s for rewind: %s', leader.name, leader_status)
+
+        if self.pg_rewind(r):
+            self._rewind_state = REWIND_STATUS.SUCCESS
+        elif not self.check_leader_is_not_in_recovery(**r):
+            logger.warning('Failed to rewind because master %s become unreachable', leader.name)
         else:
-            return self._do_follow(primary_conninfo, leader, recovery, timeout)
+            logger.error('Failed to rewind from healty master: %s', leader.name)
 
-    def _do_follow(self, primary_conninfo, leader, recovery=False, timeout=None):
+            if self.config.get('remove_data_directory_on_rewind_failure', False):
+                logger.warning('remove_data_directory_on_rewind_failure is set. removing...')
+                self.remove_data_directory()
+                self._rewind_state = REWIND_STATUS.INITIAL
+            else:
+                self._rewind_state = REWIND_STATUS.FAILED
+        return False
+
+    def trigger_check_diverged_lsn(self):
+        if self.can_rewind and self._rewind_state != REWIND_STATUS.NEED:
+            self._rewind_state = REWIND_STATUS.CHECK
+
+    def rewind_needed_and_possible(self, leader):
+        if leader and leader.name != self.name and leader.conn_url and self._rewind_state == REWIND_STATUS.CHECK:
+            self._check_timeline_and_lsn(leader)
+        return leader and leader.conn_url and self._rewind_state == REWIND_STATUS.NEED
+
+    @property
+    def rewind_executed(self):
+        return self._rewind_state > REWIND_STATUS.NOT_NEED
+
+    def follow(self, member, timeout=None):
+        primary_conninfo = self.primary_conninfo(member)
         change_role = self.role in ('master', 'demoted')
 
-        if leader and leader.name == self.name:
-            primary_conninfo = None
-            self._need_rewind = False
-            if self.is_running():
-                return
-        elif change_role:
-            self._need_rewind = True
-
-        if self._need_rewind and not self.can_rewind:
-            logger.warning("Data directory may be out of sync master, rewind may be needed.")
-
-        if self._need_rewind and leader and leader.conn_url and self.can_rewind:
-            logger.info("rewind flag is set")
-
-            if self.is_running() and not self.stop(checkpoint=False):
-                return logger.warning('Can not run pg_rewind because postgres is still running')
-
-            # prepare pg_rewind connection
-            r = leader.conn_kwargs(self._superuser)
-
-            # first make sure that we are really trying to rewind
-            # from the master and run a checkpoint on a t in order to
-            # make it store the new timeline ([email protected])
-            leader_status = self.checkpoint(r)
-            if leader_status:
-                return logger.warning('Can not use %s for rewind: %s', leader.name, leader_status)
-
-            # at present, pg_rewind only runs when the cluster is shut down cleanly
-            # and not shutdown in recovery. We have to remove the recovery.conf if present
-            # and start/shutdown in a single user mode to emulate this.
-            # XXX: if recovery.conf is linked, it will be written anew as a normal file.
-            if os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf):
-                os.unlink(self._recovery_conf)
-
-            # Archived segments might be useful to pg_rewind,
-            # clean the flags that tell we should remove them.
-            self.cleanup_archive_status()
-
-            # Start in a single user mode and stop to produce a clean shutdown
-            opts = self.read_postmaster_opts()
-            opts.update({'archive_mode': 'on', 'archive_command': 'false'})
-            self.single_user_mode(options=opts)
-
-            if self.rewind(r) or not self.config.get('remove_data_directory_on_rewind_failure', False):
-                self.write_recovery_conf(primary_conninfo)
-                self.start()
-            else:
-                logger.error('unable to rewind the former master')
-                self.remove_data_directory()
-            self._need_rewind = False
+        self.write_recovery_conf(primary_conninfo)
+        if self.is_running():
+            self.restart()
         else:
-            self.write_recovery_conf(primary_conninfo)
-            if recovery:
-                self.start(timeout=timeout)
-            else:
-                self.restart()
-            self.set_role('replica')
+            self.start(timeout=timeout)
+        self.set_role('replica')
 
         if change_role:
             # TODO: postpone this until start completes, or maybe do even earlier
             self.call_nowait(ACTION_ON_ROLE_CHANGE)
         return True
 
+    def _do_rewind(self, leader):
+        logger.info("rewind flag is set")
+
+        if self.is_running() and not self.stop(checkpoint=False):
+            logger.warning('Can not run pg_rewind because postgres is still running')
+            return False
+
+        # prepare pg_rewind connection
+        r = leader.conn_kwargs(self._superuser)
+
+        # first make sure that we are really trying to rewind
+        # from the master and run a checkpoint on a t in order to
+        # make it store the new timeline ([email protected])
+        leader_status = self.checkpoint(r)
+        if leader_status:
+            logger.warning('Can not use %s for rewind: %s', leader.name, leader_status)
+            return False
+
+        # at present, pg_rewind only runs when the cluster is shut down cleanly
+        # and not shutdown in recovery. We have to remove the recovery.conf if present
+        # and start/shutdown in a single user mode to emulate this.
+        # XXX: if recovery.conf is linked, it will be written anew as a normal file.
+        if os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf):
+            os.unlink(self._recovery_conf)
+
+        # Archived segments might be useful to pg_rewind,
+        # clean the flags that tell we should remove them.
+        self.cleanup_archive_status()
+
+        # Start in a single user mode and stop to produce a clean shutdown
+        opts = self.read_postmaster_opts()
+        opts.update({'archive_mode': 'on', 'archive_command': 'false'})
+        self.single_user_mode(options=opts)
+
+        try:
+            if not self.rewind(r):
+                logger.error('unable to rewind the former master')
+                if self.config.get('remove_data_directory_on_rewind_failure', False):
+                    self.remove_data_directory()
+                    return False
+            return True
+        finally:
+            self._need_rewind = False
+
     def save_configuration_files(self):
         """
             copy postgresql.conf to postgresql.conf.backup to be able to retrive configuration files
@@ -1100,8 +1311,8 @@ class Postgresql(object):
         ret = self.pg_ctl('promote')
         if ret:
             self.set_role('master')
-            logger.info("cleared rewind flag after becoming the leader")
-            self._need_rewind = False
+            logger.info("cleared rewind state after becoming the leader")
+            self._rewind_state = REWIND_STATUS.INITIAL
             self.call_nowait(ACTION_ON_ROLE_CHANGE)
         return ret
 
@@ -1122,13 +1333,13 @@ BEGIN
 END;
 $$""".format(name, ' '.join(options)), name, password, password)
 
-    def xlog_position(self, retry=True):
+    def wal_position(self, retry=True):
         stmt = """SELECT CASE WHEN pg_is_in_recovery()
-                              THEN GREATEST(pg_xlog_location_diff(COALESCE(pg_last_xlog_receive_location(), '0/0'),
+                              THEN GREATEST(pg_{0}_{1}_diff(COALESCE(pg_last_{0}_receive_{1}(), '0/0'),
                                                                   '0/0')::bigint,
-                                            pg_xlog_location_diff(pg_last_xlog_replay_location(), '0/0')::bigint)
-                              ELSE pg_xlog_location_diff(pg_current_xlog_location(), '0/0')::bigint
-                          END"""
+                                            pg_{0}_{1}_diff(pg_last_{0}_replay_{1}(), '0/0')::bigint)
+                              ELSE pg_{0}_{1}_diff(pg_current_{0}_{1}(), '0/0')::bigint
+                          END""".format(self.wal_name, self.lsn_name)
 
         # This method could be called from different threads (simultaneously with some other `_query` calls).
         # If it is called not from main thread we will create a new cursor to execute statement.
@@ -1200,7 +1411,7 @@ $$""".format(name, ' '.join(options)), name, password, password)
                 self._schedule_load_slots = True
 
     def last_operation(self):
-        return str(self.xlog_position())
+        return str(self.wal_position())
 
     def clone(self, clone_member):
         """
@@ -1254,6 +1465,11 @@ $$""".format(name, ' '.join(options)), name, password, password)
             self.move_data_directory()
 
     def basebackup(self, conn_url, env):
+        # save environ to restore it later
+        old_env = os.environ.copy()
+        os.environ.clear()
+        os.environ.update(env)
+
         # creates a replica data dir using pg_basebackup.
         # this is the default, built-in create_replica_method
         # tries twice, then returns failure (as 1)
@@ -1265,13 +1481,19 @@ $$""".format(name, ' '.join(options)), name, password, password)
                 self.remove_data_directory()
 
             try:
+                version = 0
+                with psycopg2.connect(conn_url + '?replication=1') as c:
+                    version = c.server_version
+
                 ret = subprocess.call([self._pgcommand('pg_basebackup'), '--pgdata=' + self._data_dir,
-                                       '--xlog-method=stream', "--dbname=" + conn_url], env=env)
+                                       '--{0}-method=stream'.format(self._wal_name(version)), '--dbname=' + conn_url])
                 if ret == 0:
                     break
                 else:
                     logger.error('Error when fetching backup: pg_basebackup exited with code=%s', ret)
 
+            except psycopg2.Error:
+                logger.error('Can not connect to %s', conn_url)
             except Exception as e:
                 logger.error('Error when fetching backup with pg_basebackup: %s', e)
 
@@ -1279,6 +1501,10 @@ $$""".format(name, ' '.join(options)), name, password, password)
                 logger.warning('Trying again in 5 seconds')
                 time.sleep(5)
 
+        # restore environ
+        os.environ.clear()
+        os.environ.update(old_env)
+
         return ret
 
     def pick_synchronous_standby(self, cluster):
@@ -1297,7 +1523,7 @@ $$""".format(name, ' '.join(options)), name, password, password)
         for app_name, state, sync_state in self.query(
                 """SELECT application_name, state, sync_state
                      FROM pg_stat_replication
-                    ORDER BY flush_location DESC"""):
+                    ORDER BY flush_{0} DESC""".format(self.lsn_name)):
             member = members.get(app_name)
             if state != 'streaming' or not member or member.tags.get('nosync', False):
                 continue
@@ -1357,3 +1583,13 @@ $$""".format(name, ' '.join(options)), name, password, password)
         except ValueError:
             raise Exception("Invalid PostgreSQL version: {0}".format(pg_version))
         return result
+
+    @staticmethod
+    def postgres_major_version_to_int(pg_version):
+        """
+        >>> Postgresql.postgres_major_version_to_int('10')
+        100000
+        >>> Postgresql.postgres_major_version_to_int('9.6')
+        90600
+        """
+        return Postgresql.postgres_version_to_int(pg_version + '.0')

patroni/scripts/wale_restore.py

@@ -23,19 +23,29 @@
 # currently also requires that you configure the restore_command to use wal_e, example:
 #       recovery_conf:
 #               restore_command: envdir /etc/wal-e.d/env wal-e wal-fetch "%f" "%p" -p 1
-
-from collections import namedtuple
+import argparse
+import csv
 import logging
 import os
 import psycopg2
 import subprocess
 import sys
 import time
-import argparse
+
+from collections import namedtuple
 
 logger = logging.getLogger(__name__)
 
 RETRY_SLEEP_INTERVAL = 1
+si_prefixes = ['K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y']
+
+
+# Meaningful names to the exit codes used by WALERestore
+ExitCode = type('Enum', (), {
+    'SUCCESS': 0,  #: Succeeded
+    'RETRY_LATER': 1,  #: External issue, retry later
+    'FAIL': 2  #: Don't try again unless configuration changes
+})
 
 
 # We need to know the current PG version in order to figure out the correct WAL directory name
@@ -50,66 +60,141 @@ def get_major_version(data_dir):
     return 0.0
 
 
+def repr_size(n_bytes):
+    """
+    >>> repr_size(1000)
+    '1000 Bytes'
+    >>> repr_size(8257332324597)
+    '7.5 TiB'
+    """
+    if n_bytes < 1024:
+        return '{0} Bytes'.format(n_bytes)
+    i = -1
+    while n_bytes > 1023:
+        n_bytes /= 1024.0
+        i += 1
+    return '{0} {1}iB'.format(round(n_bytes, 1), si_prefixes[i])
+
+
+def size_as_bytes(size_, prefix):
+    """
+    >>> size_as_bytes(7.5, 'T')
+    8246337208320
+    """
+    prefix = prefix.upper()
+
+    assert prefix in si_prefixes
+
+    exponent = si_prefixes.index(prefix) + 1
+
+    return int(size_ * (1024.0 ** exponent))
+
+
+WALEConfig = namedtuple(
+    'WALEConfig',
+    [
+        'env_dir',
+        'threshold_mb',
+        'threshold_pct',
+        'cmd',
+    ]
+)
+
+
 class WALERestore(object):
-    def __init__(self, scope, datadir, connstring, env_dir, threshold_mb, threshold_pct, use_iam, no_master, retries):
+    def __init__(self, scope, datadir, connstring, env_dir, threshold_mb,
+                 threshold_pct, use_iam, no_master, retries):
         self.scope = scope
         self.master_connection = connstring
         self.data_dir = datadir
-        self.wal_e = namedtuple('wale', 'dir,threshold_mb,threshold_pct,iam_string,cmd')
-        self.wal_e.dir = env_dir
-        self.wal_e.threshold_mb = threshold_mb
-        self.wal_e.threshold_pct = threshold_pct
-        self.wal_e.iam_string = ' --aws-instance-profile ' if use_iam == 1 else ''
         self.no_master = no_master
-        self.wal_e.cmd = 'envdir {0} wal-e {1} '.format(self.wal_e.dir, self.wal_e.iam_string)
-        self.init_error = (not os.path.exists(self.wal_e.dir))
+
+        wale_cmd = [
+            'envdir',
+            env_dir,
+            'wal-e',
+        ]
+
+        if use_iam == 1:
+            wale_cmd += ['--aws-instance-profile']
+
+        self.wal_e = WALEConfig(
+            env_dir=env_dir,
+            threshold_mb=threshold_mb,
+            threshold_pct=threshold_pct,
+            cmd=wale_cmd,
+        )
+
+        self.init_error = (not os.path.exists(self.wal_e.env_dir))
         self.retries = retries
 
     def run(self):
-        """ creates a new replica using WAL-E """
-        if not self.init_error:
-            try:
-                ret = self.should_use_s3_to_create_replica()
-                if ret:
-                    return self.create_replica_with_s3()
-                elif ret is None:  # caught an exception, need to retry
-                    return 1
-            except Exception:
-                logger.exception("Exception when running WAL-E restore")
-        return 2
+        """
+        Creates a new replica using WAL-E
+
+        Returns
+        -------
+        ExitCode
+            0 = Success
+            1 = Error, try again
+            2 = Error, don't try again
+
+        """
+        if self.init_error:
+            logger.error('init error: %r did not exist at initialization time',
+                         self.wal_e.env_dir)
+            return ExitCode.FAIL
+
+        try:
+            should_use_s3 = self.should_use_s3_to_create_replica()
+            if should_use_s3 is None:  # Need to retry
+                return ExitCode.RETRY_LATER
+            elif should_use_s3:
+                return self.create_replica_with_s3()
+            elif not should_use_s3:
+                return ExitCode.FAIL
+        except Exception:
+            logger.exception("Unhandled exception when running WAL-E restore")
+        return ExitCode.FAIL
 
     def should_use_s3_to_create_replica(self):
         """ determine whether it makes sense to use S3 and not pg_basebackup """
 
         threshold_megabytes = self.wal_e.threshold_mb
-        threshold_backup_size_percentage = self.wal_e.threshold_pct
+        threshold_percent = self.wal_e.threshold_pct
 
         try:
-            latest_backup = subprocess.check_output(self.wal_e.cmd.split() + ['backup-list', '--detail', 'LATEST'])
-            # name    last_modified   expanded_size_bytes wal_segment_backup_start    wal_segment_offset_backup_start
-            #                                                   wal_segment_backup_stop wal_segment_offset_backup_stop
-            # base_00000001000000000000007F_00000040  2015-05-18T10:13:25.000Z
-            # 20310671    00000001000000000000007F    00000040
-            # 00000001000000000000007F    00000240
-            backup_strings = latest_backup.decode('utf-8').splitlines() if latest_backup else ()
-            if len(backup_strings) != 2:
+            cmd = self.wal_e.cmd + ['backup-list', '--detail', 'LATEST']
+
+            logger.debug('calling %r', cmd)
+            wale_output = subprocess.check_output(cmd)
+
+            reader = csv.DictReader(wale_output.decode('utf-8').splitlines(),
+                                    dialect='excel-tab')
+            rows = list(reader)
+            if not len(rows):
+                logger.warning('wal-e did not find any backups')
                 return False
 
-            names = backup_strings[0].split()
-            vals = backup_strings[1].split()
-            if (len(names) != len(vals)) or (len(names) != 7):
+            # This check might not add much, it was performed in the previous
+            # version of this code. since the old version rolled CSV parsing the
+            # check may have been part of the CSV parsing.
+            if len(rows) > 1:
+                logger.warning(
+                    'wal-e returned more than one row of backups: %r',
+                    rows)
                 return False
 
-            backup_info = dict(zip(names, vals))
+            backup_info = rows[0]
         except subprocess.CalledProcessError:
             logger.exception("could not query wal-e latest backup")
             return None
 
         try:
-            backup_size = backup_info['expanded_size_bytes']
+            backup_size = int(backup_info['expanded_size_bytes'])
             backup_start_segment = backup_info['wal_segment_backup_start']
             backup_start_offset = backup_info['wal_segment_offset_backup_start']
-        except Exception:
+        except KeyError:
             logger.exception("unable to get some of WALE backup parameters")
             return None
 
@@ -124,24 +209,29 @@ class WALERestore(object):
         # construct the LSN from the segment and offset
         backup_start_lsn = '{0}/{1}'.format(lsn_segment, lsn_offset)
 
-        diff_in_bytes = int(backup_size)
+        diff_in_bytes = backup_size
         attempts_no = 0
         while True:
             if self.master_connection:
                 try:
                     # get the difference in bytes between the current WAL location and the backup start offset
                     with psycopg2.connect(self.master_connection) as con:
+                        if con.server_version >= 100000:
+                            wal_name = 'wal'
+                            lsn_name = 'lsn'
+                        else:
+                            wal_name = 'xlog'
+                            lsn_name = 'location'
                         con.autocommit = True
                         with con.cursor() as cur:
                             cur.execute("""SELECT CASE WHEN pg_is_in_recovery()
                                                        THEN GREATEST(
-                                                                pg_xlog_location_diff(COALESCE(
-                                                                  pg_last_xlog_receive_location(), '0/0'), %s)::bigint,
-                                                                pg_xlog_location_diff(
-                                                                  pg_last_xlog_replay_location(), %s)::bigint)
-                                                       ELSE pg_xlog_location_diff(
-                                                                pg_current_xlog_location(), %s)::bigint
-                                                   END""", (backup_start_lsn, backup_start_lsn, backup_start_lsn))
+                                                                pg_{0}_{1}_diff(COALESCE(
+                                                                  pg_last_{0}_receive_{1}(), '0/0'), %s)::bigint,
+                                                                pg_{0}_{1}_diff(pg_last_{0}_replay_{1}(), %s)::bigint)
+                                                       ELSE pg_{0}_{1}_diff(pg_current_{0}_{1}(), %s)::bigint
+                                                   END""".format(wal_name, lsn_name),
+                                        (backup_start_lsn, backup_start_lsn, backup_start_lsn))
 
                             diff_in_bytes = int(cur.fetchone()[0])
                 except psycopg2.Error:
@@ -163,8 +253,47 @@ class WALERestore(object):
 
         # if the size of the accumulated WAL segments is more than a certan percentage of the backup size
         # or exceeds the pre-determined size - pg_basebackup is chosen instead.
-        return (diff_in_bytes < int(threshold_megabytes) * 1048576) and\
-            (diff_in_bytes < int(backup_size) * float(threshold_backup_size_percentage) / 100)
+        is_size_thresh_ok = diff_in_bytes < int(threshold_megabytes) * 1048576
+        threshold_pct_bytes = backup_size * threshold_percent / 100.0
+        is_percentage_thresh_ok = float(diff_in_bytes) < int(threshold_pct_bytes)
+        are_thresholds_ok = is_size_thresh_ok and is_percentage_thresh_ok
+
+        class Size(object):
+            def __init__(self, n_bytes, prefix=None):
+                self.n_bytes = n_bytes
+                self.prefix = prefix
+
+            def __repr__(self):
+                if self.prefix is not None:
+                    n_bytes = size_as_bytes(self.n_bytes, self.prefix)
+                else:
+                    n_bytes = self.n_bytes
+                return repr_size(n_bytes)
+
+        class HumanContext(object):
+            def __init__(self, items):
+                self.items = items
+
+            def __repr__(self):
+                return ', '.join('{}={!r}'.format(key, value)
+                                 for key, value in self.items)
+
+        human_context = repr(HumanContext([
+            ('threshold_size', Size(threshold_megabytes, 'M')),
+            ('threshold_percent', threshold_percent),
+            ('threshold_percent_size', Size(threshold_pct_bytes)),
+            ('backup_size', Size(backup_size)),
+            ('backup_diff', Size(diff_in_bytes)),
+            ('is_size_thresh_ok', is_size_thresh_ok),
+            ('is_percentage_thresh_ok', is_percentage_thresh_ok),
+        ]))
+
+        if not are_thresholds_ok:
+            logger.info('wal-e backup size diff is over threshold, falling back '
+                        'to other means of restore: %s', human_context)
+        else:
+            logger.info('Thresholds are OK, using wal-e basebackup: %s', human_context)
+        return are_thresholds_ok
 
     def fix_subdirectory_path_if_broken(self, dirname):
         # in case it is a symlink pointing to a non-existing location, remove it and create the actual directory
@@ -187,15 +316,19 @@ class WALERestore(object):
     def create_replica_with_s3(self):
         # if we're set up, restore the replica using fetch latest
         try:
-            ret = subprocess.call(self.wal_e.cmd.split() + ['backup-fetch', '{}'.format(self.data_dir), 'LATEST'])
+            cmd = self.wal_e.cmd + ['backup-fetch',
+                                    '{}'.format(self.data_dir),
+                                    'LATEST']
+            logger.debug('calling: %r', cmd)
+            exit_code = subprocess.call(cmd)
         except Exception as e:
             logger.error('Error when fetching backup with WAL-E: {0}'.format(e))
-            return 1
+            return ExitCode.RETRY_LATER
 
-        if (ret == 0 and not
-           self.fix_subdirectory_path_if_broken('pg_xlog' if get_major_version(self.data_dir) < 10.0 else 'pg_wal')):
-            return 2
-        return ret
+        if (exit_code == 0 and not
+           self.fix_subdirectory_path_if_broken('pg_xlog' if get_major_version(self.data_dir) < 10 else 'pg_wal')):
+            return ExitCode.FAIL
+        return exit_code
 
 
 def main():
@@ -213,6 +346,9 @@ def main():
     parser.add_argument('--no_master', type=int, default=0)
     args = parser.parse_args()
 
+    exit_code = None
+    assert args.retries >= 0
+
     # Retry cloning in a loop. We do separate retries for the master
     # connection attempt inside should_use_s3_to_create_replica,
     # because we need to differentiate between the last attempt and
@@ -223,12 +359,13 @@ def main():
                               env_dir=args.envdir, threshold_mb=args.threshold_megabytes,
                               threshold_pct=args.threshold_backup_size_percentage, use_iam=args.use_iam,
                               no_master=args.no_master, retries=args.retries)
-        ret = restore.run()
-        if ret != 1:  # only WAL-E failures lead to the retry
+        exit_code = restore.run()
+        if not exit_code == ExitCode.RETRY_LATER:  # only WAL-E failures lead to the retry
+            logger.debug('exit_code is %r, not retrying', exit_code)
             break
         time.sleep(RETRY_SLEEP_INTERVAL)
 
-    return ret
+    return exit_code
 
 
 if __name__ == '__main__':

patroni/utils.py

@@ -1,3 +1,4 @@
+import contextlib
 import random
 import time
 import re
@@ -280,3 +281,8 @@ def polling_loop(timeout, interval=1):
         yield iteration
         iteration += 1
         time.sleep(interval)
+
+
+@contextlib.contextmanager
+def null_context():
+    yield

patroni/watchdog/__init__.py

@@ -0,0 +1,2 @@
+from patroni.watchdog.base import WatchdogError, Watchdog
+__all__ = ['WatchdogError', 'Watchdog']

patroni/watchdog/base.py

@@ -0,0 +1,207 @@
+import abc
+import logging
+import platform
+import six
+import sys
+
+from patroni.exceptions import WatchdogError
+
+__all__ = ['WatchdogError', 'Watchdog']
+
+logger = logging.getLogger(__name__)
+
+MODE_REQUIRED = 'required'    # Will not run if a watchdog is not available
+MODE_AUTOMATIC = 'automatic'  # Will use a watchdog if one is available
+MODE_OFF = 'off'              # Will not try to use a watchdog
+
+
+def parse_mode(mode):
+    if mode is False:
+        return MODE_OFF
+    mode = mode.lower()
+    if mode in ['require', 'required']:
+        return MODE_REQUIRED
+    elif mode in ['auto', 'automatic']:
+        return MODE_AUTOMATIC
+    else:
+        if mode not in ['off', 'disable', 'disabled']:
+            logger.warning("Watchdog mode {0} not recognized, disabling watchdog".format(mode))
+        return MODE_OFF
+
+
+class Watchdog(object):
+    """Facade to dynamically manage watchdog implementations and handle config changes."""
+    def __init__(self, config):
+        self.ttl = config['ttl']
+        self.loop_wait = config['loop_wait']
+        self.mode = parse_mode(config['watchdog'].get('mode', 'automatic'))
+        self.driver = config['watchdog'].get('driver')
+        self.config = config
+
+        if self.mode == MODE_OFF:
+            self.impl = NullWatchdog()
+        else:
+            self.impl = self._get_impl()
+            if self.mode == MODE_REQUIRED and isinstance(self.impl, NullWatchdog):
+                logger.error("Configuration requires a watchdog, but watchdog is not supported on this platform.")
+                sys.exit(1)
+
+    def activate(self):
+        """Activates the watchdog device with suitable timeouts. While watchdog is active keepalive needs
+        to be called every time loop_wait expires.
+        """
+        desired_timeout = int(self.ttl // 2)
+        slack = desired_timeout - self.loop_wait
+        if slack < 0:
+            logger.warning('Watchdog not supported because leader TTL {0} is less than 2x loop_wait {1}'
+                           .format(self.ttl, self.loop_wait))
+            self.impl = NullWatchdog()
+
+        try:
+            self.impl.open()
+        except WatchdogError as e:
+            logger.warning("Could not activate %s: %s", self.impl.describe(), e)
+            self.impl = NullWatchdog()
+
+        if self.impl.is_running and not self.impl.can_be_disabled:
+            logger.warning("Watchdog implementation can't be disabled."
+                           " Watchdog will trigger after Patroni is shut down.")
+
+        if self.impl.has_set_timeout():
+            self.impl.set_timeout(desired_timeout)
+
+        # Safety checks for watchdog implementations that don't support configurable timeouts
+        actual_timeout = self.impl.get_timeout()
+        if self.impl.is_running and actual_timeout < self.loop_wait:
+            logger.error('loop_wait of {0} seconds is too long for watchdog {1} second timeout'
+                         .format(self.loop_wait, actual_timeout))
+            if self.impl.can_be_disabled:
+                logger.info('Disabling watchdog due to unsafe timeout.')
+                self.impl.close()
+                self.impl = NullWatchdog()
+
+        if not self.impl.is_running or actual_timeout > desired_timeout:
+            if self.mode == MODE_REQUIRED:
+                logger.error("Configuration requires watchdog, but a safe watchdog timeout {0} could"
+                             " not be configured. Watchdog timeout is {1}.".format(desired_timeout, actual_timeout))
+                sys.exit(1)
+            else:
+                if not isinstance(self.impl, NullWatchdog):
+                    logger.warning("Watchdog timeout {0} seconds does not ensure safe termination within {1} seconds"
+                                   .format(actual_timeout, desired_timeout))
+
+        if self.is_running:
+            logger.info("{0} activated with {1} second timeout, timing slack {2} seconds"
+                        .format(self.impl.describe(), actual_timeout, slack))
+        else:
+            if self.mode == MODE_REQUIRED:
+                logger.error("Configuration requires watchdog, but watchdog could not be activated")
+                sys.exit(1)
+
+    def disable(self):
+        try:
+            if self.impl.is_running and not self.impl.can_be_disabled:
+                # Give sysadmin some extra time to clean stuff up.
+                self.impl.keepalive()
+                logger.warning("Watchdog implementation can't be disabled. System will reboot after "
+                               "{0} seconds when watchdog times out.".format(self.impl.get_timeout()))
+            self.impl.close()
+        except WatchdogError as e:
+            logger.error("Error while disabling watchdog: %s", e)
+
+    def keepalive(self):
+        try:
+            self.impl.keepalive()
+        except WatchdogError as e:
+            logger.error("Error while sending keepalive: %s", e)
+
+    def _get_impl(self):
+        if self.mode not in [MODE_AUTOMATIC, MODE_REQUIRED]:
+            return NullWatchdog()
+
+        if self.driver == 'testing':
+            from patroni.watchdog.linux import TestingWatchdogDevice
+            return TestingWatchdogDevice.from_config(self.config['watchdog'])
+        elif platform.system() == 'Linux':
+            from patroni.watchdog.linux import LinuxWatchdogDevice
+            return LinuxWatchdogDevice.from_config(self.config['watchdog'])
+        else:
+            return NullWatchdog()
+
+    @property
+    def is_running(self):
+        return self.impl.is_running
+
+
+@six.add_metaclass(abc.ABCMeta)
+class WatchdogBase(object):
+    """A watchdog object when opened requires periodic calls to keepalive.
+    When keepalive is not called within a timeout the system will be terminated."""
+
+    @property
+    def is_running(self):
+        """Returns True when watchdog is activated and capable of performing it's task."""
+        return False
+
+    @property
+    def can_be_disabled(self):
+        """Returns True when watchdog will be disabled by calling close(). Some watchdog devices
+        will keep running no matter what once activated. May raise WatchdogError if called without
+        calling open() first."""
+        return True
+
+    @abc.abstractmethod
+    def open(self):
+        """Open watchdog device.
+
+        When watchdog is opened keepalive must be called. Returns nothing on success
+        or raises WatchdogError if the device could not be opened."""
+
+    @abc.abstractmethod
+    def close(self):
+        """Gracefully close watchdog device."""
+
+    @abc.abstractmethod
+    def keepalive(self):
+        """Resets the watchdog timer.
+
+        Watchdog must be open when keepalive is called."""
+
+    @abc.abstractmethod
+    def get_timeout(self):
+        """Returns the current keepalive timeout in effect."""
+
+    @staticmethod
+    def has_set_timeout():
+        """Returns True if setting a timeout is supported."""
+        return False
+
+    def set_timeout(self, timeout):
+        """Set the watchdog timer timeout.
+
+        :param timeout: watchdog timeout in seconds"""
+        raise WatchdogError("Setting timeout is not supported on {0}".format(self.describe()))
+
+    def describe(self):
+        """Human readable name for this device"""
+        return self.__class__.__name__
+
+    @classmethod
+    def from_config(cls, config):
+        return cls()
+
+
+class NullWatchdog(WatchdogBase):
+    """Null implementation when watchdog is not supported."""
+    def open(self):
+        return
+
+    def close(self):
+        return
+
+    def keepalive(self):
+        return
+
+    def get_timeout(self):
+        # A big enough number to not matter
+        return 1000000000

patroni/watchdog/linux.py

@@ -0,0 +1,220 @@
+import collections
+import ctypes
+import fcntl
+import os
+import platform
+from patroni.watchdog.base import WatchdogBase, WatchdogError
+
+# Pythonification of linux/ioctl.h
+IOC_NONE = 0
+IOC_WRITE = 1
+IOC_READ = 2
+
+IOC_NRBITS = 8
+IOC_TYPEBITS = 8
+IOC_SIZEBITS = 14
+IOC_DIRBITS = 2
+
+# Non-generic platform special cases
+machine = platform.machine()
+if machine in ['mips', 'sparc', 'powerpc', 'ppc64']:
+    IOC_SIZEBITS = 13
+    IOC_DIRBITS = 3
+    IOC_NONE, IOC_WRITE, IOC_READ = 1, 2, 4
+elif machine == 'parisc':
+    IOC_WRITE, IOC_READ = 2, 1
+
+IOC_NRSHIFT = 0
+IOC_TYPESHIFT = IOC_NRSHIFT + IOC_NRBITS
+IOC_SIZESHIFT = IOC_TYPESHIFT + IOC_TYPEBITS
+IOC_DIRSHIFT = IOC_SIZESHIFT + IOC_SIZEBITS
+
+
+def IOW(type_, nr, size):
+    return IOC(IOC_WRITE, type_, nr, size)
+
+
+def IOR(type_, nr, size):
+    return IOC(IOC_READ, type_, nr, size)
+
+
+def IOWR(type_, nr, size):
+    return IOC(IOC_READ | IOC_WRITE, type_, nr, size)
+
+
+def IOC(dir_, type_, nr, size):
+    return (dir_ << IOC_DIRSHIFT) \
+        | (ord(type_) << IOC_TYPESHIFT) \
+        | (nr << IOC_NRSHIFT) \
+        | (size << IOC_SIZESHIFT)
+
+
+# Pythonification of linux/watchdog.h
+
+WATCHDOG_IOCTL_BASE = 'W'
+
+
+class watchdog_info(ctypes.Structure):
+    _fields_ = [
+        ('options', ctypes.c_uint32),           # Options the card/driver supports
+        ('firmware_version', ctypes.c_uint32),  # Firmware version of the card
+        ('identity', ctypes.c_uint8 * 32),      # Identity of the board
+    ]
+
+
+struct_watchdog_info_size = ctypes.sizeof(watchdog_info)
+int_size = ctypes.sizeof(ctypes.c_int)
+
+WDIOC_GETSUPPORT = IOR(WATCHDOG_IOCTL_BASE, 0, struct_watchdog_info_size)
+WDIOC_GETSTATUS = IOR(WATCHDOG_IOCTL_BASE, 1, int_size)
+WDIOC_GETBOOTSTATUS = IOR(WATCHDOG_IOCTL_BASE, 2, int_size)
+WDIOC_GETTEMP = IOR(WATCHDOG_IOCTL_BASE, 3, int_size)
+WDIOC_SETOPTIONS = IOR(WATCHDOG_IOCTL_BASE, 4, int_size)
+WDIOC_KEEPALIVE = IOR(WATCHDOG_IOCTL_BASE, 5, int_size)
+WDIOC_SETTIMEOUT = IOWR(WATCHDOG_IOCTL_BASE, 6, int_size)
+WDIOC_GETTIMEOUT = IOR(WATCHDOG_IOCTL_BASE, 7, int_size)
+WDIOC_SETPRETIMEOUT = IOWR(WATCHDOG_IOCTL_BASE, 8, int_size)
+WDIOC_GETPRETIMEOUT = IOR(WATCHDOG_IOCTL_BASE, 9, int_size)
+WDIOC_GETTIMELEFT = IOR(WATCHDOG_IOCTL_BASE, 10, int_size)
+
+
+WDIOF_UNKNOWN = -1  # Unknown flag error
+WDIOS_UNKNOWN = -1  # Unknown status error
+
+WDIOF = {
+    "OVERHEAT": 0x0001,       # Reset due to CPU overheat
+    "FANFAULT": 0x0002,       # Fan failed
+    "EXTERN1": 0x0004,        # External relay 1
+    "EXTERN2": 0x0008,        # External relay 2
+    "POWERUNDER": 0x0010,     # Power bad/power fault
+    "CARDRESET": 0x0020,      # Card previously reset the CPU
+    "POWEROVER": 0x0040,      # Power over voltage
+    "SETTIMEOUT": 0x0080,     # Set timeout (in seconds)
+    "MAGICCLOSE": 0x0100,     # Supports magic close char
+    "PRETIMEOUT": 0x0200,     # Pretimeout (in seconds), get/set
+    "ALARMONLY": 0x0400,      # Watchdog triggers a management or other external alarm not a reboot
+    "KEEPALIVEPING": 0x8000,  # Keep alive ping reply
+}
+
+WDIOS = {
+    "DISABLECARD": 0x0001,    # Turn off the watchdog timer
+    "ENABLECARD": 0x0002,     # Turn on the watchdog timer
+    "TEMPPANIC": 0x0004,      # Kernel panic on temperature trip
+}
+
+# Implementation
+
+
+class WatchdogInfo(collections.namedtuple('WatchdogInfo', 'options,version,identity')):
+    """Watchdog descriptor from the kernel"""
+    def __getattr__(self, name):
+        """Convenience has_XYZ attributes for checking WDIOF bits in options"""
+        if name.startswith('has_') and name[4:] in WDIOF:
+            return bool(self.options & WDIOF[name[4:]])
+
+        raise AttributeError("WatchdogInfo instance has no attribute '{0}'".format(name))
+
+
+class LinuxWatchdogDevice(WatchdogBase):
+    DEFAULT_DEVICE = '/dev/watchdog'
+
+    def __init__(self, device):
+        self.device = device
+        self._support_cache = None
+        self._fd = None
+
+    @classmethod
+    def from_config(cls, config):
+        device = config.get('device', cls.DEFAULT_DEVICE)
+        return cls(device)
+
+    @property
+    def is_running(self):
+        return self._fd is not None
+
+    def open(self):
+        try:
+            self._fd = os.open(self.device, os.O_WRONLY)
+        except OSError as e:
+            raise WatchdogError("Can't open watchdog device: {0}".format(e))
+
+    def close(self):
+        if self.is_running:
+            try:
+                os.write(self._fd, b'V')
+                os.close(self._fd)
+                self._fd = None
+            except OSError as e:
+                return WatchdogError("Error while closing {0}: {1}".format(self.describe(), e))
+
+    @property
+    def can_be_disabled(self):
+        return self.get_support().has_MAGICCLOSE
+
+    def _ioctl(self, func, arg, mutate_arg=False):
+        if self._fd is None:
+            raise WatchdogError("Watchdog device is closed")
+
+        result = fcntl.ioctl(self._fd, func, arg, mutate_arg)
+        if result < 0:
+            raise IOError(result)
+
+    def get_support(self):
+        if self._support_cache is None:
+            info = watchdog_info()
+            self._ioctl(WDIOC_GETSUPPORT, info, True)
+            self._support_cache = WatchdogInfo(info.options,
+                                               info.firmware_version,
+                                               str(bytearray(info.identity)).rstrip('\x00'))
+        return self._support_cache
+
+    def describe(self):
+        dev_str = " at {0}".format(self.device) if self.device != self.DEFAULT_DEVICE else ""
+        ver_str = ""
+        identity = "Linux watchdog device"
+        if self._fd:
+            try:
+                _, version, identity = self.get_support()
+                ver_str = " (firmware {0})".format(version) if version else ""
+            except WatchdogError:
+                pass
+
+        return identity + ver_str + dev_str
+
+    def keepalive(self):
+        try:
+            os.write(self._fd, b'1')
+        except OSError as e:
+            raise WatchdogError("Could not send watchdog keepalive: {0}".format(e))
+
+    def has_set_timeout(self):
+        """Returns True if setting a timeout is supported."""
+        return self.get_support().has_SETTIMEOUT
+
+    def set_timeout(self, timeout):
+        timeout = int(timeout)
+        if not 0 < timeout < 0xFFFF:
+            raise WatchdogError("Invalid timeout {0}. Supported values are between 1 and 65535".format(timeout))
+        self._ioctl(WDIOC_SETTIMEOUT, ctypes.c_int(timeout))
+
+    def get_timeout(self):
+        timeout = ctypes.c_int()
+        self._ioctl(WDIOC_GETTIMEOUT, timeout, True)
+        return timeout.value
+
+
+class TestingWatchdogDevice(LinuxWatchdogDevice):
+    """Converts timeout ioctls to regular writes that can be intercepted from a named pipe."""
+    timeout = 60
+
+    def get_support(self):
+        return WatchdogInfo(WDIOF['MAGICCLOSE'] | WDIOF['SETTIMEOUT'], 0, "Watchdog test harness")
+
+    def set_timeout(self, timeout):
+        buf = "Ctimeout={0}\n".format(timeout).encode('utf8')
+        while len(buf):
+            buf = buf[os.write(self._fd, buf):]
+        self.timeout = timeout
+
+    def get_timeout(self):
+        return self.timeout

postgres0.yml

@@ -76,6 +76,11 @@ postgresql:
       password: zalando
   parameters:
     unix_socket_directories: '.'
+
+#watchdog:
+#  mode: automatic # Allowed values: off, automatic, required
+#  device: /dev/watchdog
+
 tags:
     nofailover: false
     noloadbalance: false

requirements.txt

@@ -11,3 +11,5 @@ click>=4.1
 prettytable>=0.7
 tzlocal
 python-dateutil
+psutil
+cdiff

tests/test_api.py

@@ -25,6 +25,8 @@ class MockPostgresql(object):
     sysid = 'dummysysid'
     scope = 'dummy'
     pending_restart = True
+    wal_name = 'wal'
+    lsn_name = 'lsn'
 
     @staticmethod
     def connection():
@@ -64,7 +66,7 @@ class MockHa(object):
         return True
 
     @staticmethod
-    def is_lagging(xlog):
+    def is_lagging(wal):
         return False
 
     @staticmethod
@@ -104,6 +106,7 @@ class MockRequest(object):
     def sendall(self, *args, **kwargs):
         pass
 
+
 class MockRestApiServer(RestApiServer):
 
     def __init__(self, Handler, request):

tests/test_ctl.py

@@ -7,7 +7,8 @@ import unittest
 from click.testing import CliRunner
 from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, request_patroni, get_dcs, parse_dcs, \
-    get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
+    get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException, apply_config_changes, \
+    format_config_for_editing, show_diff, invoke_editor
 from patroni.dcs.etcd import Client
 from psycopg2 import OperationalError
 from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse
@@ -438,3 +439,70 @@ class TestCtl(unittest.TestCase):
             with patch('requests.patch', Mock(side_effect=Exception)):
                 result = self.runner.invoke(ctl, ['resume', 'dummy'])
                 assert 'Can not find accessible cluster member' in result.output
+
+    def test_apply_config_changes(self):
+        config = {"postgresql": {"parameters": {"work_mem": "4MB"}, "use_pg_rewind": True}, "ttl": 30}
+
+        before_editing = format_config_for_editing(config)
+
+        # Spaces are allowed and stripped, numbers and booleans are interpreted
+        after_editing, changed_config = apply_config_changes(before_editing, config,
+                                                             ["postgresql.parameters.work_mem = 5MB",
+                                                              "ttl=15", "postgresql.use_pg_rewind=off", 'a.b=c'])
+        self.assertEquals(changed_config, {"a": {"b": "c"}, "postgresql": {"parameters": {"work_mem": "5MB"},
+                                                                           "use_pg_rewind": False}, "ttl": 15})
+
+        # postgresql.parameters namespace is flattened
+        after_editing, changed_config = apply_config_changes(before_editing, config,
+                                                             ["postgresql.parameters.work_mem.sub = x"])
+        self.assertEquals(changed_config, {"postgresql": {"parameters": {"work_mem": "4MB", "work_mem.sub": "x"},
+                                                          "use_pg_rewind": True}, "ttl": 30})
+
+        # Setting to null deletes
+        after_editing, changed_config = apply_config_changes(before_editing, config,
+                                                             ["postgresql.parameters.work_mem=null"])
+        self.assertEquals(changed_config, {"postgresql": {"use_pg_rewind": True}, "ttl": 30})
+        after_editing, changed_config = apply_config_changes(before_editing, config,
+                                                             ["postgresql.use_pg_rewind=null",
+                                                              "postgresql.parameters.work_mem=null"])
+        self.assertEquals(changed_config, {"ttl": 30})
+
+        self.assertRaises(PatroniCtlException, apply_config_changes, before_editing, config, ['a'])
+
+    @patch('sys.stdout.isatty', return_value=False)
+    @patch('cdiff.markup_to_pager')
+    def test_show_diff(self, mock_markup_to_pager, mock_isatty):
+        show_diff("foo:\n  bar: 1\n", "foo:\n  bar: 2\n")
+        mock_markup_to_pager.assert_not_called()
+
+        mock_isatty.return_value = True
+        show_diff("foo:\n  bar: 1\n", "foo:\n  bar: 2\n")
+        mock_markup_to_pager.assert_called_once()
+
+        # Test that unicode handling doesn't fail with an exception
+        show_diff(b"foo:\n  bar: \xc3\xb6\xc3\xb6\n".decode('utf-8'),
+                  b"foo:\n  bar: \xc3\xbc\xc3\xbc\n".decode('utf-8'))
+
+    def test_invoke_editor(self):
+        for e in ('', 'false'):
+            os.environ['EDITOR'] = e
+            self.assertRaises(PatroniCtlException, invoke_editor, 'foo: bar\n', 'test')
+
+    @patch('patroni.ctl.get_dcs')
+    def test_show_config(self, mock_get_dcs):
+        mock_get_dcs.return_value = self.e
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        self.runner.invoke(ctl, ['show-config', 'dummy'])
+
+    @patch('patroni.ctl.get_dcs')
+    def test_edit_config(self, mock_get_dcs):
+        mock_get_dcs.return_value = self.e
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        os.environ['EDITOR'] = 'true'
+        self.runner.invoke(ctl, ['edit-config', 'dummy'])
+        self.runner.invoke(ctl, ['edit-config', 'dummy', '-s', 'foo=bar'])
+        self.runner.invoke(ctl, ['edit-config', 'dummy', '--replace', 'postgres0.yml'])
+        self.runner.invoke(ctl, ['edit-config', 'dummy', '--apply', '-'], input='foo: bar')
+        self.runner.invoke(ctl, ['edit-config', 'dummy', '--force', '--apply', '-'], input='foo: bar')
+        mock_get_dcs.return_value.set_config_value = Mock(return_value=True)
+        self.runner.invoke(ctl, ['edit-config', 'dummy', '--force', '--apply', '-'], input='foo: bar')

tests/test_ha.py

@@ -10,6 +10,7 @@ from patroni.dcs.etcd import Client
 from patroni.exceptions import DCSError, PostgresException
 from patroni.ha import Ha, _MemberStatus
 from patroni.postgresql import Postgresql
+from patroni.watchdog import Watchdog
 from patroni.utils import tzutc
 from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get
 from test_postgresql import psycopg2_connect
@@ -54,12 +55,12 @@ def get_cluster_initialized_with_only_leader(failover=None):
     return get_cluster(True, l, [l], failover, None)
 
 
-def get_node_status(reachable=True, in_recovery=True, xlog_location=10, nofailover=False):
+def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailover=False):
     def fetch_node_status(e):
         tags = {}
         if nofailover:
             tags['nofailover'] = True
-        return _MemberStatus(e, reachable, in_recovery, xlog_location, tags)
+        return _MemberStatus(e, reachable, in_recovery, wal_position, tags)
     return fetch_node_status
 
 future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5)
@@ -84,6 +85,8 @@ postgresql:
   pg_rewind:
     username: postgres
     password: postgres
+watchdog:
+  mode: off
 zookeeper:
   exhibitor:
     hosts: [localhost]
@@ -101,6 +104,7 @@ zookeeper:
         self.nosync = False
         self.scheduled_restart = {'schedule': future_restart_time,
                                   'postmaster_start_time': str(postmaster_start_time)}
+        self.watchdog = Watchdog(self.config)
 
 
 def run_async(self, func, args=()):
@@ -109,13 +113,13 @@ def run_async(self, func, args=()):
 
 @patch.object(Postgresql, 'is_running', Mock(return_value=True))
 @patch.object(Postgresql, 'is_leader', Mock(return_value=True))
-@patch.object(Postgresql, 'xlog_position', Mock(return_value=10))
+@patch.object(Postgresql, 'wal_position', Mock(return_value=10))
 @patch.object(Postgresql, 'call_nowait', Mock(return_value=True))
 @patch.object(Postgresql, 'data_directory_empty', Mock(return_value=False))
 @patch.object(Postgresql, 'controldata', Mock(return_value={'Database system identifier': '1234567890'}))
 @patch.object(Postgresql, 'sync_replication_slots', Mock())
 @patch.object(Postgresql, 'write_pg_hba', Mock())
-@patch.object(Postgresql, 'write_pgpass', Mock())
+@patch.object(Postgresql, 'write_pgpass', Mock(return_value={}))
 @patch.object(Postgresql, 'write_recovery_conf', Mock())
 @patch.object(Postgresql, 'query', Mock())
 @patch.object(Postgresql, 'checkpoint', Mock())
@@ -126,6 +130,7 @@ def run_async(self, func, args=()):
 @patch('patroni.async_executor.AsyncExecutor.busy', PropertyMock(return_value=False))
 @patch('patroni.async_executor.AsyncExecutor.run_async', run_async)
 @patch('subprocess.call', Mock(return_value=0))
+@patch('time.sleep', Mock())
 class TestHa(unittest.TestCase):
 
     @patch('socket.getaddrinfo', socket_getaddrinfo)
@@ -158,7 +163,7 @@ class TestHa(unittest.TestCase):
         self.assertTrue(self.ha.update_lock(True))
 
     def test_touch_member(self):
-        self.p.xlog_position = Mock(side_effect=Exception)
+        self.p.wal_position = Mock(side_effect=Exception)
         self.ha.touch_member()
 
     def test_start_as_replica(self):
@@ -167,7 +172,6 @@ class TestHa(unittest.TestCase):
 
     def test_recover_replica_failed(self):
         self.p.controldata = lambda: {'Database cluster state': 'in production'}
-        self.p.is_healthy = false
         self.p.is_running = false
         self.p.follow = false
         self.assertEquals(self.ha.run_cycle(), 'starting as a secondary')
@@ -175,7 +179,6 @@ class TestHa(unittest.TestCase):
 
     def test_recover_master_failed(self):
         self.p.follow = false
-        self.p.is_healthy = false
         self.p.is_running = false
         self.p.name = 'leader'
         self.p.set_role('master')
@@ -183,6 +186,12 @@ class TestHa(unittest.TestCase):
         self.ha.cluster = get_cluster_initialized_with_leader()
         self.assertEquals(self.ha.run_cycle(), 'starting as readonly because i had the session lock')
 
+    @patch.object(Postgresql, 'rewind_needed_and_possible', Mock(return_value=True))
+    def test_recover_with_rewind(self):
+        self.p.is_running = false
+        self.ha.cluster = get_cluster_initialized_with_leader()
+        self.assertEquals(self.ha.run_cycle(), 'running pg_rewind from leader')
+
     @patch('sys.exit', return_value=1)
     @patch('patroni.ha.Ha.sysid_valid', MagicMock(return_value=True))
     def test_sysid_no_match(self, exit_mock):
@@ -260,10 +269,18 @@ class TestHa(unittest.TestCase):
         self.p.is_leader = false
         self.assertEquals(self.ha.run_cycle(), 'PAUSE: no action')
 
+    @patch.object(Postgresql, 'rewind_needed_and_possible', Mock(return_value=True))
+    def test_follow_triggers_rewind(self):
+        self.p.is_leader = false
+        self.p.trigger_check_diverged_lsn()
+        self.ha.cluster = get_cluster_initialized_with_leader()
+        self.assertEquals(self.ha.run_cycle(), 'running pg_rewind from leader')
+
     def test_no_etcd_connection_master_demote(self):
         self.ha.load_cluster_from_dcs = Mock(side_effect=DCSError('Etcd is not responding properly'))
         self.assertEquals(self.ha.run_cycle(), 'demoted self because DCS is not accessible and i was a leader')
 
+    @patch('time.sleep', Mock())
     def test_bootstrap_from_another_member(self):
         self.ha.cluster = get_cluster_initialized_with_leader()
         self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from replica \'other\'')
@@ -292,6 +309,7 @@ class TestHa(unittest.TestCase):
         self.p.bootstrap = Mock(side_effect=PostgresException("Could not bootstrap master PostgreSQL"))
         self.assertRaises(PostgresException, self.ha.bootstrap)
 
+    @patch('psycopg2.connect', psycopg2_connect)
     def test_reinitialize(self):
         self.assertIsNotNone(self.ha.reinitialize())
 
@@ -303,6 +321,7 @@ class TestHa(unittest.TestCase):
         self.ha.state_handler.name = self.ha.cluster.leader.name
         self.assertIsNotNone(self.ha.reinitialize())
 
+    @patch('time.sleep', Mock())
     def test_restart(self):
         self.assertEquals(self.ha.restart({}), (True, 'restarted successfully'))
         self.p.restart = Mock(return_value=None)
@@ -319,7 +338,7 @@ class TestHa(unittest.TestCase):
         with patch('patroni.async_executor.AsyncExecutor.busy', PropertyMock(return_value=True)):
             self.ha.restart({}, run_async=True)
             self.assertTrue(self.ha.restart_scheduled())
-            self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race')
+            self.assertEquals(self.ha.run_cycle(), 'restart in progress')
 
             self.ha.cluster = get_cluster_initialized_with_leader()
             self.assertEquals(self.ha.run_cycle(), 'restart in progress')
@@ -328,10 +347,12 @@ class TestHa(unittest.TestCase):
             self.assertEquals(self.ha.run_cycle(), 'updated leader lock during restart')
 
             self.ha.update_lock = false
-            self.assertEquals(self.ha.run_cycle(), 'failed to update leader lock during restart')
+            self.p.set_role('master')
+            with patch('patroni.postgresql.Postgresql.stop') as stop_mock:
+                self.assertEquals(self.ha.run_cycle(), 'lost leader lock during restart')
+                stop_mock.assert_called()
 
     @patch('requests.get', requests_get)
-    @patch('time.sleep', Mock())
     def test_manual_failover_from_leader(self):
         self.ha.fetch_node_status = get_node_status()
         self.ha.has_lock = true
@@ -344,9 +365,11 @@ class TestHa(unittest.TestCase):
         f = Failover(0, self.p.name, '', None)
         self.ha.cluster = get_cluster_initialized_with_leader(f)
         self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself')
+        self.p.rewind_needed_and_possible = true
+        self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself')
         self.ha.fetch_node_status = get_node_status(nofailover=True)
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
-        self.ha.fetch_node_status = get_node_status(xlog_location=1)
+        self.ha.fetch_node_status = get_node_status(wal_position=1)
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
         # manual failover from the previous leader to us won't happen if we hold the nofailover flag
         self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, None))
@@ -384,7 +407,6 @@ class TestHa(unittest.TestCase):
         self.assertEquals('PAUSE: no action.  i am the leader with the lock', self.ha.run_cycle())
 
     @patch('requests.get', requests_get)
-    @patch('time.sleep', Mock())
     def test_manual_failover_process_no_leader(self):
         self.p.is_leader = false
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', self.p.name, None))
@@ -409,7 +431,6 @@ class TestHa(unittest.TestCase):
         self.ha.patroni.nofailover = True
         self.assertEquals(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
 
-    @patch('time.sleep', Mock())
     def test_manual_failover_process_no_leader_in_pause(self):
         self.ha.is_paused = true
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None))
@@ -440,9 +461,9 @@ class TestHa(unittest.TestCase):
         self.assertTrue(self.ha._is_healthiest_node(self.ha.old_cluster.members))
         self.ha.fetch_node_status = get_node_status(in_recovery=False)  # accessible, not in_recovery
         self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members))
-        self.ha.fetch_node_status = get_node_status(xlog_location=11)  # accessible, in_recovery, xlog location ahead
+        self.ha.fetch_node_status = get_node_status(wal_position=11)  # accessible, in_recovery, wal position ahead
         self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members))
-        with patch('patroni.postgresql.Postgresql.xlog_position', return_value=1):
+        with patch('patroni.postgresql.Postgresql.wal_position', return_value=1):
             self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members))
         self.ha.patroni.nofailover = True
         self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members))
@@ -569,7 +590,6 @@ class TestHa(unittest.TestCase):
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am a secondary and i am following a leader')
         check_calls([(update_lock, False), (demote, False)])
 
-    @patch('time.sleep', Mock())
     def test_manual_failover_while_starting(self):
         self.ha.has_lock = true
         self.p.check_for_startup = true
@@ -589,15 +609,13 @@ class TestHa(unittest.TestCase):
         self.assertEquals(self.ha.run_cycle(), 'stopped PostgreSQL to fail over after a crash')
         demote.assert_called_once()
 
-    @patch('time.sleep', Mock())
     @patch('patroni.postgresql.Postgresql.follow')
     def test_demote_immediate(self, follow):
         self.ha.has_lock = true
         self.e.get_cluster = Mock(return_value=get_cluster_initialized_without_leader())
         self.ha.demote('immediate')
-        follow.assert_called_once_with(None, None, True, None, True)
+        follow.assert_called_once_with(None)
 
-    @patch('time.sleep', Mock())
     def test_process_sync_replication(self):
         self.ha.has_lock = true
         mock_set_sync = self.p.set_synchronous_standby = Mock()
@@ -663,6 +681,13 @@ class TestHa(unittest.TestCase):
         self.ha.run_cycle()
         self.assertEquals(self.ha.dcs.write_sync_state.call_count, 1)
 
+        # Test sync set to '*' when synchronous_mode_strict is enabled
+        mock_set_sync.reset_mock()
+        self.ha.is_synchronous_mode_strict = true
+        self.p.pick_synchronous_standby = Mock(return_value=(None, False))
+        self.ha.run_cycle()
+        mock_set_sync.assert_called_once_with('*')
+
     def test_sync_replication_become_master(self):
         self.ha.is_synchronous_mode = true
 
@@ -717,8 +742,7 @@ class TestHa(unittest.TestCase):
         mock_promote.assert_called_once()
         mock_write_sync.assert_called_once_with('other', None, index=0)
 
-    @patch('time.sleep')
-    def test_disable_sync_when_restarting(self, mock_sleep):
+    def test_disable_sync_when_restarting(self):
         self.ha.is_synchronous_mode = true
 
         self.p.name = 'other'
@@ -731,10 +755,10 @@ class TestHa(unittest.TestCase):
             get_cluster_initialized_with_leader(sync=('leader', syncstandby))
             for syncstandby in ['other', None]])
 
-        self.ha.restart({})
-
-        mock_restart.assert_called_once()
-        mock_sleep.assert_called()
+        with patch('time.sleep') as mock_sleep:
+            self.ha.restart({})
+            mock_restart.assert_called_once()
+            mock_sleep.assert_called()
 
         # Restart is still called when DCS connection fails
         mock_restart.reset_mock()
@@ -772,6 +796,7 @@ class TestHa(unittest.TestCase):
     def test_wakup(self):
         self.ha.wakeup()
 
+    @patch('time.sleep', Mock())
     def test_leader_with_empty_directory(self):
         self.ha.cluster = get_cluster_initialized_with_leader()
         self.ha.has_lock = true

tests/test_postgresql.py

@@ -30,7 +30,7 @@ class MockCursor(object):
         elif sql.startswith('SELECT slot_name'):
             self.results = [('blabla',), ('foobar',)]
         elif sql.startswith('SELECT CASE WHEN pg_is_in_recovery()'):
-            self.results = [(0,)]
+            self.results = [(2,)]
         elif sql == 'SELECT pg_is_in_recovery()':
             self.results = [(False, )]
         elif sql.startswith('WITH replication_info AS ('):
@@ -43,6 +43,13 @@ class MockCursor(object):
                             ('port', '5433', None, 'integer', 'postmaster'),
                             ('listen_addresses', '*', None, 'string', 'postmaster'),
                             ('autovacuum', 'on', None, 'bool', 'sighup')]
+        elif sql.startswith('IDENTIFY_SYSTEM'):
+            self.results = [('1', 2, '0/402EEC0', '')]
+        elif sql.startswith('TIMELINE_HISTORY '):
+            self.results = [('', b'x\t0/40159C0\tno recovery target specified\n\n' +
+                                 b'1\t0/40159C0\tno recovery target specified\n\n' +
+                                 b'2\t0/402DD98\tno recovery target specified\n\n' +
+                                 b'3\t0/403DD98\tno recovery target specified\n')]
         else:
             self.results = [(None, None, None, None, None, None, None, None, None, None)]
 
@@ -65,7 +72,7 @@ class MockCursor(object):
 
 class MockConnect(object):
 
-    server_version = '99999'
+    server_version = 99999
     autocommit = False
     closed = 0
 
@@ -138,21 +145,10 @@ Data page checksum version:           0
 """
 
 
-def postmaster_opts_string(*args, **kwargs):
-    return '/usr/local/pgsql/bin/postgres "-D" "data/postgresql0" "--listen_addresses=127.0.0.1" \
-"--port=5432" "--hot_standby=on" "--wal_keep_segments=8" "--wal_level=hot_standby" \
-"--archive_command=mkdir -p ../wal_archive && cp %p ../wal_archive/%f" "--wal_log_hints=on" \
-"--max_wal_senders=5" "--archive_timeout=1800s" "--archive_mode=on" "--max_replication_slots=5"\n'
-
-
 def psycopg2_connect(*args, **kwargs):
     return MockConnect()
 
 
-def fake_listdir(path):
-    return ["a", "b", "c"] if path.endswith('pg_xlog/archive_status') else []
-
-
 @patch('subprocess.call', Mock(return_value=0))
 @patch('psycopg2.connect', psycopg2_connect)
 class TestPostgresql(unittest.TestCase):
@@ -165,7 +161,7 @@ class TestPostgresql(unittest.TestCase):
     @patch('subprocess.call', Mock(return_value=0))
     @patch('psycopg2.connect', psycopg2_connect)
     @patch('os.rename', Mock())
-    @patch.object(Postgresql, 'get_major_version', Mock(return_value=9.6))
+    @patch.object(Postgresql, 'get_major_version', Mock(return_value=90600))
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
     def setUp(self):
         self.data_dir = 'data/test0'
@@ -293,39 +289,84 @@ class TestPostgresql(unittest.TestCase):
     @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_pg_rewind(self, mock_call):
         r = {'user': '', 'host': '', 'port': '', 'database': '', 'password': ''}
-        self.assertTrue(self.p.rewind(r))
+        self.assertTrue(self.p.pg_rewind(r))
         subprocess.call = mock_call
-        self.assertFalse(self.p.rewind(r))
+        self.assertFalse(self.p.pg_rewind(r))
 
-    @patch('os.unlink', Mock(return_value=True))
-    @patch('subprocess.check_output', Mock(return_value=0, side_effect=pg_controldata_string))
-    @patch.object(Postgresql, 'remove_data_directory', Mock(return_value=True))
-    @patch.object(Postgresql, 'single_user_mode', Mock(return_value=1))
-    @patch.object(Postgresql, 'write_pgpass', Mock(return_value={}))
-    @patch.object(Postgresql, 'is_running', Mock(return_value=True))
+    def test_check_recovery_conf(self):
+        self.p.write_recovery_conf('foo')
+        self.assertFalse(self.p.check_recovery_conf(None))
+        self.p.write_recovery_conf(None)
+        self.assertTrue(self.p.check_recovery_conf(None))
+
+    @patch.object(Postgresql, 'start', Mock())
     @patch.object(Postgresql, 'can_rewind', PropertyMock(return_value=True))
-    @patch.object(Postgresql, 'rewind', return_value=False)
-    def test_follow(self, mock_pg_rewind):
-        with patch.object(Postgresql, 'check_recovery_conf', Mock(return_value=True)):
-            self.assertTrue(self.p.follow(None, None))  # nothing to do, recovery.conf has good primary_conninfo
+    def test__get_local_timeline_lsn(self):
+        self.p.trigger_check_diverged_lsn()
+        with patch.object(Postgresql, 'controldata', Mock(return_value={'Database cluster state': 'shut down'})):
+            self.p.rewind_needed_and_possible(self.leader)
+        with patch.object(Postgresql, 'controldata',
+                          Mock(return_value={'Database cluster state': 'shut down in recovery'})):
+            self.p.rewind_needed_and_possible(self.leader)
+        with patch.object(Postgresql, 'is_running', Mock(return_value=True)):
+            with patch.object(MockCursor, 'fetchone', Mock(side_effect=[(False, ), Exception])):
+                self.p.rewind_needed_and_possible(self.leader)
 
-        self.p.follow(self.me, self.me)  # follow is called when the node is holding leader lock
+    @patch.object(Postgresql, 'start', Mock())
+    @patch.object(Postgresql, 'can_rewind', PropertyMock(return_value=True))
+    @patch.object(Postgresql, '_get_local_timeline_lsn', Mock(return_value=(2, '0/40159C1')))
+    @patch.object(Postgresql, 'check_leader_is_not_in_recovery')
+    def test__check_timeline_and_lsn(self, mock_check_leader_is_not_in_recovery):
+        mock_check_leader_is_not_in_recovery.return_value = False
+        self.p.trigger_check_diverged_lsn()
+        self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+        mock_check_leader_is_not_in_recovery.return_value = True
+        self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+        self.p.trigger_check_diverged_lsn()
+        with patch('psycopg2.connect', Mock(side_effect=Exception)):
+            self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+        with patch.object(MockCursor, 'fetchone',
+                          Mock(side_effect=[('', 2, '0/0'), ('', b'2\tG/40159C0\tno recovery target specified\n\n')])):
+            self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+        self.p.trigger_check_diverged_lsn()
+        with patch.object(MockCursor, 'fetchone',
+                          Mock(side_effect=[('', 2, '0/0'), ('', b'3\t040159C0\tno recovery target specified\n')])):
+            self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+        self.p.trigger_check_diverged_lsn()
+        with patch.object(MockCursor, 'fetchone', Mock(return_value=('', 1, '0/0'))):
+            with patch.object(Postgresql, '_get_local_timeline_lsn', Mock(return_value=(1, '0/0'))):
+                self.assertFalse(self.p.rewind_needed_and_possible(self.leader))
+            self.p.trigger_check_diverged_lsn()
+            self.assertTrue(self.p.rewind_needed_and_possible(self.leader))
 
-        with patch.object(Postgresql, 'restart', Mock(return_value=False)):
-            self.p.set_role('replica')
-            self.p.follow(None, None)  # restart without rewind
+    @patch.object(MockCursor, 'fetchone', Mock(side_effect=[(True,), Exception]))
+    def test_check_leader_is_not_in_recovery(self):
+        self.p.check_leader_is_not_in_recovery()
+        self.p.check_leader_is_not_in_recovery()
 
-        with patch.object(Postgresql, 'stop', Mock(return_value=False)):
-            self.p.follow(self.leader, self.leader, need_rewind=True)  # failed to stop postgres
+    @patch.object(Postgresql, 'checkpoint', side_effect=['', '1'])
+    @patch.object(Postgresql, 'stop', Mock(return_value=False))
+    @patch.object(Postgresql, 'start', Mock())
+    def test_rewind(self, mock_checkpoint):
+        self.p.rewind(self.leader)
+        with patch.object(Postgresql, 'pg_rewind', Mock(return_value=False)):
+            mock_checkpoint.side_effect = ['1', '', '', '']
+            self.p.rewind(self.leader)
+            self.p.rewind(self.leader)
+            with patch.object(Postgresql, 'check_leader_is_not_in_recovery', Mock(return_value=False)):
+                self.p.rewind(self.leader)
+            self.p.config['remove_data_directory_on_rewind_failure'] = False
+            self.p.trigger_check_diverged_lsn()
+            self.p.rewind(self.leader)
+        with patch.object(Postgresql, 'is_running', Mock(return_value=True)):
+            self.p.rewind(self.leader)
+            self.p.is_leader = Mock(return_value=False)
+            self.p.rewind(self.leader)
 
-        self.p.follow(self.leader, self.leader)  # "leader" is not accessible or is_in_recovery
-
-        with patch.object(Postgresql, 'checkpoint', Mock(return_value=None)):
-            self.p.follow(self.leader, self.leader)
-            mock_pg_rewind.return_value = True
-            self.p.follow(self.leader, self.leader, need_rewind=True)
-
-        self.p.follow(None, None)  # check_recovery_conf...
+    @patch.object(Postgresql, 'is_running', Mock(return_value=False))
+    @patch.object(Postgresql, 'start', Mock())
+    def test_follow(self):
+        self.p.follow(None)
 
     @patch('subprocess.check_output', Mock(return_value=0, side_effect=pg_controldata_string))
     def test_can_rewind(self):
@@ -333,7 +374,7 @@ class TestPostgresql(unittest.TestCase):
             self.assertFalse(self.p.can_rewind)
         with patch('subprocess.call', side_effect=OSError):
             self.assertFalse(self.p.can_rewind)
-        with patch.object(Postgresql, 'controldata', Mock(return_value={'wal_log_hints setting': 'on'})):
+        with patch.object(Postgresql, 'controldata', Mock(return_value={'Current wal_log_hints setting': 'on'})):
             self.assertTrue(self.p.can_rewind)
         self.p.config['use_pg_rewind'] = False
         self.assertFalse(self.p.can_rewind)
@@ -407,12 +448,12 @@ class TestPostgresql(unittest.TestCase):
         self.assertFalse(self.p.is_healthy())
 
     def test_promote(self):
-        self.p._role = 'replica'
+        self.p.set_role('replica')
         self.assertTrue(self.p.promote())
         self.assertTrue(self.p.promote())
 
     def test_last_operation(self):
-        self.assertEquals(self.p.last_operation(), '0')
+        self.assertEquals(self.p.last_operation(), '2')
         Thread(target=self.p.last_operation).start()
 
     @patch('os.path.isfile', Mock(return_value=True))
@@ -425,7 +466,9 @@ class TestPostgresql(unittest.TestCase):
         self.assertFalse(self.p.is_running())
 
     @patch('shlex.split', Mock(side_effect=OSError))
+    @patch.object(Postgresql, 'can_rewind', PropertyMock(return_value=True))
     def test_call_nowait(self):
+        self.p.set_role('replica')
         self.assertIsNone(self.p.call_nowait('on_start'))
 
     def test_non_existing_callback(self):
@@ -502,71 +545,12 @@ class TestPostgresql(unittest.TestCase):
             data = self.p.controldata()
             self.assertEquals(len(data), 50)
             self.assertEquals(data['Database cluster state'], 'shut down in recovery')
-            self.assertEquals(data['wal_log_hints setting'], 'on')
+            self.assertEquals(data['Current wal_log_hints setting'], 'on')
             self.assertEquals(int(data['Database block size']), 8192)
 
         with patch('subprocess.check_output', Mock(side_effect=subprocess.CalledProcessError(1, ''))):
             self.assertEquals(self.p.controldata(), {})
 
-    def test_read_postmaster_opts(self):
-        m = mock_open(read_data=postmaster_opts_string())
-        with patch.object(builtins, 'open', m):
-            data = self.p.read_postmaster_opts()
-            self.assertEquals(data['wal_level'], 'hot_standby')
-            self.assertEquals(int(data['max_replication_slots']), 5)
-            self.assertEqual(data.get('D'), None)
-
-            m.side_effect = IOError
-            data = self.p.read_postmaster_opts()
-            self.assertEqual(data, dict())
-
-    @patch('subprocess.Popen')
-    @patch.object(builtins, 'open', MagicMock(return_value=42))
-    def test_single_user_mode(self, subprocess_popen_mock):
-        subprocess_popen_mock.return_value.wait.return_value = 0
-        self.assertEquals(self.p.single_user_mode(options=dict(archive_mode='on', archive_command='false')), 0)
-        subprocess_popen_mock.assert_called_once_with(['postgres', '--single', '-D', self.data_dir,
-                                                      '-c', 'archive_command=false', '-c', 'archive_mode=on',
-                                                       'postgres'], stdin=subprocess.PIPE,
-                                                      stdout=42,
-                                                      stderr=subprocess.STDOUT)
-        subprocess_popen_mock.reset_mock()
-        self.assertEquals(self.p.single_user_mode(command="CHECKPOINT"), 0)
-        subprocess_popen_mock.assert_called_once_with(['postgres', '--single', '-D', self.data_dir,
-                                                      'postgres'], stdin=subprocess.PIPE,
-                                                      stdout=42,
-                                                      stderr=subprocess.STDOUT)
-        subprocess_popen_mock.return_value = None
-        self.assertEquals(self.p.single_user_mode(), 1)
-
-    @patch('os.listdir', MagicMock(side_effect=fake_listdir))
-    @patch('os.unlink', return_value=True)
-    @patch('os.remove', return_value=True)
-    @patch('os.path.islink', return_value=False)
-    @patch('os.path.isfile', return_value=True)
-    def test_cleanup_archive_status(self, mock_file, mock_link, mock_remove, mock_unlink):
-        ap = os.path.join(self.data_dir, 'pg_xlog', 'archive_status/')
-        self.p.cleanup_archive_status()
-        mock_remove.assert_has_calls([mock.call(ap + 'a'), mock.call(ap + 'b'), mock.call(ap + 'c')])
-        mock_unlink.assert_not_called()
-
-        mock_remove.reset_mock()
-
-        mock_file.return_value = False
-        mock_link.return_value = True
-        self.p.cleanup_archive_status()
-        mock_unlink.assert_has_calls([mock.call(ap + 'a'), mock.call(ap + 'b'), mock.call(ap + 'c')])
-        mock_remove.assert_not_called()
-
-        mock_unlink.reset_mock()
-        mock_remove.reset_mock()
-
-        mock_file.side_effect = OSError
-        mock_link.side_effect = OSError
-        self.p.cleanup_archive_status()
-        mock_unlink.assert_not_called()
-        mock_remove.assert_not_called()
-
     @patch('patroni.postgresql.Postgresql._version_file_exists', Mock(return_value=True))
     @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string))
     def test_sysid(self):
@@ -601,21 +585,23 @@ class TestPostgresql(unittest.TestCase):
     def test_reload_config(self):
         parameters = self._PARAMETERS.copy()
         parameters.pop('f.oo')
-        self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters})
+        config = {'authentication': {}, 'retry_timeout': 10, 'listen': '*', 'parameters': parameters}
+        self.p.reload_config(config)
         parameters['b.ar'] = 'bar'
-        self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters})
+        self.p.reload_config(config)
         parameters['autovacuum'] = 'on'
-        self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters})
+        self.p.reload_config(config)
         parameters['autovacuum'] = 'off'
         parameters.pop('search_path')
-        self.p.reload_config({'retry_timeout': 10, 'listen': '*:5433', 'parameters': parameters})
+        config['listen'] = '*:5433'
+        self.p.reload_config(config)
 
     @patch.object(Postgresql, '_version_file_exists', Mock(return_value=True))
     def test_get_major_version(self):
         with patch.object(builtins, 'open', mock_open(read_data='9.4')):
-            self.assertEquals(self.p.get_major_version(), 9.4)
+            self.assertEquals(self.p.get_major_version(), 90400)
         with patch.object(builtins, 'open', Mock(side_effect=Exception)):
-            self.assertEquals(self.p.get_major_version(), 0.0)
+            self.assertEquals(self.p.get_major_version(), 0)
 
     def test_postmaster_start_time(self):
         with patch.object(MockCursor, "fetchone", Mock(return_value=('foo', True, '', '', '', '', False))):
@@ -768,5 +754,7 @@ class TestPostgresql(unittest.TestCase):
     def test_get_server_parameters(self):
         config = {'synchronous_mode': True, 'parameters': {'wal_level': 'hot_standby'}, 'listen': '0'}
         self.p.get_server_parameters(config)
+        config['synchronous_mode_strict'] = True
+        self.p.get_server_parameters(config)
         self.p.set_synchronous_standby('foo')
         self.p.get_server_parameters(config)

tests/test_wale_restore.py

@@ -2,41 +2,67 @@ import psycopg2
 import subprocess
 import unittest
 
-from mock import Mock, MagicMock, patch, mock_open
+from mock import Mock, PropertyMock, patch, mock_open
+from patroni.scripts import wale_restore
 from patroni.scripts.wale_restore import WALERestore, main as _main, get_major_version
 from six.moves import builtins
+from test_postgresql import MockConnect, psycopg2_connect
 
 
-wale_output = b'name last_modified expanded_size_bytes wal_segment_backup_start ' +\
-              b'wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop\n' +\
-              b'base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 ' +\
-              b'00000001000000000000007F 00000040 00000001000000000000007F 00000240\n'
+wale_output_header = (
+    b'name\tlast_modified\t'
+    b'expanded_size_bytes\t'
+    b'wal_segment_backup_start\twal_segment_offset_backup_start\t'
+    b'wal_segment_backup_stop\twal_segment_offset_backup_stop\n'
+)
+
+wale_output_values = (
+    b'base_00000001000000000000007F_00000040\t2015-05-18T10:13:25.000Z\t'
+    b'167772160\t'
+    b'00000001000000000000007F\t00000040\t'
+    b'00000001000000000000007F\t00000240\n'
+)
+
+wale_output = wale_output_header + wale_output_values
+
+wale_restore.RETRY_SLEEP_INTERVAL = 0.001  # Speed up retries
+WALE_TEST_RETRIES = 2
 
 
 @patch('os.access', Mock(return_value=True))
 @patch('os.makedirs', Mock(return_value=True))
 @patch('os.path.exists', Mock(return_value=True))
 @patch('os.path.isdir', Mock(return_value=True))
-@patch('psycopg2.extensions.cursor', Mock(autospec=True))
-@patch('psycopg2.extensions.connection', Mock(autospec=True))
-@patch('psycopg2.connect', MagicMock(autospec=True))
+@patch('psycopg2.connect', psycopg2_connect)
 @patch('subprocess.check_output', Mock(return_value=wale_output))
 class TestWALERestore(unittest.TestCase):
 
     def setUp(self):
-        self.wale_restore = WALERestore("batman", "/data", "host=batman port=5432 user=batman",
-                                        "/etc", 100, 100, 1, 0, 1)
+        self.wale_restore = WALERestore('batman', '/data', 'host=batman port=5432 user=batman',
+                                        '/etc', 100, 100, 1, 0, WALE_TEST_RETRIES)
 
     def test_should_use_s3_to_create_replica(self):
         self.assertTrue(self.wale_restore.should_use_s3_to_create_replica())
+        with patch.object(MockConnect, 'server_version', PropertyMock(return_value=100000)):
+            self.assertTrue(self.wale_restore.should_use_s3_to_create_replica())
+
+        with patch('subprocess.check_output', Mock(return_value=wale_output.replace(b'167772160', b'1'))):
+            self.assertFalse(self.wale_restore.should_use_s3_to_create_replica())
 
         with patch('psycopg2.connect', Mock(side_effect=psycopg2.Error("foo"))):
             save_no_master = self.wale_restore.no_master
             save_master_connection = self.wale_restore.master_connection
 
             self.assertFalse(self.wale_restore.should_use_s3_to_create_replica())
-            self.wale_restore.no_master = 1
-            self.assertTrue(self.wale_restore.should_use_s3_to_create_replica())  # this would do 2 retries 1 sec each
+
+            with patch('time.sleep', Mock(return_value=None)) as mock_sleep:
+                self.wale_restore.no_master = 1
+                assert self.wale_restore.should_use_s3_to_create_replica()
+                # verify retries
+                mock_sleep.assert_has_calls(
+                    [((wale_restore.RETRY_SLEEP_INTERVAL,),)] * WALE_TEST_RETRIES
+                )
+
             self.wale_restore.master_connection = ''
             self.assertTrue(self.wale_restore.should_use_s3_to_create_replica())
 
@@ -45,10 +71,9 @@ class TestWALERestore(unittest.TestCase):
 
         with patch('subprocess.check_output', Mock(side_effect=subprocess.CalledProcessError(1, "cmd", "foo"))):
             self.assertFalse(self.wale_restore.should_use_s3_to_create_replica())
-        with patch('subprocess.check_output', Mock(return_value=wale_output.split(b'\n')[0])):
+        with patch('subprocess.check_output', Mock(return_value=wale_output_header)):
             self.assertFalse(self.wale_restore.should_use_s3_to_create_replica())
-        with patch('subprocess.check_output',
-                   Mock(return_value=wale_output.replace(b' wal_segment_offset_backup_stop', b''))):
+        with patch('subprocess.check_output', Mock(return_value=wale_output + wale_output_values)):
             self.assertFalse(self.wale_restore.should_use_s3_to_create_replica())
         with patch('subprocess.check_output',
                    Mock(return_value=wale_output.replace(b'expanded_size_bytes', b'expanded_size_foo'))):
@@ -70,17 +95,22 @@ class TestWALERestore(unittest.TestCase):
         with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(return_value=True)):
             with patch.object(self.wale_restore, 'create_replica_with_s3', Mock(return_value=0)):
                 self.assertEqual(self.wale_restore.run(), 0)
-            with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(return_value=None)):
-                self.assertEqual(self.wale_restore.run(), 1)
-            with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(side_effect=Exception)):
-                self.assertEqual(self.wale_restore.run(), 2)
+        with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(return_value=False)):
+            self.assertEqual(self.wale_restore.run(), 2)
+        with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(return_value=None)):
+            self.assertEqual(self.wale_restore.run(), 1)
+        with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', Mock(side_effect=Exception)):
+            self.assertEqual(self.wale_restore.run(), 2)
 
     @patch('sys.exit', Mock())
     def test_main(self):
         with patch.object(WALERestore, 'run', Mock(return_value=0)):
             self.assertEqual(_main(), 0)
-        with patch.object(WALERestore, 'run', Mock(return_value=1)):
+
+        with patch.object(WALERestore, 'run', Mock(return_value=1)), \
+                patch('time.sleep', Mock(return_value=None)) as mock_sleep:
             self.assertEqual(_main(), 1)
+            assert mock_sleep.call_count == WALE_TEST_RETRIES
 
     @patch('os.path.isfile', Mock(return_value=True))
     def test_get_major_version(self):

tests/test_watchdog.py

@@ -0,0 +1,90 @@
+import unittest
+from mock import patch
+import platform
+import ctypes
+
+from patroni.watchdog import Watchdog
+import patroni.watchdog.linux as linuxwd
+
+import sys
+
+class MockDevice(object):
+    def __init__(self, fd, filename, flag):
+        self.fd = fd
+        self.filename = filename
+        self.flag = flag
+        self.timeout = 60
+        self.open = True
+        self.writes = []
+
+
+mock_devices = [None]
+
+def mock_open(filename, flag):
+    fd = len(mock_devices)
+    mock_devices.append(MockDevice(fd, filename, flag))
+    return fd
+
+def mock_ioctl(fd, op, arg=None, mutate_flag=False):
+    assert 0 < fd < len(mock_devices)
+    dev = mock_devices[fd]
+    sys.stderr.write("Ioctl %d %d %r\n" %( fd, op, arg))
+    if op == linuxwd.WDIOC_GETSUPPORT:
+        sys.stderr.write("Get support\n")
+        assert(mutate_flag == True)
+        arg.options = sum(map(linuxwd.WDIOF.get, ['SETTIMEOUT', 'KEEPALIVEPING', 'MAGICCLOSE']))
+        arg.identity = (ctypes.c_ubyte*32)(*map(ord, 'Mock Watchdog'))
+    elif op == linuxwd.WDIOC_GETTIMEOUT:
+        arg.value = dev.timeout
+    elif op == linuxwd.WDIOC_SETTIMEOUT:
+        sys.stderr.write("Set timeout called with %s\n" % arg.value)
+        assert 0 < arg.value < 65535
+        dev.timeout  = arg.value
+    else:
+        raise Exception("Unknown op %d", op)
+    return 0
+
+def mock_write(fd, string):
+    assert 0 < fd < len(mock_devices)
+    assert len(string) == 1
+    assert mock_devices[fd].open
+    mock_devices[fd].writes.append(string)
+
+def mock_close(fd):
+    assert 0 < fd < len(mock_devices)
+    assert mock_devices[fd].open
+    mock_devices[fd].open = False
+
+@patch('os.open', mock_open)
+@patch('os.write', mock_write)
+@patch('os.close', mock_close)
+@patch('fcntl.ioctl', mock_ioctl)
+class TestWatchdog(unittest.TestCase):
+    def setUp(self):
+        mock_devices[:] = [None]
+
+    def test_basic_operation(self):
+        if platform.system() != 'Linux':
+            return
+
+        watchdog = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}})
+
+        watchdog.activate()
+        self.assertEquals(len(mock_devices), 2)
+        device = mock_devices[-1]
+        self.assertTrue(device.open)
+
+        self.assertEquals(device.timeout, 15)
+
+        watchdog.keepalive()
+        self.assertEquals(len(device.writes), 1)
+
+        watchdog.disable()
+        self.assertFalse(device.open)
+        self.assertEquals(device.writes[-1], b'V')
+
+    def test_invalid_timings(self):
+        watchdog = Watchdog({'ttl': 30, 'loop_wait': 20, 'watchdog': {'mode': 'automatic'}})
+        watchdog.activate()
+        self.assertEquals(len(mock_devices), 1)
+        self.assertFalse(watchdog.is_running)