From 70d718a058a30f7b6fae61bc1bada960e72b8225 Mon Sep 17 00:00:00 2001 From: Ants Aasma Date: Thu, 27 Jul 2017 13:16:11 +0300 Subject: [PATCH] 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 | 7 ++ docs/watchdog.rst | 14 ++- features/environment.py | 6 +- features/steps/watchdog.py | 4 +- features/watchdog.feature | 30 +----- patroni/__init__.py | 5 +- patroni/api.py | 2 + patroni/ha.py | 205 +++++++++++++------------------------ patroni/postgresql.py | 35 ++++--- patroni/watchdog/base.py | 197 ++++++++++++++++++++++++++--------- patroni/watchdog/linux.py | 4 + postgres0.yml | 1 + tests/test_api.py | 5 + tests/test_ha.py | 26 ++--- tests/test_postgresql.py | 18 ++-- tests/test_watchdog.py | 41 ++++++-- 16 files changed, 332 insertions(+), 268 deletions(-) diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 84265de9..5e6ef774 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -15,6 +15,7 @@ Bootstrap configuration - **dcs**: This section will be written into `///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 `__ 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. diff --git a/docs/watchdog.rst b/docs/watchdog.rst index 5abcf959..8ace0a19 100644 --- a/docs/watchdog.rst +++ b/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. \ No newline at end of file +Patroni will log information about the watchdog when it is successfully enabled. diff --git a/features/environment.py b/features/environment.py index 87a5cef9..3f622e49 100644 --- a/features/environment.py +++ b/features/environment.py @@ -241,8 +241,8 @@ class PatroniController(AbstractController): return False return True - def postmaster_hang(self, timeout): - hang = ProcessHang(self._get_pid(), timeout) + def patroni_hang(self, 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)) diff --git a/features/steps/watchdog.py b/features/steps/watchdog.py index acd3e9b4..dda54441 100644 --- a/features/steps/watchdog.py +++ b/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): - return context.pctl.postmaster_hang(name, timeout) +def patroni_hang(context, name, timeout): + return context.pctl.patroni_hang(name, timeout) @step('I terminate {name:w} user processes') diff --git a/features/watchdog.feature b/features/watchdog.feature index a0ce85c3..00f4ddef 100644 --- a/features/watchdog.feature +++ b/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 - 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 + #TODO: test watchdog is disabled during pause + #TODO: test watchdog is disabled properly when shutting down - 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 diff --git a/patroni/__init__.py b/patroni/__init__.py index f6d8c632..a7064ab2 100644 --- a/patroni/__init__.py +++ b/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() diff --git a/patroni/api.py b/patroni/api.py index e8381e8b..327cf779 100644 --- a/patroni/api.py +++ b/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): diff --git a/patroni/ha.py b/patroni/ha.py index 999589a1..8d27b6b5 100644 --- a/patroni/ha.py +++ b/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 threading import RLock, Event, Thread +from patroni.utils import polling_loop, tzutc +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() - if ret: - self.keepalive() - return ret + return self.dcs.attempt_to_acquire_leader() 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,37 +542,30 @@ 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.set_role('demoted') + self.state_handler.trigger_check_diverged_lsn() + 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']: - 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: - 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) + 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 null_context() + 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 should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn): if scheduled_at and not self.is_paused(): @@ -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,23 +837,20 @@ 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: - # 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.has_lock() and self.update_lock(): + return 'updated leader lock during ' + self._async_executor.scheduled_action + elif not self.state_handler.bootstrapping: + # 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 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. - # TODO: detect the datadir going away (e.g. unmounted ) while PostgreSQL is running - self.keepalive() + # In case datadir went away while we were master. TODO: check for this and try to stop postgresql. + self.watchdog.disable() # 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): - self.while_not_sync_standby(lambda: self.state_handler.stop(checkpoint=False)) + disable_wd = self.watchdog.disable if self.watchdog.is_running else None + 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 diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 61868d7d..abd2b3f2 100644 --- a/patroni/postgresql.py +++ b/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): - success, pg_signaled = self._do_stop(mode, block_callbacks, checkpoint) + def stop(self, mode='fast', block_callbacks=False, checkpoint=True, on_safepoint=None): + """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. - if not self.stop_safepoint_reached.is_set(): + # We can skip safepoint detection if we don't have a callback + 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) diff --git a/patroni/watchdog/base.py b/patroni/watchdog/base.py index da65155d..2c9b7626 100644 --- a/patroni/watchdog/base.py +++ b/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): - """Facade to dynamically manage watchdog implementations and handle config changes.""" +def synchronized(func): + 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.driver = config['watchdog'].get('driver') - self.config = config + self.safety_margin = config['watchdog'].get('safety_margin', 5) + self.driver = config['watchdog'].get('driver', 'default') + 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() - if self.mode == MODE_REQUIRED and isinstance(self.impl, NullWatchdog): + self.impl = self.config.get_impl() + 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) - slack = desired_timeout - self.loop_wait - if slack < 0: + self.active = True + return self._activate() + + 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: - 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) - + @synchronized 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 diff --git a/patroni/watchdog/linux.py b/patroni/watchdog/linux.py index 92f0db0e..41c99746 100644 --- a/patroni/watchdog/linux.py +++ b/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) diff --git a/postgres0.yml b/postgres0.yml index 1cb634d3..d5425373 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -81,6 +81,7 @@ postgresql: #watchdog: # mode: automatic # Allowed values: off, automatic, required # device: /dev/watchdog +# safety_margin: 5 tags: nofailover: false diff --git a/tests/test_api.py b/tests/test_api.py index c58afbb8..02277c76 100644 --- a/tests/test_api.py +++ b/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(): diff --git a/tests/test_ha.py b/tests/test_ha.py index 992b8c96..2127dcf1 100644 --- a/tests/test_ha.py +++ b/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) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 5e613cb6..90a2836b 100644 --- a/tests/test_postgresql.py +++ b/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() - self.p.stop() + mock_callback = Mock() + 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() - self.p.stop() + mock_callback = Mock() + self.p.stop(on_safepoint=mock_callback) @patch('os.kill', Mock(side_effect=[OSError(errno.ESRCH, ''), OSError])) @patch('time.sleep', Mock()) diff --git a/tests/test_watchdog.py b/tests/test_watchdog.py index 1ffae833..236a00f2 100644 --- a/tests/test_watchdog.py +++ b/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):