From 568eb730bcd3c80e1a7746f577b00f960d057201 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 24 Jun 2016 17:39:04 +0200 Subject: [PATCH] Clear the scheduled restart after the normal one. Make sure the scheduled restart flag is cleared when the postmaster_start_time changes since the time restart was scheduled. Additionally, separate the logic of checking the restart conditions into the function in order to support conditions for the normal restart as well. --- patroni/api.py | 2 ++ patroni/ha.py | 50 ++++++++++++++++++++++++++++--------------- patroni/postgresql.py | 7 ++++++ tests/test_api.py | 7 +++++- tests/test_ha.py | 5 ++++- 5 files changed, 52 insertions(+), 19 deletions(-) diff --git a/patroni/api.py b/patroni/api.py index b91eea30..3fbab534 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -66,6 +66,7 @@ class RestApiHandler(BaseHTTPRequestHandler): response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope} if patroni.scheduled_restart and isinstance(patroni.scheduled_restart, dict): response['scheduled_restart'] = patroni.scheduled_restart.copy() + del response['scheduled_restart']['postmaster_start_time'] response['scheduled_restart']['schedule'] = (response['scheduled_restart']['schedule']).isoformat() self._write_json_response(status_code, response) @@ -227,6 +228,7 @@ class RestApiHandler(BaseHTTPRequestHandler): data = "Schedule required for the scheduled restart" status_code = 400 else: + request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time() if self.server.patroni.ha.schedule_future_restart(request): data = "Restart scheduled" status_code = 202 diff --git a/patroni/ha.py b/patroni/ha.py index 8c282c95..6ac09533 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -388,33 +388,49 @@ class Ha(object): def evaluate_scheduled_restart(self): # restart if we need to restart_data = self.future_restart_scheduled() + if restart_data: + recent_time = self.state_handler.postmaster_start_time() + request_time = restart_data['postmaster_start_time'] + # check if postmaster start time has changed since the last restart + if recent_time and request_time and recent_time != request_time: + logger.info("Cancelling scheduled restart: postgres restart has already happened at {0}". + format(recent_time)) + self.delete_future_restart() + return + if (restart_data and self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)): try: - reason_to_cancel = "" - # checking the restart filters here seem to be less ugly than moving them into the - # run_scheduled_action. - if restart_data.get('role') and restart_data['role'] != self.state_handler.role: - reason_to_cancel = "host role mismatch" - - if (restart_data.get('postgres_version') and - self.state_hander.postgres_version_to_int(restart_data['postgres_version']) <= - int(self.state_hander.server_version)): - reason_to_cancel = "postgres version mismatch" - - if 'with_pending_restart_flag' in restart_data and not self.state_handler.pending_restart: - reason_to_cancel = "pending restart flag is not set" - - if not reason_to_cancel: + if self.scheduled_restart_matches(restart_data.get('role'), + restart_data.get('postgres_version'), + ('with_pending_restart_flag' in restart_data)): if self.state_handler.restart(): logger.info("Scheduled restart successfull") else: logger.warning("Scheduled restart failed") - else: - logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel)) finally: self.delete_future_restart() + def scheduled_restart_matches(self, role, postgres_version, pending_restart): + reason_to_cancel = "" + # checking the restart filters here seem to be less ugly than moving them into the + # run_scheduled_action. + if role and role != self.state_handler.role: + reason_to_cancel = "host role mismatch" + + if (postgres_version and + self.state_hander.postgres_version_to_int(postgres_version) <= int(self.state_hander.server_version)): + reason_to_cancel = "postgres version mismatch" + + if pending_restart and not self.state_handler.pending_restart: + reason_to_cancel = "pending restart flag is not set" + + if not reason_to_cancel: + return True + else: + logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel)) + return False + def schedule(self, action): with self._async_executor: return self._async_executor.schedule(action) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 8951e4c3..1120f842 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -806,6 +806,13 @@ $$""".format(name, ' '.join(options)), name, password, password) self._replication_slots = [r[0] for r in cursor] self._schedule_load_slots = False + def postmaster_start_time(self): + try: + cursor = self.query("""SELECT to_char(pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ')""") + return cursor.fetchone()[0] + except psycopg2.Error: + return None + def sync_replication_slots(self, cluster): if self.use_slots: try: diff --git a/tests/test_api.py b/tests/test_api.py index 6b9f93de..e50b58bb 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -26,6 +26,10 @@ class MockPostgresql(object): def connection(): return psycopg2_connect() + @staticmethod + def postmaster_start_time(): + return '2016-08-20 12:00TZ+1' + class MockHa(object): @@ -67,7 +71,8 @@ class MockPatroni(object): tags = {} version = '0.00' noloadbalance = Mock(return_value=False) - scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')} + scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'), + 'postmaster_start_time': '2016-08-20 12:00TZ+1'} @staticmethod def sighup_handler(): diff --git a/tests/test_ha.py b/tests/test_ha.py index 71e7087f..b8386fee 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -81,7 +81,8 @@ zookeeper: self.replicatefrom = None self.api.connection_string = 'http://127.0.0.1:8008' self.clonefrom = None - self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')} + self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'), + 'postmaster_start_time': '2016-08-20 12:00TZ+1'} def run_async(func, args=()): @@ -119,6 +120,7 @@ class TestHa(unittest.TestCase): 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8}}) self.p.set_state('running') self.p.set_role('replica') + self.p.postmaster_start_time = MagicMock(return_value="2016-08-20 12:00TZ+1") self.p.check_replication_lag = true self.p.can_create_replica_without_replication_connection = MagicMock(return_value=False) self.e = get_dcs({'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test', @@ -128,6 +130,7 @@ class TestHa(unittest.TestCase): self.ha.old_cluster = self.e.get_cluster() self.ha.cluster = get_cluster_not_initialized_without_leader() self.ha.load_cluster_from_dcs = Mock() + #self.ha.evaluate_scheduled_restart = true def test_update_lock(self): self.p.last_operation = Mock(side_effect=PostgresException(''))