Simplify watchdog code (#452)

* Only activate watchdog while master and not paused

We don't really need the protections while we are not master. This way
we only need to tickle the watchdog when we are updating leader key or
while demotion is happening.

As implemented we might fail to notice to shut down the watchdog if
someone demotes postgres and removes leader key behind Patroni's back.
There are probably other similar cases. Basically if the administrator
if being actively stupid they might get unexpected restarts. That seems
fine.

* Add configuration change support. Change MODE_REQUIRED to disable leader eligibility instead of closing Patroni.

Changes watchdog timeout during the next keepalive when ttl is changed. Watchdog driver and requirement can also be switched online.

When watchdog mode is `required` and watchdog setup does not work then the effect is similar to nofailover. Add watchdog_failed to status API to signify this. This is True only when watchdog does not work **AND** it is required.

* Reset implementation when config changed while active.

* Add watchdog safety margin configuration

Defaults to 5 seconds. Basically this is the maximum amount of time
that can pass between the calls to odcs.update_leader()` and
`watchdog.keepalive()`, which are called right after each other. Should
be safe for pretty much any sane scenario and allows the default
settings to not trigger watchdog when DCS is not responding.

* Cancel bootstrap if watchdog activation fails

The system would have demoted itself anyway the next HA loop. Doing it
in bootstrap gives at least some other node chance to try bootstrapping
in the hope that it is configured correctly.

If all nodes are unable to activate they will continue to try until the
disk is filled with moved datadirs. Perhaps not ideal behavior, but as
the situation is unlikely to resolve itself without administrator
intervention it doesn't seem too bad.

docs/SETTINGS.rst

@@ -15,6 +15,7 @@ Bootstrap configuration
 -  **dcs**: This section will be written into `/<namespace>/<scope>/config` of a given configuration store after initializing of new cluster. This is the global configuration for the cluster. If you want to change some parameters for all cluster nodes - just do it in DCS (or via Patroni API) and all nodes will apply this configuration.
     -  **loop\_wait**: the number of seconds the loop will sleep. Default value: 10
     -  **ttl**: the TTL to acquire the leader lock. Think of it as the length of time before initiation of the automatic failover process. Default value: 30
+    -  **retry\_timeout**: timeout for DCS and PostgreSQL operation retries. DCS or network issues shorter than this will not cause Patroni to demote the leader. Default value: 10
     -  **maximum\_lag\_on\_failover**: the maximum bytes a follower may lag to be able to participate in leader election.
     -  **master\_start\_timeout**: the amount of time a master is allowed to recover from failures before failover is triggered. Default is 300 seconds. When set to 0 failover is done immediately after a crash is detected if possible. When using asynchronous replication a failover can cause lost transactions. Best worst case failover time for master failure is: loop\_wait + master\_start\_timeout + loop\_wait, unless master\_start\_timeout is zero, in which case it's just loop\_wait. Set the value according to your durability/availability tradeoff.
     -  **synchronous\_mode**: turns on synchronous replication mode. In this mode a replica will be chosen as synchronous and only the latest leader and synchronous replica are able to participate in leader election. Synchronous mode makes sure that succesfully committed transactions will not be lost at failover, at the cost of losing availability for writes when Patroni cannot ensure transaction durability. See `replication modes documentation <https://github.com/zalando/patroni/blob/master/docs/replication_modes.rst>`__ for details.
@@ -119,3 +120,9 @@ REST API
 ZooKeeper
 ----------
 -  **hosts**: list of ZooKeeper cluster members in format: ['host1:port1', 'host2:port2', 'etc...'].
+
+Watchdog
+--------
+- **mode**: ``off``, ``automatic`` or ``required``. When ``off`` watchdog is disabled. When ``automatic`` watchdog will be used if available, but ignored if it is not. When ``required`` the node will not become a leader unless watchdog can be succesfully enabled.
+- **device**: Path to watchdog device. Defaults to ``/dev/watchdog``.
+- **safety_margin**: Number of seconds of safety margin between watchdog triggering and leader key expiration.

docs/watchdog.rst

@@ -10,16 +10,20 @@ Having multiple PostgreSQL servers running as master can result in transactions
 
 - 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 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. This adds an additional layer of fail safe in case usual Patroni split-brain protection mechanisms fail.
 
-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.
+Patroni will try to activate the watchdog before promoting PostgreSQL to master. If watchdog activation fails and watchdog mode is ``required`` then the node will refuse to become master. When deciding to participate in leader election Patroni will also check that watchdog configuration will allow it to become leader at all. After demoting PostgreSQL (for example due to a manual failover) Patroni will disable the watchdog again. Watchdog will also be disabled while Patroni is in paused state.
+
+By default Patroni will set up the watchdog to expire 5 seconds before TTL expires. With the default setup of ``loop_wait=10`` and ``ttl=30`` this gives HA loop at least 15 seconds (``ttl`` - ``safety_margin`` - ``loop_wait``) to complete before the system gets forcefully reset. By default accessing DCS is configured to time out after 10 seconds. This means that when DCS is unavailable, for example due to network issues, Patroni and PostgreSQL will have at least 5 seconds (``ttl`` - ``safety_margin`` - ``loop_wait`` - ``retry_timeout``) to come to a state where all client connections are terminated.
+
+Safety margin is the amount of time that Patroni reserves for time between leader key update and watchdog keepalive. Patroni will try to send a keepalive immediately after confirmation of leader key update. If Patroni process is suspended for extended amount of time at exactly the right moment the keepalive may be delayed for more than the safety margin without triggering the watchdog. This results in a window of time where watchdog will not trigger before leader key expiration, invalidating the guarantee. To be absolutely sure that watchdog will trigger under all circumstances set up the watchdog to expire after half of TTL by setting ``safety_margin`` to -1 to set watchdog timeout to ``ttl // 2``. If you need this guarantee you probably should increase ``ttl`` and/or reduce ``loop_wait`` and ``retry_timeout``.
 
 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.
+Default Patroni configuration will try to use ``/dev/watchdog`` on Linux if it is 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:
 
@@ -29,6 +33,6 @@ To enable software watchdog issue the following commands as root before starting
     # 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`.
+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.
+Patroni will log information about the watchdog when it is successfully enabled.

features/environment.py

@@ -241,8 +241,8 @@ class PatroniController(AbstractController):
             return False
         return True
 
-    def postmaster_hang(self, timeout):
+    def patroni_hang(self, timeout):
-        hang = ProcessHang(self._get_pid(), timeout)
+        hang = ProcessHang(self._handle.pid, timeout)
         self._closables.append(hang)
         hang.start()
 
@@ -519,7 +519,7 @@ class PatroniPoolController(object):
 
     def __getattr__(self, func):
         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',
+                        'get_watchdog', 'database_is_running', 'checkpoint_hang', 'patroni_hang',
                         'terminate_backends', 'backup']:
             raise AttributeError("PatroniPoolController instance has no attribute '{0}'".format(func))
 

features/steps/watchdog.py

@@ -55,8 +55,8 @@ def checkpoint_hang(context, name, timeout):
 
 
 @step('{name:w} hangs for {timeout:d} seconds')
-def postmaster_hang(context, name, timeout):
+def patroni_hang(context, name, timeout):
-    return context.pctl.postmaster_hang(name, timeout)
+    return context.pctl.patroni_hang(name, timeout)
 
 
 @step('I terminate {name:w} user processes')

features/watchdog.feature

@@ -9,35 +9,11 @@ Feature: watchdog
     When I shut down postgres0
     Then postgres0 watchdog has been closed
 
-  Scenario: watchdog is updated during pause
+  #TODO: test watchdog is disabled during pause
-    Given I start postgres0 with watchdog
+  #TODO: test watchdog is disabled properly when shutting down
-    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
+  Scenario: watchdog is triggered if patroni stops responding
     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

@@ -23,11 +23,11 @@ class Patroni(object):
         self.version = __version__
         self.config = Config()
         self.dcs = get_dcs(self.config)
+        self.watchdog = Watchdog(self.config)
         self.load_dynamic_configuration()
 
         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()
@@ -42,6 +42,7 @@ class Patroni(object):
                 if cluster and cluster.config:
                     if self.config.set_dynamic_configuration(cluster.config):
                         self.dcs.reload_config(self.config)
+                        self.watchdog.reload_config(self.config)
                 elif not self.config.dynamic_configuration and 'bootstrap' in self.config:
                     if self.config.set_dynamic_configuration(self.config['bootstrap']['dcs']):
                         self.dcs.reload_config(self.config)
@@ -65,6 +66,7 @@ class Patroni(object):
         try:
             self.tags = self.get_tags()
             self.dcs.reload_config(self.config)
+            self.watchdog.reload_config(self.config)
             self.api.reload_config(self.config['restapi'])
             self.postgresql.reload_config(self.config['postgresql'])
         except Exception:
@@ -100,7 +102,6 @@ class Patroni(object):
             self.next_run = time.time()
 
     def run(self):
-        self.ha.start()
         self.api.start()
         self.next_run = time.time()
 

patroni/api.py

@@ -68,6 +68,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
             response['scheduled_restart'] = patroni.scheduled_restart.copy()
             del response['scheduled_restart']['postmaster_start_time']
             response['scheduled_restart']['schedule'] = (response['scheduled_restart']['schedule']).isoformat()
+        if not patroni.ha.watchdog.is_healthy:
+            response['watchdog_failed'] = True
         self._write_json_response(status_code, response)
 
     def do_GET(self, write_status_code_only=False):

patroni/ha.py

@@ -12,13 +12,13 @@ from multiprocessing.pool import ThreadPool
 from patroni.async_executor import AsyncExecutor, CriticalTask
 from patroni.exceptions import DCSError, PostgresConnectionException, PatroniException
 from patroni.postgresql import ACTION_ON_START
-from patroni.utils import polling_loop, null_context, tzutc
+from patroni.utils import polling_loop, tzutc
-from threading import RLock, Event, Thread
+from threading import RLock
 
 logger = logging.getLogger(__name__)
 
 
-class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,wal_position,tags')):
+class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,wal_position,tags,watchdog_failed')):
     """Node status distilled from API response:
 
         member - dcs.Member object of the node
@@ -31,11 +31,11 @@ class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,wa
     def from_api_response(cls, member, json):
         is_master = json['role'] == 'master'
         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', {}))
+        return cls(member, True, not is_master, wal, json.get('tags', {}), json.get('watchdog_failed', False))
 
     @classmethod
     def unknown(cls, member):
-        return cls(member, False, None, 0, {})
+        return cls(member, False, None, 0, {}, False)
 
     def failover_limitation(self):
         """Returns reason why this node can't promote or None if everything is ok."""
@@ -43,56 +43,11 @@ class _MemberStatus(namedtuple('_MemberStatus', 'member,reachable,in_recovery,wa
             return 'not reachable'
         if self.tags.get('nofailover', False):
             return 'not allowed to promote'
+        if self.watchdog_failed:
+            return 'not watchdog capable'
         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):
@@ -113,10 +68,6 @@ 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()
@@ -130,15 +81,12 @@ class Ha(object):
         self.cluster = cluster
 
     def acquire_lock(self):
-        ret = self.dcs.attempt_to_acquire_leader()
+        return 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:
-            self.keepalive()
+            self.watchdog.keepalive()
             if write_leader_optime:
                 try:
                     self.dcs.write_leader_optime(self.state_handler.last_operation())
@@ -227,6 +175,9 @@ class Ha(object):
             return True
 
     def recover(self):
+        # Postgres is not running and we will restart in standby mode. Watchdog is not needed until we promote.
+        self.watchdog.disable()
+
         if self.has_lock() and self.update_lock():
             timeout = self.patroni.config['master_start_timeout']
             if timeout == 0:
@@ -277,7 +228,6 @@ class Ha(object):
         node_to_follow = self._get_node_to_follow(self.cluster)
 
         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:
@@ -287,8 +237,6 @@ class Ha(object):
         elif is_leader:
             self.demote('immediate-nolock')
             return demote_reason
-        else:
-            self.keepalive()
 
         if self._handle_rewind():
             return self._async_executor.scheduled_action
@@ -392,6 +340,15 @@ class Ha(object):
                 self._disable_sync -= 1
 
     def enforce_master_role(self, message, promote_message):
+        if not self.watchdog.is_running:
+            if not self.watchdog.activate():
+                if self.state_handler.is_leader():
+                    self.demote('immediate')
+                    return 'Demoting self because watchdog could not be activated'
+                else:
+                    self.release_leader_key_voluntarily()
+                    return 'Not promoting self because watchdog could not be actived'
+
         if self.state_handler.is_leader() or self.state_handler.role == 'master':
             # Inform the state handler about its master role.
             # It may be unaware of it if postgres is promoted manually.
@@ -545,6 +502,9 @@ class Ha(object):
         if self.cluster.failover:
             return self.manual_failover_process_no_leader()
 
+        if not self.watchdog.is_healthy:
+            return False
+
         # When in sync mode, only last known master and sync standby are allowed to promote automatically.
         all_known_members = self.cluster.members + self.old_cluster.members
         if self.is_synchronous_mode() and self.cluster.sync.leader:
@@ -582,9 +542,9 @@ class Ha(object):
             '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.stop(mode_control['stop'], checkpoint=mode_control['checkpoint'],
+                                on_safepoint=self.watchdog.disable if self.watchdog.is_running else None)
         self.state_handler.set_role('demoted')
 
         if mode_control['release']:
@@ -607,13 +567,6 @@ class Ha(object):
                 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:
-            return null_context()
-
     def should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn):
         if scheduled_at and not self.is_paused():
             # If the scheduled action is in the far future, we shouldn't do anything and just return.
@@ -720,8 +673,6 @@ class Ha(object):
     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...'
 
@@ -886,7 +837,6 @@ class Ha(object):
         self._async_executor.run_async(self._do_reinitialize, args=(self.cluster, ))
 
     def handle_long_action_in_progress(self):
-        try:
         if self.has_lock() and self.update_lock():
             return 'updated leader lock during ' + self._async_executor.scheduled_action
         elif not self.state_handler.bootstrapping:
@@ -901,8 +851,6 @@ class Ha(object):
                             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')
@@ -918,7 +866,7 @@ class Ha(object):
 
     def post_recover(self):
         if not self.state_handler.is_running():
-            self.keepalive()
+            self.watchdog.disable()
             if self.has_lock():
                 self.state_handler.set_role('demoted')
                 self.dcs.delete_leader()
@@ -939,8 +887,6 @@ class Ha(object):
         if not self.state_handler.is_running() or self._post_bootstrap_task.result is False:
             self.cancel_initialization()
 
-        self.keepalive()
-
         if self._post_bootstrap_task.result is None:
             if not self.state_handler.is_leader():
                 return 'waiting for end of recovery after bootstrap'
@@ -953,6 +899,9 @@ class Ha(object):
 
         self.state_handler.bootstrapping = False
         self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration, separators=(',', ':')))
+        if not self.watchdog.activate():
+            logger.error('Cancelling bootstrap because watchdog activation failed')
+            self.cancel_initialization()
         self.dcs.take_leader()
         self.state_handler.call_nowait(ACTION_ON_START)
         self.load_cluster_from_dcs()
@@ -1001,17 +950,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()
 
+            if self.is_paused():
+                self.watchdog.disable()
+
             if not self.cluster.has_member(self.state_handler.name):
                 self.touch_member()
 
@@ -1043,9 +989,8 @@ 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.
+                # In case datadir went away while we were master. TODO: check for this and try to stop postgresql.
-                # TODO: detect the datadir going away (e.g. unmounted ) while PostgreSQL is running
+                self.watchdog.disable()
-                self.keepalive()
 
                 # is this instance the leader?
                 if self.has_lock():
@@ -1064,8 +1009,6 @@ 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()
@@ -1106,17 +1049,12 @@ 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')
@@ -1126,11 +1064,10 @@ class Ha(object):
             # 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):
+            disable_wd = self.watchdog.disable if self.watchdog.is_running else None
-                self.while_not_sync_standby(lambda: self.state_handler.stop(checkpoint=False))
+            self.while_not_sync_standby(lambda: self.state_handler.stop(checkpoint=False, on_safepoint=disable_wd))
             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

patroni/postgresql.py

@@ -20,7 +20,7 @@ from patroni.exceptions import PostgresConnectionException
 from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError, polling_loop, null_context
 from six import string_types
 from six.moves.urllib.parse import quote_plus
-from threading import current_thread, Lock, Event
+from threading import current_thread, Lock
 
 logger = logging.getLogger(__name__)
 
@@ -153,12 +153,6 @@ 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')
@@ -840,8 +834,6 @@ class Postgresql(object):
         options = ['--{0}={1}'.format(p, self._server_parameters[p]) for p in self.CMDLINE_OPTIONS
                    if p in self._server_parameters and p != 'wal_keep_segments']
 
-        start_initiated = time.time()
-
         # Unfortunately `pg_ctl start` does not return postmaster pid to us. Without this information
         # it is hard to know the current state of postgres startup, so we had to reimplement pg_ctl start
         # in python. It will start postgres, wait for port to be open and wait until postgres will start
@@ -906,10 +898,17 @@ class Postgresql(object):
             logging.exception('Exception during CHECKPOINT')
             return 'not accessible or not healty'
 
-    def stop(self, mode='fast', block_callbacks=False, checkpoint=True):
+    def stop(self, mode='fast', block_callbacks=False, checkpoint=True, on_safepoint=None):
-        success, pg_signaled = self._do_stop(mode, block_callbacks, checkpoint)
+        """Stop PostgreSQL
+
+        Supports a callback when a safepoint is reached. A safepoint is when no user backend can return a successful
+        commit to users. Currently this means we wait for user backends to close. But in the future alternate mechanisms
+        could be added.
+
+        :param on_safepoint: This callback is called when no user backends are running.
+        """
+        success, pg_signaled = self._do_stop(mode, block_callbacks, checkpoint, on_safepoint)
         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:
@@ -921,8 +920,10 @@ class Postgresql(object):
             self.set_state('stop failed')
         return success
 
-    def _do_stop(self, mode, block_callbacks, checkpoint):
+    def _do_stop(self, mode, block_callbacks, checkpoint, on_safepoint):
         if not self.is_running():
+            if on_safepoint:
+                on_safepoint()
             return True, False
 
         if checkpoint and not self.is_starting():
@@ -934,14 +935,16 @@ class Postgresql(object):
         # Send signal to postmaster to stop
         pid, result = self._signal_postmaster_stop(mode)
         if result is not None:
+            if result and on_safepoint:
+                on_safepoint()
             return result, True
 
-        # We can skip safepoint detection if nobody is waiting for it.
+        # We can skip safepoint detection if we don't have a callback
-        if not self.stop_safepoint_reached.is_set():
+        if on_safepoint:
             # 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()
+            on_safepoint()
 
         self._wait_for_postmaster_stop(pid)
 

patroni/watchdog/base.py

@@ -3,6 +3,7 @@ import logging
 import platform
 import six
 import sys
+from threading import RLock
 
 from patroni.exceptions import WatchdogError
 
@@ -29,32 +30,102 @@ def parse_mode(mode):
         return MODE_OFF
 
 
-class Watchdog(object):
+def synchronized(func):
-    """Facade to dynamically manage watchdog implementations and handle config changes."""
+    def wrapped(self, *args, **kwargs):
+        with self._lock:
+            return func(self, *args, **kwargs)
+    return wrapped
+
+
+class WatchdogConfig(object):
+    """Helper to contain a snapshot of configuration"""
     def __init__(self, config):
+        self.mode = parse_mode(config['watchdog'].get('mode', 'automatic'))
         self.ttl = config['ttl']
         self.loop_wait = config['loop_wait']
-        self.mode = parse_mode(config['watchdog'].get('mode', 'automatic'))
+        self.safety_margin = config['watchdog'].get('safety_margin', 5)
-        self.driver = config['watchdog'].get('driver')
+        self.driver = config['watchdog'].get('driver', 'default')
-        self.config = config
+        self.driver_config = dict((k, v) for k, v in config['watchdog'].items()
+                                  if k not in ['mode', 'safety_margin', 'driver'])
 
-        if self.mode == MODE_OFF:
+    def __eq__(self, other):
+        return isinstance(other, WatchdogConfig) and \
+            all(getattr(self, attr) == getattr(other, attr) for attr in
+                ['mode', 'ttl', 'loop_wait', 'safety_margin', 'driver', 'driver_config'])
+
+    def get_impl(self):
+        if self.driver == 'testing':
+            from patroni.watchdog.linux import TestingWatchdogDevice
+            return TestingWatchdogDevice.from_config(self.driver_config)
+        elif platform.system() == 'Linux' and self.driver == 'default':
+            from patroni.watchdog.linux import LinuxWatchdogDevice
+            return LinuxWatchdogDevice.from_config(self.driver_config)
+        else:
+            return NullWatchdog()
+
+    @property
+    def timeout(self):
+        if self.safety_margin == -1:
+            return int(self.ttl // 2)
+        else:
+            return self.ttl - self.safety_margin
+
+    @property
+    def timing_slack(self):
+        return self.timeout - self.loop_wait
+
+
+class Watchdog(object):
+    """Facade to dynamically manage watchdog implementations and handle config changes.
+
+    When activation fails underlying implementation will be switched to a Null implementation. To avoid log spam
+    activation will only be retried when watchdog configuration is changed."""
+    def __init__(self, config):
+        self.active_config = self.config = WatchdogConfig(config)
+        self._lock = RLock()
+        self.active = False
+
+        if self.config.mode == MODE_OFF:
             self.impl = NullWatchdog()
         else:
-            self.impl = self._get_impl()
+            self.impl = self.config.get_impl()
-            if self.mode == MODE_REQUIRED and isinstance(self.impl, NullWatchdog):
+            if self.config.mode == MODE_REQUIRED and self.impl.is_null:
                 logger.error("Configuration requires a watchdog, but watchdog is not supported on this platform.")
                 sys.exit(1)
 
+    @synchronized
+    def reload_config(self, config):
+        self.config = WatchdogConfig(config)
+        # Turning a watchdog off can always be done immediately
+        if self.config.mode == MODE_OFF:
+            if self.active:
+                self._disable()
+            self.active_config = self.config
+            self.impl = NullWatchdog()
+        # If watchdog is not active we can apply config immediately to show any warnings early. Otherwise we need to
+        # delay until next time a keepalive is sent so timeout matches up with leader key update.
+        if not self.active:
+            if self.config.driver != self.active_config.driver or \
+               self.config.driver_config != self.active_config.driver_config:
+                self.impl = self.config.get_impl()
+            self.active_config = self.config
+
+    @synchronized
     def activate(self):
         """Activates the watchdog device with suitable timeouts. While watchdog is active keepalive needs
         to be called every time loop_wait expires.
+
+        :returns False if a safe watchdog could not be configured, but is required.
         """
-        desired_timeout = int(self.ttl // 2)
+        self.active = True
-        slack = desired_timeout - self.loop_wait
+        return self._activate()
-        if slack < 0:
+
+    def _activate(self):
+        self.active_config = self.config
+
+        if self.config.timing_slack < 0:
             logger.warning('Watchdog not supported because leader TTL {0} is less than 2x loop_wait {1}'
-                           .format(self.ttl, self.loop_wait))
+                           .format(self.config.ttl, self.config.loop_wait))
             self.impl = NullWatchdog()
 
         try:
@@ -65,40 +136,56 @@ class Watchdog(object):
 
         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.")
+                           " Watchdog will trigger after Patroni loses leader key.")
 
+        actual_timeout = self._set_timeout()
+
+        if not self.impl.is_running or actual_timeout > self.config.timeout:
+            if self.config.mode == MODE_REQUIRED:
+                if self.impl.is_null:
+                    logger.error("Configuration requires watchdog, but watchdog could not be configured.")
+                else:
+                    logger.error("Configuration requires watchdog, but a safe watchdog timeout {0} could"
+                                 " not be configured. Watchdog timeout is {1}.".format(
+                                     self.config.timeout, actual_timeout))
+                return False
+            else:
+                if not self.impl.is_null:
+                    logger.warning("Watchdog timeout {0} seconds does not ensure safe termination within {1} seconds"
+                                   .format(actual_timeout, self.config.timeout))
+
+        if self.is_running:
+            logger.info("{0} activated with {1} second timeout, timing slack {2} seconds"
+                        .format(self.impl.describe(), actual_timeout, self.config.timing_slack))
+        else:
+            if self.config.mode == MODE_REQUIRED:
+                logger.error("Configuration requires watchdog, but watchdog could not be activated")
+                return False
+
+        return True
+
+    def _set_timeout(self):
         if self.impl.has_set_timeout():
-            self.impl.set_timeout(desired_timeout)
+            self.impl.set_timeout(self.config.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:
+        if self.impl.is_running and actual_timeout < self.config.loop_wait:
             logger.error('loop_wait of {0} seconds is too long for watchdog {1} second timeout'
-                         .format(self.loop_wait, actual_timeout))
+                         .format(self.config.loop_wait, actual_timeout))
             if self.impl.can_be_disabled:
                 logger.info('Disabling watchdog due to unsafe timeout.')
                 self.impl.close()
                 self.impl = NullWatchdog()
+                return None
+        return actual_timeout
 
-        if not self.impl.is_running or actual_timeout > desired_timeout:
+    @synchronized
-            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:  # XXX: can we really get here?
-                logger.error("Configuration requires watchdog, but watchdog could not be activated")
-                sys.exit(1)
-
     def disable(self):
+        self._disable()
+        self.active = False
+
+    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.
@@ -109,40 +196,54 @@ class Watchdog(object):
         except WatchdogError as e:
             logger.error("Error while disabling watchdog: %s", e)
 
+    @synchronized
     def keepalive(self):
         try:
             self.impl.keepalive()
+            # In case there are any pending configuration changes apply them now.
+            if self.active and self.config != self.active_config:
+                if self.config.mode != MODE_OFF and self.active_config.mode == MODE_OFF:
+                    self.impl = self.config.get_impl()
+                    self._activate()
+                if self.config.driver != self.active_config.driver \
+                   or self.config.driver_config != self.active_config.driver_config:
+                    self._disable()
+                    self.impl = self.config.get_impl()
+                    self._activate()
+                if self.config.timeout != self.active_config.timeout:
+                    self.impl.set_timeout(self.config.timeout)
         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]:  # XXX: can't be reached
-            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
+    @synchronized
     def is_running(self):
         return self.impl.is_running
 
+    @property
+    @synchronized
+    def is_healthy(self):
+        if self.config.mode != MODE_REQUIRED:
+            return True
+        return self.config.timing_slack >= 0 and self.impl.is_healthy
+
 
 @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."""
+    is_null = False
 
     @property
     def is_running(self):
         """Returns True when watchdog is activated and capable of performing it's task."""
         return False
 
+    @property
+    def is_healthy(self):
+        """Returns False when calling open() is known to fail."""
+        return False
+
     @property
     def can_be_disabled(self):
         """Returns True when watchdog will be disabled by calling close(). Some watchdog devices
@@ -193,6 +294,8 @@ class WatchdogBase(object):
 
 class NullWatchdog(WatchdogBase):
     """Null implementation when watchdog is not supported."""
+    is_null = True
+
     def open(self):
         return
 

patroni/watchdog/linux.py

@@ -132,6 +132,10 @@ class LinuxWatchdogDevice(WatchdogBase):
     def is_running(self):
         return self._fd is not None
 
+    @property
+    def is_healthy(self):
+        return os.path.exists(self.device) and os.access(self.device, os.W_OK)
+
     def open(self):
         try:
             self._fd = os.open(self.device, os.O_WRONLY)

postgres0.yml

@@ -81,6 +81,7 @@ postgresql:
 #watchdog:
 #  mode: automatic # Allowed values: off, automatic, required
 #  device: /dev/watchdog
+#  safety_margin: 5
 
 tags:
     nofailover: false

tests/test_api.py

@@ -37,9 +37,14 @@ class MockPostgresql(object):
         return str(postmaster_start_time)
 
 
+class MockWatchdog(object):
+    is_healthy = True
+
+
 class MockHa(object):
 
     state_handler = MockPostgresql()
+    watchdog = MockWatchdog()
 
     @staticmethod
     def reinitialize():

tests/test_ha.py

@@ -9,7 +9,7 @@ from patroni.config import Config
 from patroni.dcs import Cluster, ClusterConfig, Failover, Leader, Member, get_dcs, SyncState
 from patroni.dcs.etcd import Client
 from patroni.exceptions import DCSError, PostgresConnectionException, PatroniException
-from patroni.ha import Ha, _MemberStatus, BackgroundKeepaliveSender
+from patroni.ha import Ha, _MemberStatus
 from patroni.postgresql import Postgresql
 from patroni.watchdog import Watchdog
 from patroni.utils import tzutc
@@ -62,7 +62,7 @@ def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailove
         tags = {}
         if nofailover:
             tags['nofailover'] = True
-        return _MemberStatus(e, reachable, in_recovery, wal_position, tags)
+        return _MemberStatus(e, reachable, in_recovery, wal_position, tags, False)
     return fetch_node_status
 
 future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5)
@@ -241,6 +241,15 @@ class TestHa(unittest.TestCase):
         self.p.is_leader = false
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock')
 
+    def test_promote_without_watchdog(self):
+        self.ha.cluster.is_unlocked = false
+        self.ha.has_lock = true
+        self.p.is_leader = true
+        with patch.object(Watchdog, 'activate', Mock(return_value=False)):
+            self.assertEquals(self.ha.run_cycle(), 'Demoting self because watchdog could not be activated')
+            self.p.is_leader = false
+            self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be actived')
+
     def test_leader_with_lock(self):
         self.ha.cluster.is_unlocked = false
         self.ha.has_lock = true
@@ -823,16 +832,3 @@ class TestHa(unittest.TestCase):
         self.ha.has_lock = false
         # will not say bootstrap from leader as replica can't self elect
         self.assertEquals(self.ha.run_cycle(), "trying to bootstrap from replica 'other'")
-
-
-class TestBackgroundKeepaliveSender(unittest.TestCase):
-
-    def test_run(self):
-        safe_event = Event()
-        ha = Mock()
-        ha.dcs.loop_wait = 0.1
-        with BackgroundKeepaliveSender(ha, safe_event):
-            time.sleep(1)
-            safe_event.set()
-            time.sleep(1)
-        self.assertTrue(ha.keepalive.call_count > 2)

tests/test_postgresql.py

@@ -275,16 +275,15 @@ class TestPostgresql(unittest.TestCase):
     def test_stop(self, mock_get_pid, mock_is_running):
         mock_is_running.return_value = True
         mock_get_pid.return_value = 0
-        self.assertTrue(self.p.stop())
+        mock_callback = Mock()
+        self.assertTrue(self.p.stop(on_safepoint=mock_callback))
+        mock_callback.assert_called()
         mock_get_pid.return_value = -1
         self.assertFalse(self.p.stop())
         mock_get_pid.return_value = 123
         with patch('os.kill', Mock(side_effect=[OSError(errno.ESRCH, ''), OSError, None])),\
                 patch('psutil.Process', Mock(side_effect=psutil.NoSuchProcess(123))):
             self.assertTrue(self.p.stop())
-            self.assertFalse(self.p.stop())
-            self.p.stop_safepoint_reached.clear()
-            self.assertTrue(self.p.stop())
         with patch.object(Postgresql, '_signal_postmaster_stop', Mock(return_value=(123, None))):
             with patch.object(Postgresql, 'is_pid_running', Mock(side_effect=[True, False, False])):
                 self.assertTrue(self.p.stop())
@@ -855,13 +854,12 @@ class TestPostgresql(unittest.TestCase):
     @patch.object(Postgresql, 'is_pid_running')
     def test__wait_for_connection_close(self, mock_is_pid_running):
         mock_is_pid_running.side_effect = [True, False, False]
-        self.p.stop_safepoint_reached.clear()
+        mock_callback = Mock()
-        self.p.stop()
+        self.p.stop(on_safepoint=mock_callback)
 
         mock_is_pid_running.side_effect = [True, False, False]
-        self.p.stop_safepoint_reached.clear()
         with patch.object(MockCursor, "execute", Mock(side_effect=psycopg2.Error)):
-            self.p.stop()
+            self.p.stop(on_safepoint=mock_callback)
 
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
     @patch.object(Postgresql, '_signal_postmaster_stop', Mock(return_value=(123, None)))
@@ -872,8 +870,8 @@ class TestPostgresql(unittest.TestCase):
         child = Mock()
         child.cmdline.return_value = ['foo']
         mock_psutil.return_value.children.return_value = [child]
-        self.p.stop_safepoint_reached.clear()
+        mock_callback = Mock()
-        self.p.stop()
+        self.p.stop(on_safepoint=mock_callback)
 
     @patch('os.kill', Mock(side_effect=[OSError(errno.ESRCH, ''), OSError]))
     @patch('time.sleep', Mock())

tests/test_watchdog.py

@@ -72,18 +72,20 @@ class TestWatchdog(unittest.TestCase):
     @patch('platform.system', Mock(return_value='Linux'))
     @patch.object(LinuxWatchdogDevice, 'can_be_disabled', PropertyMock(return_value=True))
     def test_unsafe_timeout_disable_watchdog_and_exit(self):
-        self.assertRaises(SystemExit, Watchdog({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required'}}).activate)
+        watchdog = Watchdog({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required', 'safety_margin': -1}})
+        self.assertEquals(watchdog.activate(), False)
+        self.assertEquals(watchdog.is_running, False)
 
     @patch('platform.system', Mock(return_value='Linux'))
     @patch.object(LinuxWatchdogDevice, 'get_timeout', Mock(return_value=16))
     def test_timeout_does_not_ensure_safe_termination(self):
-        Watchdog({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'auto'}}).activate()
+        Watchdog({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'auto', 'safety_margin': -1}}).activate()
         self.assertEquals(len(mock_devices), 2)
 
     @patch('platform.system', Mock(return_value='Linux'))
     @patch.object(Watchdog, 'is_running', PropertyMock(return_value=False))
     def test_watchdog_not_activated(self):
-        self.assertRaises(SystemExit, Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}}).activate)
+        self.assertEquals(Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}}).activate(), False)
 
     @patch('platform.system', Mock(return_value='Linux'))
     def test_basic_operation(self):
@@ -94,7 +96,7 @@ class TestWatchdog(unittest.TestCase):
         device = mock_devices[-1]
         self.assertTrue(device.open)
 
-        self.assertEquals(device.timeout, 14)
+        self.assertEquals(device.timeout, 24)
 
         watchdog.keepalive()
         self.assertEquals(len(device.writes), 1)
@@ -104,7 +106,7 @@ class TestWatchdog(unittest.TestCase):
         self.assertEquals(device.writes[-1], b'V')
 
     def test_invalid_timings(self):
-        watchdog = Watchdog({'ttl': 30, 'loop_wait': 20, 'watchdog': {'mode': 'automatic'}})
+        watchdog = Watchdog({'ttl': 30, 'loop_wait': 20, 'watchdog': {'mode': 'automatic', 'safety_margin': -1}})
         watchdog.activate()
         self.assertEquals(len(mock_devices), 1)
         self.assertFalse(watchdog.is_running)
@@ -112,12 +114,12 @@ class TestWatchdog(unittest.TestCase):
     def test_parse_mode(self):
         with patch('patroni.watchdog.base.logger.warning', new_callable=Mock()) as warning_mock:
             watchdog = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'bad'}})
-            self.assertEquals(watchdog.mode, 'off')
+            self.assertEquals(watchdog.config.mode, 'off')
             warning_mock.assert_called_once()
 
     @patch('platform.system', Mock(return_value='Unknown'))
     def test_unsupported_platform(self):
-        self.assertRaises(SystemExit, Watchdog, {'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}})
+        self.assertRaises(SystemExit, Watchdog, {'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required', 'driver': 'bad'}})
 
     def test_exceptions(self):
         wd = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'bad'}})
@@ -125,6 +127,31 @@ class TestWatchdog(unittest.TestCase):
         self.assertIsNone(wd.disable())
         self.assertIsNone(wd.keepalive())
 
+    def test_config_reload(self):
+        watchdog = Watchdog({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required'}})
+        self.assertTrue(watchdog.activate())
+        self.assertTrue(watchdog.is_running)
+
+        watchdog.reload_config({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'off'}})
+        self.assertFalse(watchdog.is_running)
+
+        watchdog.reload_config({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required'}})
+        self.assertFalse(watchdog.is_running)
+        watchdog.keepalive()
+        self.assertTrue(watchdog.is_running)
+
+        watchdog.disable()
+        watchdog.reload_config({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required', 'driver': 'unknown'}})
+        self.assertFalse(watchdog.is_healthy)
+
+        self.assertFalse(watchdog.activate())
+        watchdog.reload_config({'ttl': 30, 'loop_wait': 15, 'watchdog': {'mode': 'required'}})
+        self.assertFalse(watchdog.is_running)
+        watchdog.keepalive()
+        self.assertTrue(watchdog.is_running)
+
+        watchdog.reload_config({'ttl': 60, 'loop_wait': 15, 'watchdog': {'mode': 'required'}})
+        watchdog.keepalive()
 
 class TestNullWatchdog(unittest.TestCase):