From 6e9cb60fd532dbd0f45d985d0f5841403de44931 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Sep 2015 14:52:03 +0200 Subject: [PATCH 01/15] Restart and reinitialize via api POST /restart -- will restart postgres You you are restartung leader node, lock would be maintained during restart. POST /reinitialize -- will reinitialize node from the leader. It's not possible to reinitialize current leader. Command will fail when the leader is unknown. --- patroni/__init__.py | 4 +- patroni/api.py | 69 +++++++++++++++++++----- patroni/dcs.py | 11 ++-- patroni/etcd.py | 10 ++-- patroni/ha.py | 112 ++++++++++++++++++++++++++++++++++----- patroni/postgresql.py | 81 +++++++++++++++++++++------- patroni/zookeeper.py | 6 ++- tests/test_api.py | 59 ++++++++++++++++++--- tests/test_etcd.py | 5 +- tests/test_ha.py | 49 ++++++++++++++++- tests/test_postgresql.py | 32 ++++++++--- tests/test_zookeeper.py | 10 ++-- 12 files changed, 366 insertions(+), 82 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 10e70956..35f206df 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -48,8 +48,6 @@ class Patroni: logger.info('waiting on DCS') sleep(5) - self.postgresql.schedule_load_slots = self.postgresql.is_running() and self.postgresql.use_slots - def schedule_next_run(self): self.next_run += self.nap_time current_time = time.time() @@ -75,7 +73,7 @@ class Patroni: def main(): - logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO) + logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.DEBUG) logging.getLogger('requests').setLevel(logging.WARNING) setup_signal_handlers() diff --git a/patroni/api.py b/patroni/api.py index 30bc8914..ebb732a4 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -44,23 +44,23 @@ class RestApiHandler(BaseHTTPRequestHandler): def do_GET(self): """Default method for processing all GET requests which can not be routed to other methods""" + path = '/master' if self.path == '/' else self.path response = self.get_postgresql_status() - path = '/master' if self.path == '/' else self.path - status_code = 200 if response['running'] and 'role' in response and response['role'] in path else 503 + patroni = self.server.patroni + if 'role' in response and response['role'] in path: + status_code = 200 + elif patroni.ha.restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path: + # exceptional case for master node when the postgres is being restarted via API + status_code = 200 + else: + status_code = 503 self.send_response(status_code) self.send_header('Content-Type', 'application/json') self.end_headers() self.wfile.write(json.dumps(response).encode('utf-8')) - @check_auth - def do_GET_sampleauth(self): - self.send_response(200) - self.send_header('Content-Type', 'text/html') - self.end_headers() - self.wfile.write(b'Hello!') - def do_GET_patroni(self): response = self.get_postgresql_status(True) @@ -69,6 +69,51 @@ class RestApiHandler(BaseHTTPRequestHandler): self.end_headers() self.wfile.write(json.dumps(response).encode('utf-8')) + @check_auth + def do_POST_restart(self): + action = self.server.patroni.ha.schedule_restart() + if action is not None: + status_code = 503 + data = (action + ' already in progress').encode('utf-8') + else: + status_code = 503 + data = b'restart failed' + try: + if self.server.patroni.ha.restart(): + status_code = 200 + data = b'restarted successfully' + except: + logger.exception('Exception during restart') + + self.send_response(status_code) + self.send_header('Content-Type', 'text/html') + self.end_headers() + self.wfile.write(data) + + @check_auth + def do_POST_reinitialize(self): + ha = self.server.patroni.ha + cluster = ha.dcs.get_cluster() + if cluster.is_unlocked(): + status_code = 503 + data = b'Cluster has no leader, can not reinitialize' + elif cluster.leader.name == ha.state_handler.name: + status_code = 503 + data = b'I am the leader, can not reinitialize' + else: + action = ha.schedule_reinitialize() + if action is not None: + status_code = 503 + data = (action + ' already in progress').encode('utf-8') + else: + status_code = 200 + data = b'reinitialize scheduled' + + self.send_response(status_code) + self.send_header('Content-Type', 'text/html') + self.end_headers() + self.wfile.write(data) + def parse_request(self): """Override parse_request method to enrich basic functionality of `BaseHTTPRequestHandler` class @@ -104,9 +149,9 @@ class RestApiHandler(BaseHTTPRequestHandler): pg_last_xlog_replay_location(), pg_is_in_recovery() AND pg_is_xlog_replay_paused()""", retry=retry)[0] return { - 'running': True, + 'state': self.server.patroni.postgresql.state, 'postmaster_start_time': row[0], - 'role': 'slave' if row[1] else 'master', + 'role': 'replica' if row[1] else 'master', 'xlog': ({ 'received_location': row[3], 'replayed_location': row[4], @@ -116,7 +161,7 @@ class RestApiHandler(BaseHTTPRequestHandler): } except (psycopg2.Error, RetryFailedError, PostgresConnectionException): logger.exception('get_postgresql_status') - return {'running': self.server.patroni.postgresql.is_running()} + return {'state': self.server.patroni.postgresql.state} class RestApiServer(ThreadingMixIn, HTTPServer, Thread): diff --git a/patroni/dcs.py b/patroni/dcs.py index 908386b5..ebf625d6 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -120,14 +120,17 @@ class AbstractDCS: running as a master and exception raised instance would be demoted.""" @abc.abstractmethod - def update_leader(self, state_handler): - """Update leader key (or session) ttl and `/optime/leader` key in DCS. + def write_leader_optime(self, last_operation): + """write current xlog location into `/optime/leader` key in DCS + :param last_operation: absolute xlog location in bytes""" + + @abc.abstractmethod + def update_leader(self): + """Update leader key (or session) ttl - :param state_handler: reference to `Postgresql` object :returns: `!True` if leader key (or session) has been updated successfully. If not, `!False` must be returned and current instance would be demoted. - If you failed to update `/optime/leader` this error is not critical and you can return `!True` You have to use CAS (Compare And Swap) operation in order to update leader key, for example for etcd `prevValue` parameter must be used.""" diff --git a/patroni/etcd.py b/patroni/etcd.py index fc6d5a97..b189c188 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -222,14 +222,12 @@ class Etcd(AbstractDCS): return False @catch_etcd_errors - def write_leader_optime(self, state_handler): - return self.client.set(self.leader_optime_path, state_handler.last_operation()) + def write_leader_optime(self, last_operation): + return self.client.set(self.leader_optime_path, last_operation) @catch_etcd_errors - def update_leader(self, state_handler): - ret = self.retry(self.client.test_and_set, self.leader_path, self._name, self._name, self.ttl) - ret and self.write_leader_optime(state_handler) - return ret + def update_leader(self): + return self.retry(self.client.test_and_set, self.leader_path, self._name, self._name, self.ttl) @catch_etcd_errors def initialize(self): diff --git a/patroni/ha.py b/patroni/ha.py index 0d95d372..64179a78 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -2,6 +2,7 @@ import logging import psycopg2 from patroni.exceptions import DCSError, PostgresConnectionException +from threading import Lock logger = logging.getLogger(__name__) @@ -13,6 +14,10 @@ class Ha: self.dcs = etcd self.cluster = None self.old_cluster = None + self.scheduled_action = None + self.scheduled_action_lock = Lock() + self.restart_in_progress = False + self.restart_thread_lock = Lock() def load_cluster_from_dcs(self): cluster = self.dcs.get_cluster() @@ -28,7 +33,13 @@ class Ha: return self.dcs.attempt_to_acquire_leader() def update_lock(self): - return self.dcs.update_leader(self.state_handler) + ret = self.dcs.update_leader() + if ret: + try: + self.dcs.write_leader_optime(self.state_handler.last_operation()) + except: + pass + return ret def has_lock(self): lock_owner = self.cluster.leader and self.cluster.leader.name @@ -37,8 +48,9 @@ class Ha: def bootstrap(self): if not self.cluster.is_unlocked(): # cluster already has leader - logger.info('trying to bootstrap from leader', ) + logger.info('trying to bootstrap from leader') if self.state_handler.bootstrap(self.cluster.leader): + self.reinitialize_scheduled() and self.reset_scheduled_action() return 'bootstrapped from leader' else: self.state_handler.stop('immediate') @@ -63,15 +75,17 @@ class Ha: return 'waiting for leader to bootstrap' def recover(self): - if self.state_handler.is_healthy(): - return False has_lock = self.has_lock() self.state_handler.write_recovery_conf(None if has_lock else self.cluster.leader) - self.state_handler.start() - if has_lock: - logger.info('started as readonly because i had the session lock') - self.load_cluster_from_dcs() - return True + if not self.state_handler.start(): + if not has_lock: + return 'failed to start postgres' + self.dcs.delete_leader() + return 'removed leader key after trying and failing to start postgres' + if not has_lock: + return 'started as a secondary' + logger.info('started as readonly because i had the session lock') + self.load_cluster_from_dcs() def follow_the_leader(self, demote_reason, follow_reason, refresh=True): refresh and self.load_cluster_from_dcs() @@ -112,7 +126,68 @@ class Ha: return self.follow_the_leader('demoting self because i do not have the lock and i was a leader', 'no action. i am a secondary and i am following a leader', False) - def run_cycle(self): + def schedule_action(self, action): + with self.scheduled_action_lock: + if self.scheduled_action is not None: + return self.scheduled_action + self.scheduled_action = action + return None + + def get_scheduled_action(self): + with self.scheduled_action_lock: + return self.scheduled_action + + def reset_scheduled_action(self): + with self.scheduled_action_lock: + self.scheduled_action = None + + def schedule_restart(self): + return self.schedule_action('restart') + + def restart_scheduled(self): + return self.get_scheduled_action() == 'restart' + + def schedule_reinitialize(self): + return self.schedule_action('reinitialize') + + def reinitialize_scheduled(self): + return self.get_scheduled_action() == 'reinitialize' + + def restart(self): + with self.restart_thread_lock: + self.restart_in_progress = True + try: + return self.state_handler.restart() + finally: + with self.restart_thread_lock: + self.restart_in_progress = False + self.reset_scheduled_action() + + def process_scheduled_action(self): + if self.reinitialize_scheduled(): + if self.cluster.is_unlocked(): + logger.error('Cluster has no leader, can not reinitialize') + self.reset_scheduled_action() + elif self.has_lock(): + logger.error('I am the leader, can not reinitialize') + self.reset_scheduled_action() + else: + self.state_handler.stop('immediate') + self.state_handler.remove_data_directory() + self.load_cluster_from_dcs() + + def handle_restart_in_progress(self): + if self.has_lock(): + if self.update_lock(): + return 'updated leader lock during restart' + else: + return 'failed to update leader lock during restart' + elif self.cluster.is_unlocked(): + return 'not healthy enough for leader race' + else: + return 'restart in progress' + + def _run_cycle(self): try: self.load_cluster_from_dcs() @@ -120,6 +195,9 @@ class Ha: if not self.cluster.is_unlocked() and not self.cluster.initialize: self.dcs.initialize() # fix it + # currently it can trigger only reinitialize + self.process_scheduled_action() + # is data directory empty? if self.state_handler.data_directory_empty(): return self.bootstrap() # new node @@ -127,10 +205,14 @@ class Ha: elif not self.cluster.initialize and self.cluster.is_unlocked(): self.dcs.initialize() + if self.restart_in_progress: + return self.handle_restart_in_progress() + # try to start dead postgres - if self.recover() and not self.has_lock(): - # no lock, do not try to promote immediately - return 'started as a secondary' + if not self.state_handler.is_healthy(): + msg = self.recover() + if msg is not None: + return msg if self.cluster.is_unlocked(): return self.process_unhealthy_cluster() @@ -143,3 +225,7 @@ class Ha: return 'demoted self because DCS is not accessible and i was a leader' except (psycopg2.Error, PostgresConnectionException): logger.exception('Error communicating with Postgresql. Will try again') + + def run_cycle(self): + with self.restart_thread_lock: + return self._run_cycle() diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 81f5c02e..2bd372cf 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -56,7 +56,6 @@ class Postgresql: self.postmaster_pid = os.path.join(self.data_dir, 'postmaster.pid') self.trigger_file = config.get('recovery_conf', {}).get('trigger_file', None) or 'promote' self.trigger_file = os.path.abspath(os.path.join(self.data_dir, self.trigger_file)) - self._role = 'replica' self._pg_ctl = ['pg_ctl', '-w', '-D', self.data_dir] @@ -67,9 +66,16 @@ class Postgresql: self._connection = None self._cursor_holder = None - self.members = [] # list of already existing replication slots + self.replication_slots = [] # list of already existing replication slots self.retry = Retry(max_tries=-1, deadline=10, max_delay=1, retry_exceptions=PostgresConnectionException) + self._state = 'stopped' + self._role = 'replica' + + if self.is_running(): + self._state = 'running' + self._role = 'master' if self.is_leader() else 'replica' + def get_local_address(self): listen_addresses = self.listen_addresses.split(',') local_address = listen_addresses[0].strip() # take first address from listen_addresses @@ -101,6 +107,8 @@ class Postgresql: except psycopg2.Error as e: if cursor and cursor.connection.closed == 0: raise e + if self.state == 'restarting': + raise RetryFailedError('cluster is being restarted') raise PostgresConnectionException('connection problems') def query(self, sql, *params): @@ -113,8 +121,12 @@ class Postgresql: return not os.path.exists(self.data_dir) or os.listdir(self.data_dir) == [] def initialize(self): + self._state = 'initalizing new cluster' ret = subprocess.call(self._pg_ctl + ['initdb', '-o', '--encoding=UTF8']) == 0 - ret and self.write_pg_hba() + if ret: + self.write_pg_hba() + else: + self._state = 'initdb failed' return ret def delete_trigger_file(self): @@ -137,6 +149,7 @@ class Postgresql: return "host={host} port={port} user={user}".format(**conn) def create_replica(self, master_connection, env): + self._state = 'building replica from {host}:{port}'.format(**master_connection) connstring = self.build_connstring(master_connection) cmd = self.config['restore'] try: @@ -144,7 +157,9 @@ class Postgresql: self.delete_trigger_file() except: logger.exception('Error when creating replica') - return 1 + ret = 1 + if ret != 0: + self._state = 'failed to build replica from {host}:{port}'.format(**master_connection) return ret def is_leader(self): @@ -169,38 +184,60 @@ class Postgresql: def role(self): return self._role + @property + def state(self): + return self._state + def start(self, block_callbacks=False): if self.is_running(): - self._role = 'master' if self.is_leader() else 'replica' - self.schedule_load_slots = self.use_slots logger.error('Cannot start PostgreSQL because one is already running.') - return False + return True self._role = 'replica' if os.path.exists(self.recovery_conf) else 'master' if os.path.exists(self.postmaster_pid): os.remove(self.postmaster_pid) logger.info('Removed %s', self.postmaster_pid) + if not block_callbacks: + self._state = 'starting' + ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()]) == 0 + + self._state = 'running' if ret else 'start failed' + self.schedule_load_slots = ret and self.use_slots self.save_configuration_files() # block_callbacks is used during restart to avoid # running start/stop callbacks in addition to restart ones - ret and not block_callbacks and ret and self.call_nowait(ACTION_ON_START) + ret and not block_callbacks and self.call_nowait(ACTION_ON_START) return ret + def checkpoint(self): + try: + self.query('SET statement_timeout TO 0') + self.query('CHECKPOINT') + except: + logging.exception('Exception diring CHECKPOINT') + def stop(self, mode='fast', block_callbacks=False): + if not self.is_running(): + if not block_callbacks: + self._state = 'stopped' + return True + if block_callbacks: - try: - self.query('SET statement_timeout TO 0') - self.query('CHECKPOINT') - except: - logging.exception('Exception diring CHECKPOINT') + self.checkpoint() + else: + self._state = 'stopping' ret = subprocess.call(self._pg_ctl + ['stop', '-m', mode]) == 0 # block_callbacks is used during restart to avoid # running start/stop callbacks in addition to restart ones - ret and not block_callbacks and self.call_nowait(ACTION_ON_STOP) + if not ret: + self._state = 'stop failed' + elif not block_callbacks: + self._state = 'stopped' + self.call_nowait(ACTION_ON_STOP) return ret def reload(self): @@ -209,8 +246,12 @@ class Postgresql: return ret def restart(self): + self._state = 'restarting' ret = self.stop(block_callbacks=True) and self.start(block_callbacks=True) - ret and self.call_nowait(ACTION_ON_RESTART) + if ret: + self.call_nowait(ACTION_ON_RESTART) + else: + self._state = 'restart failed ({})'.format(self._state) return ret def server_options(self): @@ -356,26 +397,26 @@ recovery_target_timeline = 'latest' def load_replication_slots(self): if self.use_slots and self.schedule_load_slots: cursor = self.query("SELECT slot_name FROM pg_replication_slots WHERE slot_type='physical'") - self.members = [r[0] for r in cursor] + self.replication_slots = [r[0] for r in cursor] self.schedule_load_slots = False def sync_replication_slots(self, cluster): if self.use_slots: self.load_replication_slots() - members = [m.name for m in cluster.members if m.name != self.name] if self.role == 'master' else [] + slots = [m.name for m in cluster.members if m.name != self.name] if self.role == 'master' else [] # drop unused slots - for slot in set(self.members) - set(members): + for slot in set(self.replication_slots) - set(slots): self.query("""SELECT pg_drop_replication_slot(%s) WHERE EXISTS(SELECT 1 FROM pg_replication_slots WHERE slot_name = %s)""", slot, slot) # create new slots - for slot in set(members) - set(self.members): + for slot in set(slots) - set(self.replication_slots): self.query("""SELECT pg_create_physical_replication_slot(%s) WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots WHERE slot_name = %s)""", slot, slot) - self.members = members + self.replication_slots = slots def last_operation(self): return str(self.xlog_position()) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 9cd15fbc..5d221ac6 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -211,8 +211,8 @@ class ZooKeeper(AbstractDCS): def take_leader(self): return self.attempt_to_acquire_leader() - def update_leader(self, state_handler): - last_operation = state_handler.last_operation().encode('utf-8') + def write_leader_optime(self, last_operation): + last_operation = last_operation.encode('utf-8') if last_operation != self.last_leader_operation: self.last_leader_operation = last_operation path = self.leader_optime_path @@ -225,6 +225,8 @@ class ZooKeeper(AbstractDCS): logger.exception('Failed to create %s', path) except: logger.exception('Failed to update %s', path) + + def update_leader(self): return True def delete_leader(self): diff --git a/tests/test_api.py b/tests/test_api.py index b8f9e038..4bb481bf 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -10,6 +10,10 @@ from test_postgresql import psycopg2_connect, MockCursor class MockPostgresql(Mock): + name = 'test' + state = 'running' + role = 'master' + def connection(self): return psycopg2_connect() @@ -17,9 +21,28 @@ class MockPostgresql(Mock): return True +class MockHa(Mock): + + dcs = Mock() + state_handler = MockPostgresql() + + def schedule_restart(self): + return 'restart' + + def schedule_reinitialize(self): + return 'reinitialize' + + def restart(self): + return True + + def restart_scheduled(self): + return False + + class MockPatroni: postgresql = MockPostgresql() + ha = MockHa() class MockRequest: @@ -47,18 +70,38 @@ class MockRestApiServer(RestApiServer): class TestRestApiHandler(unittest.TestCase): def test_do_GET(self): - MockRestApiServer(RestApiHandler, b'GET /') - with patch.object(RestApiServer, 'query', Mock(side_effect=psycopg2.OperationalError())): - MockRestApiServer(RestApiHandler, b'GET /') - - def test_do_GET_sampleauth(self): - MockRestApiServer(RestApiHandler, b'GET /sampleauth') - MockRestApiServer(RestApiHandler, b'GET /sampleauth\nAuthorization:') - MockRestApiServer(RestApiHandler, b'GET /sampleauth\nAuthorization: Basic dGVzdDp0ZXN0') + MockRestApiServer(RestApiHandler, b'GET /master') + MockRestApiServer(RestApiHandler, b'GET /replica') + with patch.object(MockHa, 'restart_scheduled', Mock(return_value=True)): + MockRestApiServer(RestApiHandler, b'GET /master') def test_do_GET_patroni(self): MockRestApiServer(RestApiHandler, b'GET /patroni') + def test_basicauth(self): + MockRestApiServer(RestApiHandler, b'POST /restart HTTP/1.0') + MockRestApiServer(RestApiHandler, b'POST /restart HTTP/1.0\nAuthorization:') + + def test_do_POST_restart(self): + request = b'POST /restart HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' + MockRestApiServer(RestApiHandler, request) + with patch.object(MockHa, 'schedule_restart', Mock(return_value=None)): + MockRestApiServer(RestApiHandler, request) + with patch.object(MockHa, 'restart', Mock(side_effect=Exception)): + MockRestApiServer(RestApiHandler, request) + + @patch.object(MockHa, 'dcs') + def test_do_POST_reinitialize(self, dcs): + cluster = dcs.get_cluster.return_value + request = b'POST /reinitialize HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' + MockRestApiServer(RestApiHandler, request) + cluster.is_unlocked.return_value = False + MockRestApiServer(RestApiHandler, request) + with patch.object(MockHa, 'schedule_reinitialize', Mock(return_value=None)): + MockRestApiServer(RestApiHandler, request) + cluster.leader.name = 'test' + MockRestApiServer(RestApiHandler, request) + @patch('time.sleep', Mock()) def test_RestApiServer_query(self): with patch.object(MockCursor, 'execute', Mock(side_effect=psycopg2.OperationalError)): diff --git a/tests/test_etcd.py b/tests/test_etcd.py index d2824699..18e8f22e 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -242,8 +242,11 @@ class TestEtcd(unittest.TestCase): self.etcd._base_path = '/service/failed' self.assertFalse(self.etcd.attempt_to_acquire_leader()) + def test_write_leader_optime(self): + self.etcd.write_leader_optime('0') + def test_update_leader(self): - self.assertTrue(self.etcd.update_leader(MockPostgresql())) + self.assertTrue(self.etcd.update_leader()) def test_initialize(self): self.assertFalse(self.etcd.initialize()) diff --git a/tests/test_ha.py b/tests/test_ha.py index 925beb33..70018672 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -82,10 +82,25 @@ class TestHa(unittest.TestCase): self.e.get_cluster = get_cluster_not_initialized_without_leader ha.load_cluster_from_dcs() - def test_start_as_slave(self): + def test_update_lock(self): + self.p.last_operation = Mock(side_effect=PostgresException('')) + self.assertTrue(self.ha.update_lock()) + + def test_start_as_replica(self): self.p.is_healthy = false self.assertEquals(self.ha.run_cycle(), 'started as a secondary') + def test_recover_replica_failed(self): + self.p.is_healthy = false + self.p.start = false + self.assertEquals(self.ha.run_cycle(), 'failed to start postgres') + + def test_recover_master_failed(self): + self.p.is_healthy = false + self.p.start = false + self.ha.has_lock = true + self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres') + def test_start_as_readonly(self): self.ha.cluster.is_unlocked = false self.p.is_leader = self.p.is_healthy = false @@ -174,3 +189,35 @@ class TestHa(unittest.TestCase): self.e.initialize = true self.p.bootstrap = Mock(side_effect=PostgresException("Could not bootstrap master PostgreSQL")) self.assertRaises(PostgresException, self.ha.bootstrap) + + def test_reinitialize(self): + self.ha.schedule_reinitialize() + self.ha.run_cycle() + self.assertIsNone(self.ha.get_scheduled_action()) + + self.ha.cluster = get_cluster_initialized_with_leader() + self.ha.schedule_reinitialize() + self.ha.run_cycle() + + self.ha.has_lock = true + self.ha.schedule_reinitialize() + self.ha.run_cycle() + self.assertIsNone(self.ha.get_scheduled_action()) + + def test_restart(self): + self.ha.schedule_restart() + self.assertTrue(self.ha.restart_scheduled()) + self.ha.restart() + + def test_restart_in_progress(self): + self.ha.restart_in_progress = True + self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race') + + self.ha.cluster = get_cluster_initialized_with_leader() + self.assertEquals(self.ha.run_cycle(), 'restart in progress') + + self.ha.has_lock = true + self.assertEquals(self.ha.run_cycle(), 'updated leader lock during restart') + + self.ha.update_lock = false + self.assertEquals(self.ha.run_cycle(), 'failed to update leader lock during restart') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index b4988c25..d1ab7e8d 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -85,10 +85,12 @@ def psycopg2_connect(*args, **kwargs): @patch('subprocess.call', Mock(return_value=0)) -@patch('shutil.copy', Mock()) @patch('psycopg2.connect', psycopg2_connect) +@patch('shutil.copy', Mock()) class TestPostgresql(unittest.TestCase): + @patch('subprocess.call', Mock(return_value=0)) + @patch('psycopg2.connect', psycopg2_connect) def setUp(self): self.p = Postgresql({'name': 'test0', 'scope': 'batman', 'data_dir': 'data/test0', 'listen': '127.0.0.1, *:5432', 'connect_address': '127.0.0.2:5432', @@ -121,13 +123,24 @@ class TestPostgresql(unittest.TestCase): self.assertTrue(self.p.initialize()) self.assertTrue(os.path.exists(os.path.join(self.p.data_dir, 'pg_hba.conf'))) - def test_start_stop(self): - self.assertFalse(self.p.start()) - self.p.is_running = false - with open(os.path.join(self.p.data_dir, 'postmaster.pid'), 'w'): - pass + def test_start(self): self.assertTrue(self.p.start()) + self.p.is_running = false + open(os.path.join(self.p.data_dir, 'postmaster.pid'), 'w').close() + self.assertTrue(self.p.start()) + + def test_stop(self): self.assertTrue(self.p.stop()) + with patch('subprocess.call', Mock(return_value=1)): + self.assertTrue(self.p.stop()) + self.p.is_running = Mock(return_value=True) + self.assertFalse(self.p.stop()) + + def test_restart(self): + self.p.start = false + self.p.is_running = false + self.assertFalse(self.p.restart()) + self.assertEquals(self.p.state, 'restart failed (restarting)') def test_sync_from_leader(self): self.assertTrue(self.p.sync_from_leader(self.leader)) @@ -157,6 +170,8 @@ class TestPostgresql(unittest.TestCase): @patch.object(MockConnect, 'closed', 2) def test__query(self): self.assertRaises(PostgresConnectionException, self.p._query, 'blabla') + self.p._state = 'restarting' + self.assertRaises(RetryFailedError, self.p._query, 'blabla') def test_query(self): self.p.query('select 1') @@ -184,6 +199,7 @@ class TestPostgresql(unittest.TestCase): self.assertFalse(self.p.is_healthy()) def test_promote(self): + self.p._role = 'replica' self.assertTrue(self.p.promote()) self.assertTrue(self.p.promote()) @@ -211,8 +227,8 @@ class TestPostgresql(unittest.TestCase): self.p.move_data_directory() def test_bootstrap(self): - self.assertRaises(PostgresException, self.p.bootstrap) - self.p.start = Mock(return_value=True) + with patch('subprocess.call', Mock(return_value=1)): + self.assertRaises(PostgresException, self.p.bootstrap) self.p.bootstrap() def test_remove_data_directory(self): diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index afbdad5f..b04f5b5a 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -134,11 +134,13 @@ class TestZooKeeper(unittest.TestCase): self.zk.take_leader() def test_update_leader(self): - self.zk.last_leader_operation = -1 - self.assertTrue(self.zk.update_leader(MockPostgresql())) + self.assertTrue(self.zk.update_leader()) + + def test_write_leader_optime(self): + self.zk.last_leader_operation = '0' + self.zk.write_leader_optime('1') self.zk._base_path = self.zk._base_path.replace('test', 'bla') - self.zk.last_leader_operation = -1 - self.assertTrue(self.zk.update_leader(MockPostgresql())) + self.zk.write_leader_optime('2') def test_watch(self): self.zk.watch(0) From 3b1b6ff448e104ac41bba963435bc67ce3d37ff5 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Sep 2015 16:54:16 +0200 Subject: [PATCH 02/15] revert log level to INFO --- patroni/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 35f206df..cadf335a 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -73,7 +73,7 @@ class Patroni: def main(): - logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.DEBUG) + logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO) logging.getLogger('requests').setLevel(logging.WARNING) setup_signal_handlers() From c218054d05e4ddbaee5a144ae439e7c6ffe72bdb Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 28 Sep 2015 17:00:42 +0200 Subject: [PATCH 03/15] Implement manual failover Implementation is done on top of feature/is-healthiest-via-api and feature/api branches. In order to trigger manual failover one has to create 'failover' key in a configuration store with the value in following format: 'leader_name:member_name' leader_name can be empty or should match with the name of current leader member_name can be empty or should match with the name one of cluster nodes Leader always checks that either desired member (if specified) or one of the memners is accessible and healthy before demote. After leader has deomted himself other nodes are performig checks that desired node is healthy. If it is not they are participating in a leader race. In some cases (when accidently there is no healthy nodes) former leader can also participate in a leader race. Current implementation does not provide REST API endpoint for a manual failover. --- patroni/__init__.py | 11 +-- patroni/api.py | 9 +-- patroni/dcs.py | 25 ++++++- patroni/etcd.py | 22 ++++-- patroni/ha.py | 147 +++++++++++++++++++++++++++++++++++++-- patroni/postgresql.py | 32 +-------- patroni/zookeeper.py | 35 +++++++--- tests/test_etcd.py | 4 ++ tests/test_ha.py | 88 +++++++++++++++++++---- tests/test_patroni.py | 7 +- tests/test_postgresql.py | 29 ++------ tests/test_zookeeper.py | 19 ++++- 12 files changed, 320 insertions(+), 108 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index cadf335a..6ca96df0 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -8,7 +8,7 @@ from patroni.api import RestApiServer from patroni.etcd import Etcd from patroni.ha import Ha from patroni.postgresql import Postgresql -from patroni.utils import setup_signal_handlers, sleep, reap_children +from patroni.utils import setup_signal_handlers, reap_children from patroni.zookeeper import ZooKeeper logger = logging.getLogger(__name__) @@ -42,12 +42,6 @@ class Patroni: return True return self.ha.dcs.touch_member(connection_string, ttl) - def initialize(self): - # wait for etcd to be available - while not self.touch_member(): - logger.info('waiting on DCS') - sleep(5) - def schedule_next_run(self): self.next_run += self.nap_time current_time = time.time() @@ -62,12 +56,12 @@ class Patroni: self.next_run = time.time() while True: - self.touch_member() logger.info(self.ha.run_cycle()) try: self.ha.cluster and self.ha.state_handler.sync_replication_slots(self.ha.cluster) except: logger.exception('Exception when changing replication slots') + self.touch_member() reap_children() self.schedule_next_run() @@ -85,7 +79,6 @@ def main(): config = yaml.load(f) patroni = Patroni(config) - patroni.initialize() try: patroni.run() except KeyboardInterrupt: diff --git a/patroni/api.py b/patroni/api.py index ebb732a4..d507bfb3 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -143,10 +143,11 @@ class RestApiHandler(BaseHTTPRequestHandler): row = self.query("""SELECT to_char(pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ'), pg_is_in_recovery(), CASE WHEN pg_is_in_recovery() - THEN null - ELSE pg_current_xlog_location() END, - pg_last_xlog_receive_location(), - pg_last_xlog_replay_location(), + THEN 0 + ELSE pg_xlog_location_diff(pg_current_xlog_location(), '0/0')::bigint + END, + pg_xlog_location_diff(pg_last_xlog_receive_location(), '0/0')::bigint, + pg_xlog_location_diff(pg_last_xlog_replay_location(), '0/0')::bigint, pg_is_in_recovery() AND pg_is_xlog_replay_paused()""", retry=retry)[0] return { 'state': self.server.patroni.postgresql.state, diff --git a/patroni/dcs.py b/patroni/dcs.py index ebf625d6..fb034322 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -56,7 +56,15 @@ class Leader(namedtuple('Leader', 'index,expiration,ttl,member')): return self.member.conn_url -class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,members')): +class Failover(namedtuple('Failover', 'index,leader,member')): + + @staticmethod + def from_node(index, value): + t = [a.strip() for a in value.split(':')] + [''] + return Failover(index, t[0], t[1]) if t[0] or t[1] else None + + +class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,members,failover')): """Immutable object (namedtuple) which represents PostgreSQL cluster. Consists of the following fields: @@ -64,7 +72,8 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem :param leader: `Leader` object which represents current leader of the cluster :param last_leader_operation: int or long object containing position of last known leader operation. This value is stored in `/optime/leader` key - :param members: list of Member object, all PostgreSQL cluster members including leader""" + :param members: list of Member object, all PostgreSQL cluster members including leader + :param failover: reference to `Failover` object""" def is_unlocked(self): return not (self.leader and self.leader.name) @@ -76,6 +85,7 @@ class AbstractDCS: _INITIALIZE = 'initialize' _LEADER = 'leader' + _FAILOVER = 'failover' _MEMBERS = 'members/' _OPTIME = 'optime' _LEADER_OPTIME = _OPTIME + '/' + _LEADER @@ -109,6 +119,10 @@ class AbstractDCS: def leader_path(self): return self.client_path(self._LEADER) + @property + def failover_path(self): + return self.client_path(self._FAILOVER) + @property def leader_optime_path(self): return self.client_path(self._LEADER_OPTIME) @@ -143,6 +157,13 @@ class AbstractDCS: Key must be created atomically. In case if key already exists it should not be overwritten and `!False` must be returned""" + @abc.abstractmethod + def set_failover_value(self, value, index=None): + """Create or update `/failover` key""" + + def manual_failover(self, leader, member, index=None): + return self.set_failover_value(leader + (':' + member if member else ''), index) + def current_leader(self): try: cluster = self.get_cluster() diff --git a/patroni/etcd.py b/patroni/etcd.py index b189c188..c2995da9 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -10,7 +10,8 @@ import urllib3 from dns.exception import DNSException from dns import resolver -from patroni.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string +from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member, parse_connection_string +from patroni.exceptions import DCSError from patroni.utils import Retry, RetryFailedError, sleep from requests.exceptions import RequestException @@ -80,7 +81,7 @@ class Client(etcd.Client): for host, port in self.get_srv_record(discovery_srv): url = '{}://{}:{}/members'.format(self._protocol, host, port) try: - response = requests.get(url) + response = requests.get(url, timeout=5) if response.ok: for member in response.json(): ret.extend(member['clientURLs']) @@ -195,9 +196,14 @@ class Etcd(AbstractDCS): member = ([m for m in members if m.name == leader.value] or [member])[0] leader = Leader(leader.modifiedIndex, leader.expiration, leader.ttl, member) - self.cluster = Cluster(initialize, leader, last_leader_operation, members) + # failover key + failover = nodes.get(self._FAILOVER, None) + if failover: + failover = Failover.from_node(failover.modifiedIndex, failover.value) + + self.cluster = Cluster(initialize, leader, last_leader_operation, members, failover) except etcd.EtcdKeyNotFound: - self.cluster = Cluster(False, None, None, []) + self.cluster = Cluster(False, None, None, [], None) except: self.cluster = None logger.exception('get_cluster') @@ -221,6 +227,10 @@ class Etcd(AbstractDCS): pass return False + @catch_etcd_errors + def set_failover_value(self, value, index=None): + return self.client.write(self.failover_path, value, prevIndex=index or 0) + @catch_etcd_errors def write_leader_optime(self, last_operation): return self.client.set(self.leader_optime_path, last_operation) @@ -231,7 +241,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def initialize(self): - return self.client.write(self.initialize_path, self._name, prevExist=False) + return self.retry(self.client.write, self.initialize_path, self._name, prevExist=False) @catch_etcd_errors def delete_leader(self): @@ -239,7 +249,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def cancel_initialization(self): - return self.client.delete(self.initialize_path, prevValue=self._name) + return self.retry(self.client.delete, self.initialize_path, prevValue=self._name) def watch(self, timeout): # watch on leader key changes if it is defined and current node is not lock owner diff --git a/patroni/ha.py b/patroni/ha.py index 64179a78..628b9c41 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -1,7 +1,9 @@ import logging import psycopg2 +import requests from patroni.exceptions import DCSError, PostgresConnectionException +from multiprocessing.pool import ThreadPool from threading import Lock logger = logging.getLogger(__name__) @@ -9,9 +11,9 @@ logger = logging.getLogger(__name__) class Ha: - def __init__(self, state_handler, etcd): + def __init__(self, state_handler, dcs): self.state_handler = state_handler - self.dcs = etcd + self.dcs = dcs self.cluster = None self.old_cluster = None self.scheduled_action = None @@ -100,9 +102,141 @@ class Ha: self.state_handler.promote() return promote_message + @staticmethod + def fetch_node_status(member): + """This function perform http get request on member.api_url and fetches its status + :returns: tuple(`member`, reachable, in_recovery, xlog_location) + + reachable - `!False` if the node is not reachable or is not responding with correct JSON + in_recovery - `!True` if pg_is_in_recovery() == true + xlog_location - value of `replayed_location` or `location` from JSON, dependin on its role.""" + + try: + response = requests.get(member.api_url, timeout=2, verify=False) + logger.info('Got response from %s %s: %s', member.name, member.api_url, response.content) + json = response.json() + is_master = json['role'] == 'master' + xlog_location = json['xlog']['location' if is_master else 'replayed_location'] + return (member, True, not is_master, xlog_location) + except: + logging.exception('request failed: GET %s', member.api_url) + return (member, False, None, 0) + + def fetch_nodes_statuses(self, members): + pool = ThreadPool(len(members)) + results = pool.map(self.fetch_node_status, members) # Run API calls on members in parallel + pool.close() + pool.join() + return results + + def _is_healthiest_node(self, members, check_replication_lag=True): + """This method tries to determine whether I am healthy enough to became a new leader candidate or not.""" + + if self.state_handler.is_leader(): + return True + + if check_replication_lag and not self.state_handler.check_replication_lag(self.cluster.last_leader_operation): + return False # Too far behind last reported xlog location on master + + # Prepare list of nodes to run check against + members = [m for m in members if m.name != self.state_handler.name and m.api_url] + + if members: + my_xlog_location = self.state_handler.xlog_position() + for member, reachable, in_recovery, xlog_location in self.fetch_nodes_statuses(members): + if reachable: # If the node is unreachable it's not healhy + if not in_recovery: + logger.warning('Master (%s) is still alive', member.name) + return False + if my_xlog_location < xlog_location: + return False + return True + + def is_failover_possible(self, members): + ret = False + members = [m for m in members if m.name != self.state_handler.name and m.api_url] + if members: + for member, reachable, in_recovery, xlog_location in self.fetch_nodes_statuses(members): + if reachable: + ret = True # TODO: check xlog_location + else: + logger.info('Member %s is not reachable', member.name) + else: + logger.warning('manual failover: members list is empty') + return ret + + def manual_failover_process_no_leader(self): + failover = self.cluster.failover + if failover.member: # manual failover to specific member + if failover.member == self.state_handler.name: # manual failover to me + return True + + # find specific node and check that it is healthy + members = [m for m in self.cluster.members if m.name == failover.member] + if members: + member, reachable, in_recovery, xlog_location = self.fetch_node_status(members[0]) + if reachable: # node is healthy + logger.info('manual failover: to %s, i am %s', member.name, self.state_handler.name) + return False + # we wanted to failover to specific member but it is not healthy + logger.warning('manual failover: member %s is unhealthy', member.name) + + # at this point we should consider all members as a candidates for failover + # i.e. we assume that failover.member is None + + # try to pick some other members to failover and check that they are healthy + if failover.leader: + if self.state_handler.name == failover.leader: # I was the leader + # exclude me and desired member which is unhealthy (failover.member can be None) + members = [m for m in self.cluster.members if m.name != failover.member] + if self.is_failover_possible(members): # check that there are healthy members + return False + else: # I was the leader and it looks like currently I am the only healthy member + return True + + # at this point we assume that our node is a candidate for a failover among all nodes except former leader + + # exclude former leader from the list (failover.leader can be None) + members = [m for m in self.cluster.members if m.name != failover.leader] + return self._is_healthiest_node(members, check_replication_lag=False) + + def is_healthiest_node(self): + if self.cluster.failover: + return self.manual_failover_process_no_leader() + + # run usual health check + members = {m.name: m for m in self.old_cluster.members + self.cluster.members} + return self._is_healthiest_node(members.values()) + + def process_manual_failover_from_leader(self): + failover = self.cluster.failover + if not failover.leader or failover.leader == self.state_handler.name: + if not failover.member or failover.member != self.state_handler.name: + members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] + if self.is_failover_possible(members): # check that there are healthy members + self.state_handler.follow_the_leader(None) + self.cluster = None + if self.dcs.delete_leader(): + return 'manual failover: demoted self and released leader lock' + else: + return 'manual failover: demoted self but failed to release leader lock' + else: + logger.warning('manual failover: no healthy members found, failover is not possible') + else: + logger.warning('manual failover: I am already the leader, no need to failover') + else: + logger.warning('manual failover: leader name does not match: %s != %s', + self.cluster.failover.leader, self.state_handler.name) + + logger.info('Trying to clean up failover key') + self.dcs.manual_failover('', '', self.cluster.failover.index) + def process_unhealthy_cluster(self): - if self.state_handler.is_healthiest_node(self.old_cluster): + if self.is_healthiest_node(): if self.acquire_lock(): + if self.cluster.failover: + logger.info('Cleanning up failover key after acquiring leader lock...') + self.dcs.manual_failover('', '') return self.enforce_master_role('acquired session lock as a leader', 'promoted self to leader by acquiring session lock') else: @@ -114,6 +248,11 @@ class Ha: def process_healthy_cluster(self): if self.has_lock(): + if self.cluster.failover: + msg = self.process_manual_failover_from_leader() + if msg is not None: + return msg + if self.update_lock(): return self.enforce_master_role('no action. i am the leader with the lock', 'promoted self to leader because i had the session lock') @@ -220,7 +359,7 @@ class Ha: return self.process_healthy_cluster() except DCSError: logger.error('Error communicating with DCS') - if self.state_handler.is_leader(): + if self.state_handler.is_running() and self.state_handler.is_leader(): self.state_handler.demote(None) return 'demoted self because DCS is not accessible and i was a leader' except (psycopg2.Error, PostgresConnectionException): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 2bd372cf..bc3188ed 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -266,36 +266,8 @@ class Postgresql: return False return True - def is_healthiest_node(self, cluster): - if self.is_leader(): - return True - - if cluster.last_leader_operation - self.xlog_position() > self.config.get('maximum_lag_on_failover', 0): - return False - - for member in cluster.members: - if member.name == self.name: - continue - try: - r = parseurl(member.conn_url) - member_conn = psycopg2.connect(**r) - member_conn.autocommit = True - member_cursor = member_conn.cursor() - member_cursor.execute( - "SELECT pg_is_in_recovery(), %s - pg_xlog_location_diff(pg_last_xlog_replay_location(), '0/0')", - (self.xlog_position(),)) - row = member_cursor.fetchone() - member_cursor.close() - member_conn.close() - logger.error([self.name, member.name, row]) - if not row[0]: - logger.warning('Master (%s) is still alive', member.name) - return False - if row[1] < 0: - return False - except psycopg2.Error: - continue - return True + def check_replication_lag(self, last_leader_operation): + return last_leader_operation - self.xlog_position() <= self.config.get('maximum_lag_on_failover', 0) def write_pg_hba(self): with open(os.path.join(self.data_dir, 'pg_hba.conf'), 'a') as f: diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 5d221ac6..7458ae46 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -5,7 +5,8 @@ import time from kazoo.client import KazooClient, KazooState from kazoo.exceptions import NoNodeError, NodeExistsError -from patroni.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string +from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member, parse_connection_string +from patroni.exceptions import DCSError from patroni.utils import sleep from requests.exceptions import RequestException @@ -134,7 +135,7 @@ class ZooKeeper(AbstractDCS): def _inner_load_cluster(self): self.cluster_event.clear() - nodes = set(self.get_children(self.client_path(''))) + nodes = set(self.get_children(self.client_path(''), self.cluster_watcher)) # get initialize flag initialize = self._INITIALIZE in nodes @@ -143,7 +144,7 @@ class ZooKeeper(AbstractDCS): members = self.load_members() if self._MEMBERS[:-1] in nodes else [] # get leader - leader = self.get_node(self.leader_path, self.cluster_watcher) if self._LEADER in nodes else None + leader = self.get_node(self.leader_path) if self._LEADER in nodes else None if leader: client_id = self.client.client_id if leader[0] == self._name and client_id is not None and client_id[0] != leader[1].ephemeralOwner: @@ -157,10 +158,15 @@ class ZooKeeper(AbstractDCS): leader = Leader(leader[1].version, None, None, member) self.fetch_cluster = member.index == -1 + # failover key + failover = self.get_node(self.failover_path, watch=self.cluster_watcher) if self._FAILOVER in nodes else None + if failover: + failover = Failover.from_node(failover[1].version, failover[0]) + # get last leader operation self.last_leader_operation = self.get_node(self.leader_optime_path) if self.fetch_cluster else None self.last_leader_operation = 0 if self.last_leader_operation is None else int(self.last_leader_operation[0]) - self.cluster = Cluster(initialize, leader, self.last_leader_operation, members) + self.cluster = Cluster(initialize, leader, self.last_leader_operation, members, failover) def get_cluster(self): if self.exhibitor and self.exhibitor.poll(): @@ -188,11 +194,21 @@ class ZooKeeper(AbstractDCS): ret or logger.info('Could not take out TTL lock') return ret + def set_failover_value(self, value, index=None): + try: + self.client.retry(self.client.set, self.failover_path, value.encode('utf-8'), version=index or -1) + return True + except NoNodeError: + return value == '' or (not index and self._create(self.failover_path, value.encode('utf-8'))) + except: + logging.exception('foo') + return False + def initialize(self): return self._create(self.initialize_path, self._name, makepath=True) def touch_member(self, connection_string, ttl=None): - if self.cluster and any(m.name == self._name for m in self.cluster.members): + if not self.fetch_cluster and self.cluster and any(m.name == self._name for m in self.cluster.members): return True path = self.member_path connection_string = connection_string.encode('utf-8') @@ -201,6 +217,9 @@ class ZooKeeper(AbstractDCS): return True except NodeExistsError: try: + node = self.get_node(path) + if node and self.client.client_id is not None and node[1].ephemeralOwner == self.client.client_id[0]: + return True self.client.retry(self.client.delete, path) self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) return True @@ -230,8 +249,8 @@ class ZooKeeper(AbstractDCS): return True def delete_leader(self): - if isinstance(self.cluster, Cluster) and self.cluster.leader.name == self._name: - self.client.delete(self.leader_path, version=self.cluster.leader.index) + self.client.restart() + return True def _cancel_initialization(self): node = self.get_node(self.initialize_path) @@ -248,5 +267,5 @@ class ZooKeeper(AbstractDCS): self.cluster_event.wait(timeout) if self.cluster_event.isSet(): self.fetch_cluster = True - return not self.cluster or not self.cluster.leader or self.cluster.leader.name != self._name + return True return False diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 18e8f22e..f55aa15e 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -50,6 +50,8 @@ def requests_get(url, **kwargs): response = MockResponse() if url.startswith('http://local'): raise requests.exceptions.RequestException() + elif ':8011/patroni' in url: + response.content = '{"role": "replica", "xlog": {"replayed_location": 0}}' elif url.endswith('/members'): if url.startswith('http://error'): response.content = '[{}]' @@ -92,6 +94,8 @@ def etcd_read(key, **kwargs): raise etcd.EtcdKeyNotFound response = {"action": "get", "node": {"key": "/service/batman5", "dir": True, "nodes": [ + {"key": "/service/batman5/failover", "value": "", + "modifiedIndex": 1582, "createdIndex": 1582}, {"key": "/service/batman5/initialize", "value": "postgresql0", "modifiedIndex": 1582, "createdIndex": 1582}, {"key": "/service/batman5/leader", "value": "postgresql1", diff --git a/tests/test_ha.py b/tests/test_ha.py index 70018672..6e50c629 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1,11 +1,11 @@ import unittest from mock import Mock, patch -from patroni.dcs import Cluster, DCSError, Leader, Member +from patroni.dcs import Cluster, Failover, Leader, Member from patroni.etcd import Client, Etcd -from patroni.exceptions import PostgresException +from patroni.exceptions import DCSError, PostgresException from patroni.ha import Ha -from test_etcd import socket_getaddrinfo, etcd_read, etcd_write +from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get def true(*args, **kwargs): @@ -16,22 +16,25 @@ def false(*args, **kwargs): return False -def get_cluster(initialize, leader): - return Cluster(initialize, leader, None, None) +def get_cluster(initialize, leader, members, failover): + return Cluster(initialize, leader, None, members, failover) def get_cluster_not_initialized_without_leader(): - return get_cluster(None, None) + return get_cluster(None, None, [], None) -def get_cluster_initialized_without_leader(): - return get_cluster(True, None) +def get_cluster_initialized_without_leader(leader=False, failover=None): + m = Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', + 'http://127.0.0.1:8008/patroni', None, 28) + l = Leader(0, 0, 0, m) if leader else None + o = Member(0, 'other', 'postgres://replicator:rep-pass@127.0.0.1:5436/postgres', + 'http://127.0.0.1:8011/patroni', None, 28) + return get_cluster(True, l, [m, o], failover) -def get_cluster_initialized_with_leader(): - return get_cluster(True, Leader(0, 0, 0, - Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', - None, None, 28))) +def get_cluster_initialized_with_leader(failover=None): + return get_cluster_initialized_without_leader(leader=True, failover=failover) class MockPostgresql(Mock): @@ -51,6 +54,9 @@ class MockPostgresql(Mock): def is_leader(self): return True + def xlog_position(self): + return 0 + def last_operation(self): return 0 @@ -60,6 +66,9 @@ class MockPostgresql(Mock): def bootstrap(self, *args, **kwargs): return True + def check_replication_lag(self, last_leader_operation): + return True + class TestHa(unittest.TestCase): @@ -111,6 +120,7 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'acquired session lock as a leader') def test_promoted_by_acquiring_lock(self): + self.ha.is_healthiest_node = true self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') @@ -119,16 +129,17 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'demoted self due after trying and failing to obtain lock') def test_follow_new_leader_after_failing_to_obtain_lock(self): + self.ha.is_healthiest_node = true self.ha.acquire_lock = false self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'following new leader after trying and failing to obtain lock') def test_demote_because_not_healthiest(self): - self.p.is_healthiest_node = false + self.ha.is_healthiest_node = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i am not the healthiest node') def test_follow_new_leader_because_not_healthiest(self): - self.p.is_healthiest_node = false + self.ha.is_healthiest_node = false self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') @@ -221,3 +232,52 @@ class TestHa(unittest.TestCase): self.ha.update_lock = false self.assertEquals(self.ha.run_cycle(), 'failed to update leader lock during restart') + + @patch('requests.get', requests_get) + def test_manual_failover_from_leader(self): + self.ha.has_lock = true + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '')) + self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name)) + self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla')) + self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') + f = Failover(0, MockPostgresql.name, '') + self.ha.cluster = get_cluster_initialized_with_leader(f) + self.assertEquals(self.ha.run_cycle(), 'manual failover: demoted self but failed to release leader lock') + self.ha.cluster = get_cluster_initialized_with_leader(f) + self.e.client.delete = Mock(return_value=True) + self.assertEquals(self.ha.run_cycle(), 'manual failover: demoted self and released leader lock') + + @patch('requests.get', requests_get) + def test_manual_failover_process_no_leader(self): + self.p.is_leader = false + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name)) + self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader')) + self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') + self.ha.fetch_node_status = lambda e: (e, True, True, 0) # accessible, in_recovery + self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '')) + self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') + self.ha.fetch_node_status = lambda e: (e, False, True, 0) # accessible, in_recovery + self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') + + def test__is_healthiest_node(self): + self.assertTrue(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + self.p.is_leader = false + self.ha.fetch_node_status = lambda e: (e, True, True, 0) # accessible, in_recovery + self.assertTrue(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + self.ha.fetch_node_status = lambda e: (e, True, False, 0) # accessible, not in_recovery + self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + self.ha.fetch_node_status = lambda e: (e, True, True, 1) # accessible, in_recovery, xlog location ahead + self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + self.p.check_replication_lag = false + self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + + @patch('requests.get', requests_get) + def test_fetch_node_status(self): + member = Member(0, 'test', '', 'http://127.0.0.1:8011/patroni', None, None) + self.ha.fetch_node_status(member) + member = Member(0, 'test', '', 'http://localhost:8011/patroni', None, None) + self.ha.fetch_node_status(member) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 7cac2eba..8936d18b 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -48,7 +48,6 @@ class TestPatroni(unittest.TestCase): self.assertRaises(Exception, self.p.get_dcs, '', {}) @patch('time.sleep', Mock(side_effect=SleepException())) - @patch.object(Patroni, 'initialize', Mock()) @patch.object(Etcd, 'delete_leader', Mock()) @patch.object(Client, 'machines') def test_patroni_main(self, mock_machines): @@ -84,13 +83,9 @@ class TestPatroni(unittest.TestCase): now = datetime.datetime.utcnow() member = Member(0, self.p.postgresql.name, 'b', 'c', (now + datetime.timedelta( seconds=self.p.shutdown_member_ttl + 10)).strftime('%Y-%m-%dT%H:%M:%S.%fZ'), None) - self.p.ha.cluster = Cluster(True, member, 0, [member]) + self.p.ha.cluster = Cluster(True, member, 0, [member], None) self.p.touch_member() - def test_patroni_initialize(self): - self.p.touch_member = self.touch_member - self.p.initialize() - def test_schedule_next_run(self): self.p.ha.dcs.watch = Mock(return_value=True) self.p.schedule_next_run() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index d1ab7e8d..9fe012a7 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -25,18 +25,9 @@ class MockCursor: raise RetryFailedError('retry') elif sql.startswith('SELECT slot_name'): self.results = [('blabla',), ('foobar',)] - elif sql.startswith('SELECT pg_current_xlog_location()'): - self.results = [(0,)] - elif sql.startswith('SELECT pg_is_in_recovery(), %s'): - if params[0][0] == 1: - raise psycopg2.OperationalError() - elif params[0][0] == 2: - self.results = [(True, -1)] - else: - self.results = [(False, 0)] elif sql.startswith('SELECT pg_xlog_location_diff'): self.results = [(0,)] - elif sql.startswith('SELECT pg_is_in_recovery()'): + elif sql == 'SELECT pg_is_in_recovery()': self.results = [(False, )] elif sql.startswith('SELECT to_char(pg_postmaster_start_time'): self.results = [('', True, '', '', '', False)] @@ -164,7 +155,7 @@ class TestPostgresql(unittest.TestCase): def test_sync_replication_slots(self): self.p.start() - cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem]) + cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem], None) self.p.sync_replication_slots(cluster) @patch.object(MockConnect, 'closed', 2) @@ -178,17 +169,8 @@ class TestPostgresql(unittest.TestCase): self.assertRaises(PostgresConnectionException, self.p.query, 'RetryFailedError') self.assertRaises(psycopg2.OperationalError, self.p.query, 'blabla') - def test_is_healthiest_node(self): - cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem]) - self.assertTrue(self.p.is_healthiest_node(cluster)) - self.p.is_leader = false - self.assertFalse(self.p.is_healthiest_node(cluster)) - self.p.xlog_position = lambda: 1 - self.assertTrue(self.p.is_healthiest_node(cluster)) - self.p.xlog_position = lambda: 2 - self.assertFalse(self.p.is_healthiest_node(cluster)) - self.p.config['maximum_lag_on_failover'] = -3 - self.assertFalse(self.p.is_healthiest_node(cluster)) + def test_is_leader(self): + self.assertTrue(self.p.is_leader()) def test_reload(self): self.assertTrue(self.p.reload()) @@ -218,6 +200,9 @@ class TestPostgresql(unittest.TestCase): self.p.query = Mock(side_effect=psycopg2.OperationalError("not supported")) self.assertTrue(self.p.stop()) + def test_check_replication_lag(self): + self.assertTrue(self.p.check_replication_lag(0)) + @patch('os.rename', Mock()) @patch('os.path.isdir', Mock(return_value=True)) def test_move_data_directory(self): diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index b04f5b5a..039faa58 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -7,7 +7,7 @@ from patroni.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperErr from kazoo.client import KazooState from kazoo.exceptions import NoNodeError, NodeExistsError from kazoo.protocol.states import ZnodeStat -from test_etcd import MockPostgresql, SleepException, requests_get +from test_etcd import SleepException, requests_get class MockKazooClient(Mock): @@ -31,7 +31,7 @@ class MockKazooClient(Mock): elif '/members/' in path: return ( b'postgres://repuser:rep-pass@localhost:5434/postgres?application_name=http://127.0.0.1:8009/patroni', - ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0 if self.exists else -1, 0, 0, 0) ) elif path.endswith('/optime/leader'): return (b'1', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) @@ -41,6 +41,7 @@ class MockKazooClient(Mock): return (b'foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) elif path.endswith('/initialize'): return (b'foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) + return (b'', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) def get_children(self, path, watch=None, include_data=False): if not isinstance(path, six.string_types): @@ -48,7 +49,7 @@ class MockKazooClient(Mock): if path == '/no_node': raise NoNodeError elif path in ['/service/bla/', '/service/test/']: - return ['initialize', 'leader', 'members', 'optime'] + return ['initialize', 'leader', 'members', 'optime', 'failover'] return ['foo', 'bar', 'buzz'] def create(self, path, value=b"", acl=None, ephemeral=False, sequence=False, makepath=False): @@ -68,6 +69,11 @@ class MockKazooClient(Mock): raise TypeError("Invalid type for 'value' (must be a byte string)") if path == '/service/bla/optime/leader': raise Exception + if path == '/service/test/failover': + if value == b'Exception': + raise Exception + elif value == b'ok': + return raise NoNodeError def delete(self, path, version=-1, recursive=False): @@ -119,6 +125,11 @@ class TestZooKeeper(unittest.TestCase): self.zk.touch_member('foo') self.zk.delete_leader() + def test_set_failover_value(self): + self.zk.set_failover_value('') + self.zk.set_failover_value('ok') + self.zk.set_failover_value('Exception') + def test_initialize(self): self.assertFalse(self.zk.initialize()) @@ -129,6 +140,8 @@ class TestZooKeeper(unittest.TestCase): self.zk.touch_member('new') self.zk.touch_member('exists') self.zk.touch_member('retry') + self.zk.client.exists = True + self.zk.touch_member('retry') def test_take_leader(self): self.zk.take_leader() From a25976445804df4f4f16f15827ab2026f5fe54e6 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 29 Sep 2015 08:39:05 +0200 Subject: [PATCH 04/15] Suppress logging from API when postgres is being bootstrapped/initialized --- patroni/api.py | 6 ++++-- patroni/postgresql.py | 45 ++++++++++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/patroni/api.py b/patroni/api.py index d507bfb3..580d9fe8 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -161,8 +161,10 @@ class RestApiHandler(BaseHTTPRequestHandler): }) } except (psycopg2.Error, RetryFailedError, PostgresConnectionException): - logger.exception('get_postgresql_status') - return {'state': self.server.patroni.postgresql.state} + state = self.server.patroni.postgresql.state + if state in ['stopped', 'starting', 'stopping', 'restarting', 'running']: + logger.exception('get_postgresql_status') + return {'state': state} class RestApiServer(ThreadingMixIn, HTTPServer, Thread): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index bc3188ed..97d37515 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -9,6 +9,7 @@ import time from patroni.exceptions import PostgresConnectionException, PostgresException from patroni.utils import Retry, RetryFailedError from six.moves.urllib_parse import urlparse +from threading import Lock logger = logging.getLogger(__name__) @@ -70,7 +71,9 @@ class Postgresql: self.retry = Retry(max_tries=-1, deadline=10, max_delay=1, retry_exceptions=PostgresConnectionException) self._state = 'stopped' + self._state_lock = Lock() self._role = 'replica' + self._role_lock = Lock() if self.is_running(): self._state = 'running' @@ -121,12 +124,12 @@ class Postgresql: return not os.path.exists(self.data_dir) or os.listdir(self.data_dir) == [] def initialize(self): - self._state = 'initalizing new cluster' + self.set_state('initalizing new cluster') ret = subprocess.call(self._pg_ctl + ['initdb', '-o', '--encoding=UTF8']) == 0 if ret: self.write_pg_hba() else: - self._state = 'initdb failed' + self.set_state('initdb failed') return ret def delete_trigger_file(self): @@ -149,7 +152,7 @@ class Postgresql: return "host={host} port={port} user={user}".format(**conn) def create_replica(self, master_connection, env): - self._state = 'building replica from {host}:{port}'.format(**master_connection) + self.set_state('building replica from {host}:{port}'.format(**master_connection)) connstring = self.build_connstring(master_connection) cmd = self.config['restore'] try: @@ -159,7 +162,7 @@ class Postgresql: logger.exception('Error when creating replica') ret = 1 if ret != 0: - self._state = 'failed to build replica from {host}:{port}'.format(**master_connection) + self.set_state('failed to build replica from {host}:{port}'.format(**master_connection)) return ret def is_leader(self): @@ -182,28 +185,38 @@ class Postgresql: @property def role(self): - return self._role + with self._role_lock: + return self._role + + def set_role(self, value): + with self._role_lock: + self._role = value @property def state(self): - return self._state + with self._state_lock: + return self._state + + def set_state(self, value): + with self._state_lock: + self._state = value def start(self, block_callbacks=False): if self.is_running(): logger.error('Cannot start PostgreSQL because one is already running.') return True - self._role = 'replica' if os.path.exists(self.recovery_conf) else 'master' + self.set_role('replica' if os.path.exists(self.recovery_conf) else 'master') if os.path.exists(self.postmaster_pid): os.remove(self.postmaster_pid) logger.info('Removed %s', self.postmaster_pid) if not block_callbacks: - self._state = 'starting' + self.set_state('starting') ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()]) == 0 - self._state = 'running' if ret else 'start failed' + self.set_state('running' if ret else 'start failed') self.schedule_load_slots = ret and self.use_slots self.save_configuration_files() @@ -222,21 +235,21 @@ class Postgresql: def stop(self, mode='fast', block_callbacks=False): if not self.is_running(): if not block_callbacks: - self._state = 'stopped' + self.set_state('stopped') return True if block_callbacks: self.checkpoint() else: - self._state = 'stopping' + self.set_state('stopping') ret = subprocess.call(self._pg_ctl + ['stop', '-m', mode]) == 0 # block_callbacks is used during restart to avoid # running start/stop callbacks in addition to restart ones if not ret: - self._state = 'stop failed' + self.set_state('stop failed') elif not block_callbacks: - self._state = 'stopped' + self.set_state('stopped') self.call_nowait(ACTION_ON_STOP) return ret @@ -246,12 +259,12 @@ class Postgresql: return ret def restart(self): - self._state = 'restarting' + self.set_state('restarting') ret = self.stop(block_callbacks=True) and self.start(block_callbacks=True) if ret: self.call_nowait(ACTION_ON_RESTART) else: - self._state = 'restart failed ({})'.format(self._state) + self.set_state('restart failed ({})'.format(self.state)) return ret def server_options(self): @@ -337,7 +350,7 @@ recovery_target_timeline = 'latest' return True ret = subprocess.call(self._pg_ctl + ['promote']) == 0 if ret: - self._role = 'master' + self.set_role('master') self.call_nowait(ACTION_ON_ROLE_CHANGE) return ret From 0572fec6a3b77137e1400adede3affddfae87257 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 29 Sep 2015 12:59:26 +0200 Subject: [PATCH 05/15] remove leader lock after stop of postgres to speed up failover --- patroni/ha.py | 12 +++++++----- patroni/postgresql.py | 4 ++-- tests/test_postgresql.py | 5 ++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 628b9c41..a5f68068 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -214,12 +214,14 @@ class Ha: if not failover.member or failover.member != self.state_handler.name: members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] if self.is_failover_possible(members): # check that there are healthy members + self.state_handler.stop() + if self.dcs.delete_leader(): + ret = 'manual failover: demoted self and released leader lock' + else: + ret = 'manual failover: demoted self but failed to release leader lock' self.state_handler.follow_the_leader(None) self.cluster = None - if self.dcs.delete_leader(): - return 'manual failover: demoted self and released leader lock' - else: - return 'manual failover: demoted self but failed to release leader lock' + return ret else: logger.warning('manual failover: no healthy members found, failover is not possible') else: @@ -360,7 +362,7 @@ class Ha: except DCSError: logger.error('Error communicating with DCS') if self.state_handler.is_running() and self.state_handler.is_leader(): - self.state_handler.demote(None) + self.state_handler.demote() return 'demoted self because DCS is not accessible and i was a leader' except (psycopg2.Error, PostgresConnectionException): logger.exception('Error communicating with Postgresql. Will try again') diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 97d37515..b2b93f72 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -354,8 +354,8 @@ recovery_target_timeline = 'latest' self.call_nowait(ACTION_ON_ROLE_CHANGE) return ret - def demote(self, leader): - self.follow_the_leader(leader) + def demote(self): + self.follow_the_leader(None) def create_replication_user(self): self.query('CREATE USER "{}" WITH REPLICATION ENCRYPTED PASSWORD %s'.format( diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 9fe012a7..f1ddea8e 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -137,9 +137,8 @@ class TestPostgresql(unittest.TestCase): self.assertTrue(self.p.sync_from_leader(self.leader)) def test_follow_the_leader(self): - self.p.demote(self.leader) - self.p.follow_the_leader(None) - self.p.demote(self.leader) + self.p.follow_the_leader(self.leader) + self.p.demote() self.p.follow_the_leader(self.leader) self.p.follow_the_leader(Leader(-1, None, 28, self.other)) From 1997f15a7a845457f0a47a14c960ae0acb420c3f Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 30 Sep 2015 17:08:15 +0200 Subject: [PATCH 06/15] Run long time operations asynchronously i.e. restart, reinitialize, demote --- patroni/__init__.py | 6 +- patroni/api.py | 24 +++--- patroni/ha.py | 157 ++++++++++++++++++++++++--------------- patroni/postgresql.py | 10 ++- patroni/zookeeper.py | 2 + tests/test_api.py | 6 +- tests/test_ha.py | 50 +++++++++---- tests/test_patroni.py | 2 + tests/test_postgresql.py | 7 ++ 9 files changed, 161 insertions(+), 103 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 6ca96df0..c5a57a67 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -56,12 +56,8 @@ class Patroni: self.next_run = time.time() while True: - logger.info(self.ha.run_cycle()) - try: - self.ha.cluster and self.ha.state_handler.sync_replication_slots(self.ha.cluster) - except: - logger.exception('Exception when changing replication slots') self.touch_member() + logger.info(self.ha.run_cycle()) reap_children() self.schedule_next_run() diff --git a/patroni/api.py b/patroni/api.py index 580d9fe8..d1cee49f 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -71,19 +71,14 @@ class RestApiHandler(BaseHTTPRequestHandler): @check_auth def do_POST_restart(self): - action = self.server.patroni.ha.schedule_restart() - if action is not None: - status_code = 503 - data = (action + ' already in progress').encode('utf-8') - else: - status_code = 503 - data = b'restart failed' - try: - if self.server.patroni.ha.restart(): - status_code = 200 - data = b'restarted successfully' - except: - logger.exception('Exception during restart') + status_code = 503 + data = b'restart failed' + try: + status, msg = self.server.patroni.ha.restart() + status_code = 200 if status else 503 + data = msg.encode('utf-8') + except: + logger.exception('Exception during restart') self.send_response(status_code) self.send_header('Content-Type', 'text/html') @@ -135,7 +130,7 @@ class RestApiHandler(BaseHTTPRequestHandler): def query(self, sql, *params, **kwargs): if not kwargs.get('retry', False): return self.server.query(sql, *params) - retry = Retry(delay=2, retry_exceptions=PostgresConnectionException) + retry = Retry(delay=1, retry_exceptions=PostgresConnectionException) return retry(self.server.query, sql, *params) def get_postgresql_status(self, retry=False): @@ -164,6 +159,7 @@ class RestApiHandler(BaseHTTPRequestHandler): state = self.server.patroni.postgresql.state if state in ['stopped', 'starting', 'stopping', 'restarting', 'running']: logger.exception('get_postgresql_status') + state = 'unknown' if state == 'running' else state return {'state': state} diff --git a/patroni/ha.py b/patroni/ha.py index a5f68068..d791ea87 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -4,7 +4,7 @@ import requests from patroni.exceptions import DCSError, PostgresConnectionException from multiprocessing.pool import ThreadPool -from threading import Lock +from threading import Lock, Thread logger = logging.getLogger(__name__) @@ -16,10 +16,10 @@ class Ha: self.dcs = dcs self.cluster = None self.old_cluster = None - self.scheduled_action = None - self.scheduled_action_lock = Lock() - self.restart_in_progress = False - self.restart_thread_lock = Lock() + self._scheduled_action = None + self._scheduled_action_lock = Lock() + self._long_action_in_progress = False + self._long_action_thread_lock = Lock() def load_cluster_from_dcs(self): cluster = self.dcs.get_cluster() @@ -48,16 +48,37 @@ class Ha: logger.info('Lock owner: %s; I am %s', lock_owner, self.state_handler.name) return lock_owner == self.state_handler.name + def _run_async(self, func, args=()): + try: + return func(*args) if args else func() + except: + logger.exception('Exception during execution of long running task %s', self.get_scheduled_action()) + finally: + with self._long_action_thread_lock: + self._long_action_in_progress = False + self._reset_scheduled_action() + + def run_async(self, func, args=()): + self._long_action_in_progress = True + Thread(target=self._run_async, args=(func, args)).start() + + def copy_backup_from_leader(self): + if self.state_handler.bootstrap(self.cluster.leader): + logger.info('bootstrapped from leader') + else: + self.state_handler.stop('immediate') + self.state_handler.remove_data_directory() + logger.error('failed to bootstrap from leader') + def bootstrap(self): if not self.cluster.is_unlocked(): # cluster already has leader - logger.info('trying to bootstrap from leader') - if self.state_handler.bootstrap(self.cluster.leader): - self.reinitialize_scheduled() and self.reset_scheduled_action() - return 'bootstrapped from leader' + if self._long_action_in_progress: + self.copy_backup_from_leader() else: - self.state_handler.stop('immediate') - self.state_handler.remove_data_directory() - return 'failed to bootstrap from leader' + with self._scheduled_action_lock: + self._scheduled_action = 'bootstrap from leader' + self.run_async(self.copy_backup_from_leader) + return 'trying to bootstrap from leader' elif not self.cluster.initialize: # no initialize key if self.dcs.initialize(): # race for initialization try: @@ -92,7 +113,10 @@ class Ha: def follow_the_leader(self, demote_reason, follow_reason, refresh=True): refresh and self.load_cluster_from_dcs() ret = demote_reason if self.state_handler.is_leader() else follow_reason - self.state_handler.follow_the_leader(self.cluster.leader) + if not self.state_handler.check_recovery_conf(self.cluster.leader): + with self._scheduled_action_lock: + self._scheduled_action = 'changing primary_conninfo and restarting' + self.run_async(self.state_handler.follow_the_leader, (self.cluster.leader, )) return ret def enforce_master_role(self, message, promote_message): @@ -208,20 +232,22 @@ class Ha: members = {m.name: m for m in self.old_cluster.members + self.cluster.members} return self._is_healthiest_node(members.values()) + def demote(self, delete_leader=True): + if delete_leader: + self.state_handler.stop() + self.dcs.delete_leader() + self.state_handler.follow_the_leader(None) + def process_manual_failover_from_leader(self): failover = self.cluster.failover if not failover.leader or failover.leader == self.state_handler.name: if not failover.member or failover.member != self.state_handler.name: members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] if self.is_failover_possible(members): # check that there are healthy members - self.state_handler.stop() - if self.dcs.delete_leader(): - ret = 'manual failover: demoted self and released leader lock' - else: - ret = 'manual failover: demoted self but failed to release leader lock' - self.state_handler.follow_the_leader(None) - self.cluster = None - return ret + with self._scheduled_action_lock: + self._scheduled_action = 'manual failover: demote' + self.run_async(self.demote) + return 'manual failover: demoting myself' else: logger.warning('manual failover: no healthy members found, failover is not possible') else: @@ -268,22 +294,20 @@ class Ha: 'no action. i am a secondary and i am following a leader', False) def schedule_action(self, action): - with self.scheduled_action_lock: - if self.scheduled_action is not None: - return self.scheduled_action - self.scheduled_action = action + with self._long_action_thread_lock: + with self._scheduled_action_lock: + if self._scheduled_action is not None: + return self._scheduled_action + self._scheduled_action = action return None def get_scheduled_action(self): - with self.scheduled_action_lock: - return self.scheduled_action + with self._scheduled_action_lock: + return self._scheduled_action - def reset_scheduled_action(self): - with self.scheduled_action_lock: - self.scheduled_action = None - - def schedule_restart(self): - return self.schedule_action('restart') + def _reset_scheduled_action(self): + with self._scheduled_action_lock: + self._scheduled_action = None def restart_scheduled(self): return self.get_scheduled_action() == 'restart' @@ -295,38 +319,45 @@ class Ha: return self.get_scheduled_action() == 'reinitialize' def restart(self): - with self.restart_thread_lock: - self.restart_in_progress = True - try: - return self.state_handler.restart() - finally: - with self.restart_thread_lock: - self.restart_in_progress = False - self.reset_scheduled_action() + with self._long_action_thread_lock: + with self._scheduled_action_lock: + if self._scheduled_action is not None: + return False, self._scheduled_action + ' already in progress' + self._scheduled_action = 'restart' + self._long_action_in_progress = True + if self._run_async(self.state_handler.restart): + return True, 'restarted successfully' + else: + return False, 'restart failed' + + def reinitialize(self): + self.state_handler.stop('immediate') + self.state_handler.remove_data_directory() + self.load_cluster_from_dcs() + self.bootstrap() def process_scheduled_action(self): if self.reinitialize_scheduled(): if self.cluster.is_unlocked(): logger.error('Cluster has no leader, can not reinitialize') - self.reset_scheduled_action() + self._reset_scheduled_action() elif self.has_lock(): logger.error('I am the leader, can not reinitialize') - self.reset_scheduled_action() + self._reset_scheduled_action() else: - self.state_handler.stop('immediate') - self.state_handler.remove_data_directory() - self.load_cluster_from_dcs() + self.run_async(self.reinitialize) + return True - def handle_restart_in_progress(self): + def handle_long_action_in_progress(self): if self.has_lock(): if self.update_lock(): - return 'updated leader lock during restart' + return 'updated leader lock during ' + self.get_scheduled_action() else: - return 'failed to update leader lock during restart' + return 'failed to update leader lock during ' + self.get_scheduled_action() elif self.cluster.is_unlocked(): return 'not healthy enough for leader race' else: - return 'restart in progress' + return self.get_scheduled_action() + ' in progress' def _run_cycle(self): try: @@ -336,8 +367,12 @@ class Ha: if not self.cluster.is_unlocked() and not self.cluster.initialize: self.dcs.initialize() # fix it + if self._long_action_in_progress: + return self.handle_long_action_in_progress() + # currently it can trigger only reinitialize - self.process_scheduled_action() + if self.process_scheduled_action(): + return 'reinitialize started' # is data directory empty? if self.state_handler.data_directory_empty(): @@ -346,27 +381,27 @@ class Ha: elif not self.cluster.initialize and self.cluster.is_unlocked(): self.dcs.initialize() - if self.restart_in_progress: - return self.handle_restart_in_progress() - # try to start dead postgres if not self.state_handler.is_healthy(): msg = self.recover() if msg is not None: return msg - if self.cluster.is_unlocked(): - return self.process_unhealthy_cluster() - else: - return self.process_healthy_cluster() + try: + if self.cluster.is_unlocked(): + return self.process_unhealthy_cluster() + else: + return self.process_healthy_cluster() + finally: + self.state_handler.sync_replication_slots(self.cluster) except DCSError: logger.error('Error communicating with DCS') if self.state_handler.is_running() and self.state_handler.is_leader(): - self.state_handler.demote() + self.demote(delete_leader=False) return 'demoted self because DCS is not accessible and i was a leader' except (psycopg2.Error, PostgresConnectionException): - logger.exception('Error communicating with Postgresql. Will try again') + logger.exception('Error communicating with Postgresql. Will try again later') def run_cycle(self): - with self.restart_thread_lock: + with self._long_action_thread_lock: return self._run_cycle() diff --git a/patroni/postgresql.py b/patroni/postgresql.py index b2b93f72..816474a3 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -227,10 +227,14 @@ class Postgresql: def checkpoint(self): try: - self.query('SET statement_timeout TO 0') - self.query('CHECKPOINT') + r = parseurl('postgres://{}/postgres'.format(self.local_address)) + r['options'] = '-c statement_timeout=0' + with psycopg2.connect(**r) as conn: + conn.autocommit = True + with conn.cursor() as cur: + cur.execute('CHECKPOINT') except: - logging.exception('Exception diring CHECKPOINT') + logging.exception('Exception during CHECKPOINT') def stop(self, mode='fast', block_callbacks=False): if not self.is_running(): diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 7458ae46..4b3389e1 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -225,6 +225,8 @@ class ZooKeeper(AbstractDCS): return True except: logger.exception('touch_member') + except: + logger.exception('touch_member') return False def take_leader(self): diff --git a/tests/test_api.py b/tests/test_api.py index 4bb481bf..5003e43e 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -33,7 +33,7 @@ class MockHa(Mock): return 'reinitialize' def restart(self): - return True + return (True, '') def restart_scheduled(self): return False @@ -85,10 +85,8 @@ class TestRestApiHandler(unittest.TestCase): def test_do_POST_restart(self): request = b'POST /restart HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' MockRestApiServer(RestApiHandler, request) - with patch.object(MockHa, 'schedule_restart', Mock(return_value=None)): + with patch.object(MockHa, 'restart', Mock(side_effect=Exception)): MockRestApiServer(RestApiHandler, request) - with patch.object(MockHa, 'restart', Mock(side_effect=Exception)): - MockRestApiServer(RestApiHandler, request) @patch.object(MockHa, 'dcs') def test_do_POST_reinitialize(self, dcs): diff --git a/tests/test_ha.py b/tests/test_ha.py index 6e50c629..52faba35 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -6,6 +6,7 @@ from patroni.etcd import Client, Etcd from patroni.exceptions import DCSError, PostgresException from patroni.ha import Ha from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get +from threading import Thread def true(*args, **kwargs): @@ -69,6 +70,13 @@ class MockPostgresql(Mock): def check_replication_lag(self, last_leader_operation): return True + def check_recovery_conf(self, leader): + return False + + +def run_async(func, args=()): + func(args) if args else func() + class TestHa(unittest.TestCase): @@ -81,6 +89,7 @@ class TestHa(unittest.TestCase): self.e.client.read = etcd_read self.e.client.write = etcd_write self.ha = Ha(self.p, self.e) + self.ha.run_async = run_async self.ha.load_cluster_from_dcs() self.ha.cluster = get_cluster_not_initialized_without_leader() self.ha.load_cluster_from_dcs = Mock() @@ -173,14 +182,20 @@ class TestHa(unittest.TestCase): self.ha.load_cluster_from_dcs = Mock(side_effect=DCSError('Etcd is not responding properly')) self.assertEquals(self.ha.run_cycle(), 'demoted self because DCS is not accessible and i was a leader') + def test__run_async(self): + self.ha._run_async(Mock(side_effect=Exception())) + + @patch.object(Thread, 'start', Mock()) + def test_run_async(self): + ha = Ha(self.p, self.e) + ha.run_async(true) + def test_bootstrap_from_leader(self): - self.ha.cluster = get_cluster_initialized_with_leader() - self.assertEquals(self.ha.bootstrap(), 'bootstrapped from leader') - - def test_bootstrap_from_leader_failed(self): self.ha.cluster = get_cluster_initialized_with_leader() self.p.bootstrap = false - self.assertEquals(self.ha.bootstrap(), 'failed to bootstrap from leader') + self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') + self.ha._long_action_in_progress = True + self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') def test_bootstrap_waiting_for_leader(self): self.ha.cluster = get_cluster_initialized_without_leader() @@ -202,26 +217,32 @@ class TestHa(unittest.TestCase): self.assertRaises(PostgresException, self.ha.bootstrap) def test_reinitialize(self): + self.ha.schedule_reinitialize() self.ha.schedule_reinitialize() self.ha.run_cycle() self.assertIsNone(self.ha.get_scheduled_action()) self.ha.cluster = get_cluster_initialized_with_leader() - self.ha.schedule_reinitialize() - self.ha.run_cycle() - self.ha.has_lock = true self.ha.schedule_reinitialize() self.ha.run_cycle() self.assertIsNone(self.ha.get_scheduled_action()) + self.ha.has_lock = false + self.ha.schedule_reinitialize() + self.ha.run_cycle() + def test_restart(self): - self.ha.schedule_restart() - self.assertTrue(self.ha.restart_scheduled()) - self.ha.restart() + self.assertEquals(self.ha.restart(), (True, 'restarted successfully')) + self.p.restart = false + self.assertEquals(self.ha.restart(), (False, 'restart failed')) + self.ha.schedule_reinitialize() + self.assertEquals(self.ha.restart(), (False, 'reinitialize already in progress')) def test_restart_in_progress(self): - self.ha.restart_in_progress = True + self.ha._long_action_in_progress = True + self.ha._scheduled_action = 'restart' + self.assertTrue(self.ha.restart_scheduled()) self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race') self.ha.cluster = get_cluster_initialized_with_leader() @@ -244,10 +265,7 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') f = Failover(0, MockPostgresql.name, '') self.ha.cluster = get_cluster_initialized_with_leader(f) - self.assertEquals(self.ha.run_cycle(), 'manual failover: demoted self but failed to release leader lock') - self.ha.cluster = get_cluster_initialized_with_leader(f) - self.e.client.delete = Mock(return_value=True) - self.assertEquals(self.ha.run_cycle(), 'manual failover: demoted self and released leader lock') + self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself') @patch('requests.get', requests_get) def test_manual_failover_process_no_leader(self): diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 8936d18b..2c7998c4 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -8,6 +8,7 @@ from mock import Mock, patch from patroni.api import RestApiServer from patroni.dcs import Cluster, Member from patroni.etcd import Etcd +from patroni.ha import Ha from patroni import Patroni, main from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer @@ -26,6 +27,7 @@ def time_sleep(*args): @patch.object(Postgresql, 'write_pg_hba', Mock()) @patch.object(Postgresql, 'write_recovery_conf', Mock()) @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock()) +@patch.object(Ha, 'run_async', Mock()) class TestPatroni(unittest.TestCase): @patch.object(Client, 'machines') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index f1ddea8e..e7e39594 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -70,6 +70,12 @@ class MockConnect(Mock): def cursor(self): return MockCursor(self) + def __enter__(self): + return self + + def __exit__(self, *args): + pass + def psycopg2_connect(*args, **kwargs): return MockConnect() @@ -214,6 +220,7 @@ class TestPostgresql(unittest.TestCase): with patch('subprocess.call', Mock(return_value=1)): self.assertRaises(PostgresException, self.p.bootstrap) self.p.bootstrap() + self.p.bootstrap(self.leader) def test_remove_data_directory(self): self.p.data_dir = 'data_dir' From a6cb7563e57178b76499ffd95ddb335181f396a0 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 1 Oct 2015 08:06:00 +0200 Subject: [PATCH 07/15] catch all exceptions in change_replication_slots method --- patroni/postgresql.py | 29 ++++++++++++++++------------- tests/test_patroni.py | 1 - tests/test_postgresql.py | 3 +++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 816474a3..e15a4996 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -391,21 +391,24 @@ recovery_target_timeline = 'latest' def sync_replication_slots(self, cluster): if self.use_slots: - self.load_replication_slots() - slots = [m.name for m in cluster.members if m.name != self.name] if self.role == 'master' else [] - # drop unused slots - for slot in set(self.replication_slots) - set(slots): - self.query("""SELECT pg_drop_replication_slot(%s) - WHERE EXISTS(SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + try: + self.load_replication_slots() + slots = [m.name for m in cluster.members if m.name != self.name] if self.role == 'master' else [] + # drop unused slots + for slot in set(self.replication_slots) - set(slots): + self.query("""SELECT pg_drop_replication_slot(%s) + WHERE EXISTS(SELECT 1 FROM pg_replication_slots + WHERE slot_name = %s)""", slot, slot) - # create new slots - for slot in set(slots) - set(self.replication_slots): - self.query("""SELECT pg_create_physical_replication_slot(%s) - WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + # create new slots + for slot in set(slots) - set(self.replication_slots): + self.query("""SELECT pg_create_physical_replication_slot(%s) + WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots + WHERE slot_name = %s)""", slot, slot) - self.replication_slots = slots + self.replication_slots = slots + except: + logger.exception('Exception when changing replication slots') def last_operation(self): return str(self.xlog_position()) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 2c7998c4..1c82799b 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -66,7 +66,6 @@ class TestPatroni(unittest.TestCase): @patch('time.sleep', Mock(side_effect=SleepException())) def test_run(self): self.p.touch_member = self.touch_member - self.p.ha.state_handler.sync_replication_slots = time_sleep self.p.ha.dcs.watch = time_sleep self.assertRaises(SleepException, self.p.run) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index e7e39594..90874288 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -162,6 +162,9 @@ class TestPostgresql(unittest.TestCase): self.p.start() cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem], None) self.p.sync_replication_slots(cluster) + self.p.query = Mock(side_effect=psycopg2.OperationalError) + self.p.schedule_load_slots = True + self.p.sync_replication_slots(cluster) @patch.object(MockConnect, 'closed', 2) def test__query(self): From d09875a056bbbe4977150f84391f8c378978e8b2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 1 Oct 2015 17:06:42 +0200 Subject: [PATCH 08/15] refactoring: 1. run touch_member from the main loop 2. move code which takes care about long tasks into separate class 3. change format of data stored in a DCS: use json instead of url 4. change Member class: from now it deserialize everything into data property 5. rework API: from now it takes into account state of the current node in a dcs --- patroni/__init__.py | 19 +--- patroni/api.py | 13 ++- patroni/async_executor.py | 55 ++++++++++++ patroni/dcs.py | 44 ++++++--- patroni/etcd.py | 10 +-- patroni/ha.py | 183 +++++++++++++++++--------------------- patroni/postgresql.py | 4 +- patroni/utils.py | 2 + patroni/zookeeper.py | 45 ++++++---- tests/test_api.py | 1 + tests/test_etcd.py | 9 -- tests/test_ha.py | 99 ++++++++++----------- tests/test_patroni.py | 27 ++---- tests/test_postgresql.py | 10 +-- tests/test_zookeeper.py | 17 +++- 15 files changed, 298 insertions(+), 240 deletions(-) create mode 100644 patroni/async_executor.py diff --git a/patroni/__init__.py b/patroni/__init__.py index c5a57a67..150334ba 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -19,11 +19,11 @@ class Patroni: def __init__(self, config): self.nap_time = config['loop_wait'] self.postgresql = Postgresql(config['postgresql']) - self.ha = Ha(self.postgresql, self.get_dcs(self.postgresql.name, config)) + self.dcs = self.get_dcs(self.postgresql.name, config) host, port = config['restapi']['listen'].split(':') self.api = RestApiServer(self, config['restapi']) + self.ha = Ha(self) self.next_run = time.time() - self.shutdown_member_ttl = 300 @staticmethod def get_dcs(name, config): @@ -33,22 +33,13 @@ class Patroni: return ZooKeeper(name, config['zookeeper']) raise Exception('Can not find sutable configuration of distributed configuration store') - def touch_member(self, ttl=None): - connection_string = self.postgresql.connection_string + '?application_name=' + self.api.connection_string - if self.ha.cluster: - for m in self.ha.cluster.members: - # Do not update member TTL when it is far from being expired - if m.name == self.postgresql.name and m.real_ttl() > self.shutdown_member_ttl: - return True - return self.ha.dcs.touch_member(connection_string, ttl) - def schedule_next_run(self): self.next_run += self.nap_time current_time = time.time() nap_time = self.next_run - current_time if nap_time <= 0: self.next_run = current_time - elif self.ha.dcs.watch(nap_time): + elif self.dcs.watch(nap_time): self.next_run = time.time() def run(self): @@ -56,7 +47,6 @@ class Patroni: self.next_run = time.time() while True: - self.touch_member() logger.info(self.ha.run_cycle()) reap_children() self.schedule_next_run() @@ -81,6 +71,5 @@ def main(): pass finally: patroni.api.shutdown() - patroni.touch_member(patroni.shutdown_member_ttl) # schedule member removal patroni.postgresql.stop() - patroni.ha.dcs.delete_leader() + patroni.dcs.delete_leader() diff --git a/patroni/api.py b/patroni/api.py index d1cee49f..0333e86c 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -48,7 +48,18 @@ class RestApiHandler(BaseHTTPRequestHandler): response = self.get_postgresql_status() patroni = self.server.patroni - if 'role' in response and response['role'] in path: + if patroni.dcs.cluster: # dcs available + if patroni.dcs.cluster.leader and patroni.dcs.cluster.leader.name == patroni.postgresql.name: # is_leader + status_code = 200 if 'master' in path else 503 + elif 'role' not in response: + status_code = 503 + elif response['role'] == 'master': # running as master but without leader lock!!!! + status_code = 503 + elif response['role'] in path: + status_code = 200 + else: + status_code = 503 + elif 'role' in response and response['role'] in path: status_code = 200 elif patroni.ha.restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path: # exceptional case for master node when the postgres is being restarted via API diff --git a/patroni/async_executor.py b/patroni/async_executor.py new file mode 100644 index 00000000..fc222202 --- /dev/null +++ b/patroni/async_executor.py @@ -0,0 +1,55 @@ +import logging +from threading import Lock, Thread + +logger = logging.getLogger(__name__) + + +class AsyncExecutor: + + def __init__(self): + Lock.__init__(self) + self._busy = False + self._thread_lock = Lock() + self._scheduled_action = None + self._scheduled_action_lock = Lock() + + @property + def busy(self): + return self._busy + + def schedule(self, action, immediately=False): + with self._scheduled_action_lock: + if self._scheduled_action is not None: + return self._scheduled_action + self._scheduled_action = action + self._busy = immediately + return None + + @property + def scheduled_action(self): + with self._scheduled_action_lock: + return self._scheduled_action + + def reset_scheduled_action(self): + with self._scheduled_action_lock: + self._scheduled_action = None + + def run(self, func, args=()): + try: + return func(*args) if args else func() + except: + logger.exception('Exception during execution of long running task %s', self.scheduled_action) + finally: + with self: + self._busy = False + self.reset_scheduled_action() + + def run_async(self, func, args=()): + self._busy = True + Thread(target=self.run, args=(func, args)).start() + + def __enter__(self): + self._thread_lock.acquire() + + def __exit__(self, type, value, traceback): + self._thread_lock.release() diff --git a/patroni/dcs.py b/patroni/dcs.py index fb034322..1bca092e 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -1,8 +1,9 @@ import abc +import json from collections import namedtuple from patroni.exceptions import DCSError -from patroni.utils import calculate_ttl, sleep +from patroni.utils import sleep from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl @@ -23,28 +24,47 @@ def parse_connection_string(value): return conn_url, api_url -class Member(namedtuple('Member', 'index,name,conn_url,api_url,expiration,ttl')): +class Member(namedtuple('Member', 'index,name,session,data')): """Immutable object (namedtuple) which represents single member of PostgreSQL cluster. Consists of the following fields: :param index: modification index of a given member key in a Configuration Store :param name: name of PostgreSQL cluster member - :param conn_url: connection string containing host, user and password which could be used to access this member. - :param api_url: REST API url of patroni instance - :param expiration: expiration time of given member key - :param ttl: ttl of given member key in seconds""" + :param session: either session id or just ttl in seconds + :param data: arbitrary data i.e. conn_url, api_url, xlog location, state, role, tags, etc... - def real_ttl(self): - return calculate_ttl(self.expiration) or -1 + There are two mandatory keys in a data: + conn_url: connection string containing host, user and password which could be used to access this member. + api_url: REST API url of patroni instance""" + + @staticmethod + def from_node(index, name, session, data): + """ + >>> Member.from_node(-1, '', '', '{"conn_url": "postgres://foo@bar/postgres"}') is not None + True + """ + if data.startswith('postgres'): + conn_url, api_url = parse_connection_string(data) + data = {'conn_url': conn_url, 'api_url': api_url} + else: + data = json.loads(data) + return Member(index, name, session, data) + + @property + def conn_url(self): + return self.data.get('conn_url', None) + + @property + def api_url(self): + return self.data.get('api_url', None) -class Leader(namedtuple('Leader', 'index,expiration,ttl,member')): +class Leader(namedtuple('Leader', 'index,session,member')): """Immutable object (namedtuple) which represents leader key. Consists of the following fields: :param index: modification index of a leader key in a Configuration Store - :param expiration: expiration time of the leader key - :param ttl: ttl of the leader key + :param session: either session id or just ttl in seconds :param member: reference to a `Member` object which represents current leader (see `Cluster.members`)""" @property @@ -100,6 +120,8 @@ class AbstractDCS: self._scope = config['scope'] self._base_path = '/service/' + self._scope + self.cluster = None + def client_path(self, path): return '/'.join([self._base_path, path.lstrip('/')]) diff --git a/patroni/etcd.py b/patroni/etcd.py index c2995da9..f1ca5044 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -10,7 +10,7 @@ import urllib3 from dns.exception import DNSException from dns import resolver -from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member, parse_connection_string +from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member from patroni.exceptions import DCSError from patroni.utils import Retry, RetryFailedError, sleep from requests.exceptions import RequestException @@ -154,7 +154,6 @@ class Etcd(AbstractDCS): etcd.EtcdWatcherCleared, etcd.EtcdEventIndexCleared)) self.client = self.get_etcd_client(config) - self.cluster = None def retry(self, *args, **kwargs): return self._retry.copy()(*args, **kwargs) @@ -171,8 +170,7 @@ class Etcd(AbstractDCS): @staticmethod def member(node): - conn_url, api_url = parse_connection_string(node.value) - return Member(node.modifiedIndex, os.path.basename(node.key), conn_url, api_url, node.expiration, node.ttl) + return Member.from_node(node.modifiedIndex, os.path.basename(node.key), node.ttl, node.value) def get_cluster(self): try: @@ -192,9 +190,9 @@ class Etcd(AbstractDCS): # get leader leader = nodes.get(self._LEADER, None) if leader: - member = Member(-1, leader.value, None, None, None, None) + member = Member(-1, leader.value, None, {}) member = ([m for m in members if m.name == leader.value] or [member])[0] - leader = Leader(leader.modifiedIndex, leader.expiration, leader.ttl, member) + leader = Leader(leader.modifiedIndex, leader.ttl, member) # failover key failover = nodes.get(self._FAILOVER, None) diff --git a/patroni/ha.py b/patroni/ha.py index d791ea87..25a05c23 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -1,35 +1,28 @@ +import json import logging import psycopg2 import requests +from patroni.async_executor import AsyncExecutor from patroni.exceptions import DCSError, PostgresConnectionException from multiprocessing.pool import ThreadPool -from threading import Lock, Thread logger = logging.getLogger(__name__) class Ha: - def __init__(self, state_handler, dcs): - self.state_handler = state_handler - self.dcs = dcs - self.cluster = None + def __init__(self, patroni): + self.patroni = patroni + self.state_handler = patroni.postgresql + self.dcs = patroni.dcs self.old_cluster = None - self._scheduled_action = None - self._scheduled_action_lock = Lock() - self._long_action_in_progress = False - self._long_action_thread_lock = Lock() + self._async_executor = AsyncExecutor() def load_cluster_from_dcs(self): - cluster = self.dcs.get_cluster() - # We want to keep the state of cluster when it was healhy - if cluster.is_unlocked() and self.cluster and not self.cluster.is_unlocked(): - self.old_cluster = self.cluster - if not self.old_cluster: - self.old_cluster = cluster - self.cluster = cluster + if not self.dcs.get_cluster().is_unlocked() or not self.old_cluster: + self.old_cluster = self.dcs.cluster def acquire_lock(self): return self.dcs.attempt_to_acquire_leader() @@ -44,26 +37,26 @@ class Ha: return ret def has_lock(self): - lock_owner = self.cluster.leader and self.cluster.leader.name + lock_owner = self.dcs.cluster.leader and self.dcs.cluster.leader.name logger.info('Lock owner: %s; I am %s', lock_owner, self.state_handler.name) return lock_owner == self.state_handler.name - def _run_async(self, func, args=()): - try: - return func(*args) if args else func() - except: - logger.exception('Exception during execution of long running task %s', self.get_scheduled_action()) - finally: - with self._long_action_thread_lock: - self._long_action_in_progress = False - self._reset_scheduled_action() - - def run_async(self, func, args=()): - self._long_action_in_progress = True - Thread(target=self._run_async, args=(func, args)).start() + def touch_member(self): + data = { + 'conn_url': self.state_handler.connection_string, + 'api_url': self.patroni.api.connection_string, + 'state': self.state_handler.state, + 'role': self.state_handler.role + } + if data['state'] in ['running', 'restarting', 'starting']: + try: + data['xlog_location'] = self.state_handler.xlog_position() + except: + pass + self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) def copy_backup_from_leader(self): - if self.state_handler.bootstrap(self.cluster.leader): + if self.state_handler.bootstrap(self.dcs.cluster.leader): logger.info('bootstrapped from leader') else: self.state_handler.stop('immediate') @@ -71,15 +64,14 @@ class Ha: logger.error('failed to bootstrap from leader') def bootstrap(self): - if not self.cluster.is_unlocked(): # cluster already has leader - if self._long_action_in_progress: + if not self.dcs.cluster.is_unlocked(): # cluster already has leader + if self._async_executor.busy: self.copy_backup_from_leader() else: - with self._scheduled_action_lock: - self._scheduled_action = 'bootstrap from leader' - self.run_async(self.copy_backup_from_leader) + self._async_executor.schedule('bootstrap from leader') + self._async_executor.run_async(self.copy_backup_from_leader) return 'trying to bootstrap from leader' - elif not self.cluster.initialize: # no initialize key + elif not self.dcs.cluster.initialize: # no initialize key if self.dcs.initialize(): # race for initialization try: self.state_handler.bootstrap() @@ -99,7 +91,7 @@ class Ha: def recover(self): has_lock = self.has_lock() - self.state_handler.write_recovery_conf(None if has_lock else self.cluster.leader) + self.state_handler.write_recovery_conf(None if has_lock else self.dcs.cluster.leader) if not self.state_handler.start(): if not has_lock: return 'failed to start postgres' @@ -113,10 +105,9 @@ class Ha: def follow_the_leader(self, demote_reason, follow_reason, refresh=True): refresh and self.load_cluster_from_dcs() ret = demote_reason if self.state_handler.is_leader() else follow_reason - if not self.state_handler.check_recovery_conf(self.cluster.leader): - with self._scheduled_action_lock: - self._scheduled_action = 'changing primary_conninfo and restarting' - self.run_async(self.state_handler.follow_the_leader, (self.cluster.leader, )) + if not self.state_handler.check_recovery_conf(self.dcs.cluster.leader): + self._async_executor.schedule('changing primary_conninfo and restarting') + self._async_executor.run_async(self.state_handler.follow_the_leader, (self.dcs.cluster.leader, )) return ret def enforce_master_role(self, message, promote_message): @@ -159,7 +150,7 @@ class Ha: if self.state_handler.is_leader(): return True - if check_replication_lag and not self.state_handler.check_replication_lag(self.cluster.last_leader_operation): + if check_replication_lag and not self.state_handler.check_replication_lag(self.dcs.cluster.last_leader_operation): return False # Too far behind last reported xlog location on master # Prepare list of nodes to run check against @@ -190,13 +181,13 @@ class Ha: return ret def manual_failover_process_no_leader(self): - failover = self.cluster.failover + failover = self.dcs.cluster.failover if failover.member: # manual failover to specific member if failover.member == self.state_handler.name: # manual failover to me return True # find specific node and check that it is healthy - members = [m for m in self.cluster.members if m.name == failover.member] + members = [m for m in self.dcs.cluster.members if m.name == failover.member] if members: member, reachable, in_recovery, xlog_location = self.fetch_node_status(members[0]) if reachable: # node is healthy @@ -212,7 +203,7 @@ class Ha: if failover.leader: if self.state_handler.name == failover.leader: # I was the leader # exclude me and desired member which is unhealthy (failover.member can be None) - members = [m for m in self.cluster.members if m.name != failover.member] + members = [m for m in self.dcs.cluster.members if m.name != failover.member] if self.is_failover_possible(members): # check that there are healthy members return False else: # I was the leader and it looks like currently I am the only healthy member @@ -221,15 +212,15 @@ class Ha: # at this point we assume that our node is a candidate for a failover among all nodes except former leader # exclude former leader from the list (failover.leader can be None) - members = [m for m in self.cluster.members if m.name != failover.leader] + members = [m for m in self.dcs.cluster.members if m.name != failover.leader] return self._is_healthiest_node(members, check_replication_lag=False) def is_healthiest_node(self): - if self.cluster.failover: + if self.dcs.cluster.failover: return self.manual_failover_process_no_leader() # run usual health check - members = {m.name: m for m in self.old_cluster.members + self.cluster.members} + members = {m.name: m for m in self.dcs.cluster.members + self.old_cluster.members} return self._is_healthiest_node(members.values()) def demote(self, delete_leader=True): @@ -239,14 +230,13 @@ class Ha: self.state_handler.follow_the_leader(None) def process_manual_failover_from_leader(self): - failover = self.cluster.failover + failover = self.dcs.cluster.failover if not failover.leader or failover.leader == self.state_handler.name: if not failover.member or failover.member != self.state_handler.name: - members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] + members = [m for m in self.dcs.cluster.members if not failover.member or m.name == failover.member] if self.is_failover_possible(members): # check that there are healthy members - with self._scheduled_action_lock: - self._scheduled_action = 'manual failover: demote' - self.run_async(self.demote) + self._async_executor.schedule('manual failover: demote') + self._async_executor.run_async(self.demote) return 'manual failover: demoting myself' else: logger.warning('manual failover: no healthy members found, failover is not possible') @@ -254,15 +244,15 @@ class Ha: logger.warning('manual failover: I am already the leader, no need to failover') else: logger.warning('manual failover: leader name does not match: %s != %s', - self.cluster.failover.leader, self.state_handler.name) + self.dcs.cluster.failover.leader, self.state_handler.name) logger.info('Trying to clean up failover key') - self.dcs.manual_failover('', '', self.cluster.failover.index) + self.dcs.manual_failover('', '', self.dcs.cluster.failover.index) def process_unhealthy_cluster(self): if self.is_healthiest_node(): if self.acquire_lock(): - if self.cluster.failover: + if self.dcs.cluster.failover: logger.info('Cleanning up failover key after acquiring leader lock...') self.dcs.manual_failover('', '') return self.enforce_master_role('acquired session lock as a leader', @@ -276,7 +266,7 @@ class Ha: def process_healthy_cluster(self): if self.has_lock(): - if self.cluster.failover: + if self.dcs.cluster.failover: msg = self.process_manual_failover_from_leader() if msg is not None: return msg @@ -293,42 +283,28 @@ class Ha: return self.follow_the_leader('demoting self because i do not have the lock and i was a leader', 'no action. i am a secondary and i am following a leader', False) - def schedule_action(self, action): - with self._long_action_thread_lock: - with self._scheduled_action_lock: - if self._scheduled_action is not None: - return self._scheduled_action - self._scheduled_action = action - return None - - def get_scheduled_action(self): - with self._scheduled_action_lock: - return self._scheduled_action - - def _reset_scheduled_action(self): - with self._scheduled_action_lock: - self._scheduled_action = None + def schedule(self, action): + with self._async_executor: + return self._async_executor.schedule(action) def restart_scheduled(self): - return self.get_scheduled_action() == 'restart' + return self._async_executor.scheduled_action == 'restart' def schedule_reinitialize(self): - return self.schedule_action('reinitialize') + return self.schedule('reinitialize') def reinitialize_scheduled(self): - return self.get_scheduled_action() == 'reinitialize' + return self._async_executor.scheduled_action == 'reinitialize' def restart(self): - with self._long_action_thread_lock: - with self._scheduled_action_lock: - if self._scheduled_action is not None: - return False, self._scheduled_action + ' already in progress' - self._scheduled_action = 'restart' - self._long_action_in_progress = True - if self._run_async(self.state_handler.restart): - return True, 'restarted successfully' + with self._async_executor: + prev = self._async_executor.schedule('restart', True) + if prev is not None: + return (False, prev + ' already in progress') + if self._async_executor.run(self.state_handler.restart): + return (True, 'restarted successfully') else: - return False, 'restart failed' + return (False, 'restart failed') def reinitialize(self): self.state_handler.stop('immediate') @@ -338,47 +314,50 @@ class Ha: def process_scheduled_action(self): if self.reinitialize_scheduled(): - if self.cluster.is_unlocked(): + if self.dcs.cluster.is_unlocked(): logger.error('Cluster has no leader, can not reinitialize') - self._reset_scheduled_action() + self._async_executor.reset_scheduled_action() elif self.has_lock(): logger.error('I am the leader, can not reinitialize') - self._reset_scheduled_action() + self._async_executor.reset_scheduled_action() else: - self.run_async(self.reinitialize) - return True + self._async_executor.run_async(self.reinitialize) + return 'reinitialize started' def handle_long_action_in_progress(self): if self.has_lock(): if self.update_lock(): - return 'updated leader lock during ' + self.get_scheduled_action() + return 'updated leader lock during ' + self._async_executor.scheduled_action else: - return 'failed to update leader lock during ' + self.get_scheduled_action() - elif self.cluster.is_unlocked(): + return 'failed to update leader lock during ' + self._async_executor.scheduled_action + elif self.dcs.cluster.is_unlocked(): return 'not healthy enough for leader race' else: - return self.get_scheduled_action() + ' in progress' + return self._async_executor.scheduled_action + ' in progress' def _run_cycle(self): try: self.load_cluster_from_dcs() + self.touch_member() + # cluster has leader key but not initialize key - if not self.cluster.is_unlocked() and not self.cluster.initialize: + if not self.dcs.cluster.is_unlocked() and not self.dcs.cluster.initialize: self.dcs.initialize() # fix it - if self._long_action_in_progress: + if self._async_executor.busy: return self.handle_long_action_in_progress() # currently it can trigger only reinitialize - if self.process_scheduled_action(): - return 'reinitialize started' + msg = self.process_scheduled_action() + if msg is not None: + return msg # is data directory empty? if self.state_handler.data_directory_empty(): return self.bootstrap() # new node # "bootstrap", but data directory is not empty - elif not self.cluster.initialize and self.cluster.is_unlocked(): + elif not self.dcs.cluster.initialize and self.dcs.cluster.is_unlocked(): self.dcs.initialize() # try to start dead postgres @@ -388,12 +367,12 @@ class Ha: return msg try: - if self.cluster.is_unlocked(): + if self.dcs.cluster.is_unlocked(): return self.process_unhealthy_cluster() else: return self.process_healthy_cluster() finally: - self.state_handler.sync_replication_slots(self.cluster) + self.state_handler.sync_replication_slots(self.dcs.cluster) except DCSError: logger.error('Error communicating with DCS') if self.state_handler.is_running() and self.state_handler.is_leader(): @@ -403,5 +382,5 @@ class Ha: logger.exception('Error communicating with Postgresql. Will try again later') def run_cycle(self): - with self._long_action_thread_lock: + with self._async_executor: return self._run_cycle() diff --git a/patroni/postgresql.py b/patroni/postgresql.py index e15a4996..673f16c3 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -68,7 +68,7 @@ class Postgresql: self._connection = None self._cursor_holder = None self.replication_slots = [] # list of already existing replication slots - self.retry = Retry(max_tries=-1, deadline=10, max_delay=1, retry_exceptions=PostgresConnectionException) + self.retry = Retry(max_tries=-1, deadline=5, max_delay=1, retry_exceptions=PostgresConnectionException) self._state = 'stopped' self._state_lock = Lock() @@ -381,7 +381,7 @@ recovery_target_timeline = 'latest' return self.query("""SELECT pg_xlog_location_diff(CASE WHEN pg_is_in_recovery() THEN pg_last_xlog_replay_location() ELSE pg_current_xlog_location() - END, '0/0')""").fetchone()[0] + END, '0/0')::bigint""").fetchone()[0] def load_replication_slots(self): if self.use_slots and self.schedule_load_slots: diff --git a/patroni/utils.py b/patroni/utils.py index 681ca39c..9b040294 100644 --- a/patroni/utils.py +++ b/patroni/utils.py @@ -36,6 +36,8 @@ def calculate_ttl(expiration): """ >>> calculate_ttl(None) >>> calculate_ttl('2015-06-10 12:56:30.552539016Z') + >>> calculate_ttl('2015-06-10T12:56:30.552539016Z') < 0 + True """ if not expiration: return None diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 4b3389e1..a5258eac 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -5,7 +5,7 @@ import time from kazoo.client import KazooClient, KazooState from kazoo.exceptions import NoNodeError, NodeExistsError -from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member, parse_connection_string +from patroni.dcs import AbstractDCS, Cluster, Failover, Leader, Member from patroni.exceptions import DCSError from patroni.utils import sleep from requests.exceptions import RequestException @@ -93,7 +93,7 @@ class ZooKeeper(AbstractDCS): self.client.add_listener(self.session_listener) self.cluster_event = self.client.handler.event_object() - self.cluster = None + self._my_member_data = None self.fetch_cluster = True self.last_leader_operation = 0 @@ -116,8 +116,7 @@ class ZooKeeper(AbstractDCS): @staticmethod def member(name, value, znode): - conn_url, api_url = parse_connection_string(value) - return Member(znode.version, name, conn_url, api_url, None, None) + return Member.from_node(znode.version, name, znode.ephemeralOwner, value) def get_children(self, key, watch=None): try: @@ -153,9 +152,9 @@ class ZooKeeper(AbstractDCS): leader = None if leader: - member = Member(-1, leader[0], None, None, None, None) + member = Member(-1, leader[0], None, {}) member = ([m for m in members if m.name == leader[0]] or [member])[0] - leader = Leader(leader[1].version, None, None, member) + leader = Leader(leader[1].version, leader[1].ephemeralOwner, member) self.fetch_cluster = member.index == -1 # failover key @@ -207,21 +206,34 @@ class ZooKeeper(AbstractDCS): def initialize(self): return self._create(self.initialize_path, self._name, makepath=True) - def touch_member(self, connection_string, ttl=None): - if not self.fetch_cluster and self.cluster and any(m.name == self._name for m in self.cluster.members): - return True + def touch_member(self, data, ttl=None): + me = self.cluster and ([m for m in self.cluster.members if m.name == self._name] or [None])[0] path = self.member_path - connection_string = connection_string.encode('utf-8') + data = data.encode('utf-8') + create = not me + if me and self.client.client_id is not None and me.session != self.client.client_id[0]: + try: + self.client.retry(self.client.delete, path) + except NoNodeError: + pass + except: + return False + create = True + + if not create and data == self._my_member_data: + return True + try: - self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) + if create: + self.client.retry(self.client.create, path, data, makepath=True, ephemeral=True) + else: + self.client.retry(self.client.set, path, data) + self._my_member_data = data return True except NodeExistsError: try: - node = self.get_node(path) - if node and self.client.client_id is not None and node[1].ephemeralOwner == self.client.client_id[0]: - return True - self.client.retry(self.client.delete, path) - self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) + self.client.retry(self.client.set, path, data) + self._my_member_data = data return True except: logger.exception('touch_member') @@ -252,6 +264,7 @@ class ZooKeeper(AbstractDCS): def delete_leader(self): self.client.restart() + self._my_member_data = None return True def _cancel_initialization(self): diff --git a/tests/test_api.py b/tests/test_api.py index 5003e43e..1a8b59e2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -43,6 +43,7 @@ class MockPatroni: postgresql = MockPostgresql() ha = MockHa() + dcs = Mock() class MockRequest: diff --git a/tests/test_etcd.py b/tests/test_etcd.py index f55aa15e..3d2535a7 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -149,15 +149,6 @@ def http_request(method, url, **kwargs): raise socket.error -class TestMember(unittest.TestCase): - - def test_real_ttl(self): - now = datetime.datetime.utcnow() - member = Member(0, 'a', 'b', 'c', (now + datetime.timedelta(seconds=2)).strftime('%Y-%m-%dT%H:%M:%S.%fZ'), None) - self.assertLess(member.real_ttl(), 2) - self.assertEquals(Member(0, 'a', 'b', 'c', '', None).real_ttl(), -1) - - @patch('dns.resolver.query', dns_query) @patch('socket.getaddrinfo', socket_getaddrinfo) @patch('requests.get', requests_get) diff --git a/tests/test_ha.py b/tests/test_ha.py index 52faba35..a0a43d3f 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -26,11 +26,11 @@ def get_cluster_not_initialized_without_leader(): def get_cluster_initialized_without_leader(leader=False, failover=None): - m = Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', - 'http://127.0.0.1:8008/patroni', None, 28) - l = Leader(0, 0, 0, m) if leader else None - o = Member(0, 'other', 'postgres://replicator:rep-pass@127.0.0.1:5436/postgres', - 'http://127.0.0.1:8011/patroni', None, 28) + m = Member(0, 'leader', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', + 'api_url': 'http://127.0.0.1:8008/patroni'}) + l = Leader(0, 0, m) if leader else None + o = Member(0, 'other', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5436/postgres', + 'api_url': 'http://127.0.0.1:8011/patroni'}) return get_cluster(True, l, [m, o], failover) @@ -42,6 +42,8 @@ class MockPostgresql(Mock): name = 'postgresql0' role = 'replica' + state = 'running' + connection_string = 'postgres://foo@bar/postgres' def is_healthy(self): return True @@ -74,6 +76,14 @@ class MockPostgresql(Mock): return False +class MockPatroni: + + def __init__(self, p, d): + self.postgresql = p + self.dcs = d + self.api = Mock() + self.api.connection_string = 'http://127.0.0.1:8008' + def run_async(func, args=()): func(args) if args else func() @@ -88,22 +98,20 @@ class TestHa(unittest.TestCase): self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}) self.e.client.read = etcd_read self.e.client.write = etcd_write - self.ha = Ha(self.p, self.e) - self.ha.run_async = run_async - self.ha.load_cluster_from_dcs() - self.ha.cluster = get_cluster_not_initialized_without_leader() + self.ha = Ha(MockPatroni(self.p, self.e)) + self.ha._async_executor.run_async = run_async + self.ha.old_cluster = self.e.get_cluster() + self.e.cluster = get_cluster_not_initialized_without_leader() self.ha.load_cluster_from_dcs = Mock() - def test_load_cluster_from_dcs(self): - ha = Ha(self.p, self.e) - ha.load_cluster_from_dcs() - self.e.get_cluster = get_cluster_not_initialized_without_leader - ha.load_cluster_from_dcs() - def test_update_lock(self): self.p.last_operation = Mock(side_effect=PostgresException('')) self.assertTrue(self.ha.update_lock()) + def test_touch_member(self): + self.p.xlog_position = Mock(side_effect=Exception) + self.ha.touch_member() + def test_start_as_replica(self): self.p.is_healthy = false self.assertEquals(self.ha.run_cycle(), 'started as a secondary') @@ -119,8 +127,8 @@ class TestHa(unittest.TestCase): self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres') + @patch.object(Cluster, 'is_unlocked', Mock(return_value=False)) def test_start_as_readonly(self): - self.ha.cluster.is_unlocked = false self.p.is_leader = self.p.is_healthy = false self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock') @@ -153,28 +161,28 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') def test_promote_because_have_lock(self): - self.ha.cluster.is_unlocked = false + self.e.cluster.is_unlocked = false self.ha.has_lock = true self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock') def test_leader_with_lock(self): - self.ha.cluster.is_unlocked = false + self.e.cluster.is_unlocked = false self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') def test_demote_because_not_having_lock(self): - self.ha.cluster.is_unlocked = false + self.e.cluster.is_unlocked = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i do not have the lock and i was a leader') def test_demote_because_update_lock_failed(self): - self.ha.cluster.is_unlocked = false + self.e.cluster.is_unlocked = false self.ha.has_lock = true self.ha.update_lock = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i do not have the lock and i was a leader') def test_follow_the_leader(self): - self.ha.cluster.is_unlocked = false + self.e.cluster.is_unlocked = false self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'no action. i am a secondary and i am following a leader') @@ -182,36 +190,28 @@ class TestHa(unittest.TestCase): self.ha.load_cluster_from_dcs = Mock(side_effect=DCSError('Etcd is not responding properly')) self.assertEquals(self.ha.run_cycle(), 'demoted self because DCS is not accessible and i was a leader') - def test__run_async(self): - self.ha._run_async(Mock(side_effect=Exception())) - - @patch.object(Thread, 'start', Mock()) - def test_run_async(self): - ha = Ha(self.p, self.e) - ha.run_async(true) - def test_bootstrap_from_leader(self): - self.ha.cluster = get_cluster_initialized_with_leader() + self.e.cluster = get_cluster_initialized_with_leader() self.p.bootstrap = false self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') - self.ha._long_action_in_progress = True + self.ha._async_executor._busy = True self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') def test_bootstrap_waiting_for_leader(self): - self.ha.cluster = get_cluster_initialized_without_leader() + self.e.cluster = get_cluster_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'waiting for leader to bootstrap') def test_bootstrap_initialize_lock_failed(self): - self.ha.cluster = get_cluster_not_initialized_without_leader() + self.e.cluster = get_cluster_not_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'failed to acquire initialize lock') def test_bootstrap_initialized_new_cluster(self): - self.ha.cluster = get_cluster_not_initialized_without_leader() + self.e.cluster = get_cluster_not_initialized_without_leader() self.e.initialize = true self.assertEquals(self.ha.bootstrap(), 'initialized a new cluster') def test_bootstrap_release_initialize_key_on_failure(self): - self.ha.cluster = get_cluster_not_initialized_without_leader() + self.e.cluster = get_cluster_not_initialized_without_leader() self.e.initialize = true self.p.bootstrap = Mock(side_effect=PostgresException("Could not bootstrap master PostgreSQL")) self.assertRaises(PostgresException, self.ha.bootstrap) @@ -220,13 +220,13 @@ class TestHa(unittest.TestCase): self.ha.schedule_reinitialize() self.ha.schedule_reinitialize() self.ha.run_cycle() - self.assertIsNone(self.ha.get_scheduled_action()) + self.assertIsNone(self.ha._async_executor.scheduled_action) - self.ha.cluster = get_cluster_initialized_with_leader() + self.e.cluster = get_cluster_initialized_with_leader() self.ha.has_lock = true self.ha.schedule_reinitialize() self.ha.run_cycle() - self.assertIsNone(self.ha.get_scheduled_action()) + self.assertIsNone(self.ha._async_executor.scheduled_action) self.ha.has_lock = false self.ha.schedule_reinitialize() @@ -240,12 +240,11 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.restart(), (False, 'reinitialize already in progress')) def test_restart_in_progress(self): - self.ha._long_action_in_progress = True - self.ha._scheduled_action = 'restart' + self.ha._async_executor.schedule('restart', True) self.assertTrue(self.ha.restart_scheduled()) self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race') - self.ha.cluster = get_cluster_initialized_with_leader() + self.e.cluster = get_cluster_initialized_with_leader() self.assertEquals(self.ha.run_cycle(), 'restart in progress') self.ha.has_lock = true @@ -257,26 +256,26 @@ class TestHa(unittest.TestCase): @patch('requests.get', requests_get) def test_manual_failover_from_leader(self): self.ha.has_lock = true - self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '')) + self.e.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '')) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') - self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name)) + self.e.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name)) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') - self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla')) + self.e.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla')) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') f = Failover(0, MockPostgresql.name, '') - self.ha.cluster = get_cluster_initialized_with_leader(f) + self.e.cluster = get_cluster_initialized_with_leader(f) self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself') @patch('requests.get', requests_get) def test_manual_failover_process_no_leader(self): self.p.is_leader = false - self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name)) + self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name)) self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') - self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader')) + self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader')) self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') self.ha.fetch_node_status = lambda e: (e, True, True, 0) # accessible, in_recovery self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') - self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '')) + self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '')) self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') self.ha.fetch_node_status = lambda e: (e, False, True, 0) # accessible, in_recovery self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') @@ -295,7 +294,7 @@ class TestHa(unittest.TestCase): @patch('requests.get', requests_get) def test_fetch_node_status(self): - member = Member(0, 'test', '', 'http://127.0.0.1:8011/patroni', None, None) + member = Member(0, 'test', 1, {'api_url': 'http://127.0.0.1:8011/patroni'}) self.ha.fetch_node_status(member) - member = Member(0, 'test', '', 'http://localhost:8011/patroni', None, None) + member = Member(0, 'test', 1, {'api_url': 'http://localhost:8011/patroni'}) self.ha.fetch_node_status(member) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 1c82799b..b38f36ce 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -6,6 +6,7 @@ import yaml from mock import Mock, patch from patroni.api import RestApiServer +from patroni.async_executor import AsyncExecutor from patroni.dcs import Cluster, Member from patroni.etcd import Etcd from patroni.ha import Ha @@ -27,7 +28,7 @@ def time_sleep(*args): @patch.object(Postgresql, 'write_pg_hba', Mock()) @patch.object(Postgresql, 'write_recovery_conf', Mock()) @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock()) -@patch.object(Ha, 'run_async', Mock()) +@patch.object(AsyncExecutor, 'run', Mock()) class TestPatroni(unittest.TestCase): @patch.object(Client, 'machines') @@ -57,15 +58,13 @@ class TestPatroni(unittest.TestCase): sys.argv = ['patroni.py', 'postgres0.yml'] mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) - with patch.object(Patroni, 'touch_member', self.touch_member): - with patch.object(Patroni, 'run', Mock(side_effect=SleepException())): - self.assertRaises(SleepException, main) - with patch.object(Patroni, 'run', Mock(side_effect=KeyboardInterrupt())): - main() + with patch.object(Patroni, 'run', Mock(side_effect=SleepException())): + self.assertRaises(SleepException, main) + with patch.object(Patroni, 'run', Mock(side_effect=KeyboardInterrupt())): + main() @patch('time.sleep', Mock(side_effect=SleepException())) def test_run(self): - self.p.touch_member = self.touch_member self.p.ha.dcs.watch = time_sleep self.assertRaises(SleepException, self.p.run) @@ -73,20 +72,6 @@ class TestPatroni(unittest.TestCase): self.p.api.start = Mock() self.assertRaises(SleepException, self.p.run) - def touch_member(self, ttl=None): - if not self.touched: - self.touched = True - return False - return True - - def test_touch_member(self): - self.p.touch_member() - now = datetime.datetime.utcnow() - member = Member(0, self.p.postgresql.name, 'b', 'c', (now + datetime.timedelta( - seconds=self.p.shutdown_member_ttl + 10)).strftime('%Y-%m-%dT%H:%M:%S.%fZ'), None) - self.p.ha.cluster = Cluster(True, member, 0, [member], None) - self.p.touch_member() - def test_schedule_next_run(self): self.p.ha.dcs.watch = Mock(return_value=True) self.p.schedule_next_run() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 90874288..007310a5 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -105,10 +105,10 @@ class TestPostgresql(unittest.TestCase): 'restore': 'true'}) if not os.path.exists(self.p.data_dir): os.makedirs(self.p.data_dir) - self.leadermem = Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', None, None, 28) - self.leader = Leader(-1, None, 28, self.leadermem) - self.other = Member(0, 'test1', 'postgres://replicator:rep-pass@127.0.0.1:5433/postgres', None, None, 28) - self.me = Member(0, 'test0', 'postgres://replicator:rep-pass@127.0.0.1:5434/postgres', None, None, 28) + self.leadermem = Member(0, 'leader', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres'}) + self.leader = Leader(-1, 28, self.leadermem) + self.other = Member(0, 'test1', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5433/postgres'}) + self.me = Member(0, 'test0', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5434/postgres'}) def tearDown(self): shutil.rmtree('data') @@ -146,7 +146,7 @@ class TestPostgresql(unittest.TestCase): self.p.follow_the_leader(self.leader) self.p.demote() self.p.follow_the_leader(self.leader) - self.p.follow_the_leader(Leader(-1, None, 28, self.other)) + self.p.follow_the_leader(Leader(-1, 28, self.other)) def test_create_replica(self): self.p.delete_trigger_file = Mock(side_effect=OSError()) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 039faa58..9e14bec2 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -69,6 +69,9 @@ class MockKazooClient(Mock): raise TypeError("Invalid type for 'value' (must be a byte string)") if path == '/service/bla/optime/leader': raise Exception + if path == '/service/test/members/bar': + if value == b'retry': + return if path == '/service/test/failover': if value == b'Exception': raise Exception @@ -85,7 +88,9 @@ class MockKazooClient(Mock): return self.leader = True raise Exception - elif path.endswith('/initialize'): + elif path == '/service/test/members/buzz': + raise Exception + elif path.endswith('/initialize') or path == '/service/test/members/bar': raise NoNodeError @@ -137,10 +142,18 @@ class TestZooKeeper(unittest.TestCase): self.zk.cancel_initialization() def test_touch_member(self): + self.zk._name = 'buzz' + self.zk.get_cluster() self.zk.touch_member('new') + self.zk._name = 'bar' + self.zk.touch_member('new') + self.zk._name = 'na' + self.zk.client.exists = 1 self.zk.touch_member('exists') + self.zk._name = 'bar' self.zk.touch_member('retry') - self.zk.client.exists = True + self.zk.fetch_cluster = True + self.zk.get_cluster() self.zk.touch_member('retry') def test_take_leader(self): From d8f4b09478aaac13cbf3a983f686d05518131cd5 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 2 Oct 2015 10:26:48 +0200 Subject: [PATCH 09/15] use Event.wait instead of sleep it makes possible to break "sleep" for example from API plus small bugfix: catch ValueError exception from json.loads --- patroni/dcs.py | 14 ++++++++++---- patroni/etcd.py | 8 +++++--- patroni/ha.py | 3 ++- patroni/zookeeper.py | 12 ++++-------- tests/test_etcd.py | 6 ++---- tests/test_ha.py | 2 +- tests/test_patroni.py | 3 --- tests/test_zookeeper.py | 2 +- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index 1bca092e..d007b392 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -3,8 +3,8 @@ import json from collections import namedtuple from patroni.exceptions import DCSError -from patroni.utils import sleep from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl +from threading import Event def parse_connection_string(value): @@ -42,12 +42,17 @@ class Member(namedtuple('Member', 'index,name,session,data')): """ >>> Member.from_node(-1, '', '', '{"conn_url": "postgres://foo@bar/postgres"}') is not None True + >>> Member.from_node(-1, '', '', '{') + Member(index=-1, name='', session='', data={}) """ if data.startswith('postgres'): conn_url, api_url = parse_connection_string(data) data = {'conn_url': conn_url, 'api_url': api_url} else: - data = json.loads(data) + try: + data = json.loads(data) + except: + data = {} return Member(index, name, session, data) @property @@ -121,6 +126,7 @@ class AbstractDCS: self._base_path = '/service/' + self._scope self.cluster = None + self.event = Event() def client_path(self, path): return '/'.join([self._base_path, path.lstrip('/')]) @@ -234,5 +240,5 @@ class AbstractDCS: :param timeout: timeout in seconds :returns: `!True` if you would like to reschedule the next run of ha cycle""" - sleep(timeout) - return False + self.event.wait(timeout) + return self.event.isSet() diff --git a/patroni/etcd.py b/patroni/etcd.py index f1ca5044..76e00554 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -147,7 +147,6 @@ class Etcd(AbstractDCS): def __init__(self, name, config): super(Etcd, self).__init__(name, config) self.ttl = config['ttl'] - self.member_ttl = config.get('member_ttl', 3600) self._retry = Retry(deadline=10, max_delay=1, max_tries=-1, retry_exceptions=(etcd.EtcdConnectionFailed, etcd.EtcdLeaderElectionInProgress, @@ -210,7 +209,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def touch_member(self, connection_string, ttl=None): - return self.retry(self.client.set, self.member_path, connection_string, ttl or self.member_ttl) + return self.retry(self.client.set, self.member_path, connection_string, ttl or self.ttl) @catch_etcd_errors def take_leader(self): @@ -269,4 +268,7 @@ class Etcd(AbstractDCS): timeout = end_time - time.time() - return timeout > 0 and super(Etcd, self).watch(timeout) + try: + return super(Etcd, self).watch(timeout) + finally: + self.event.clear() diff --git a/patroni/ha.py b/patroni/ha.py index 25a05c23..ca79d8e3 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -150,7 +150,8 @@ class Ha: if self.state_handler.is_leader(): return True - if check_replication_lag and not self.state_handler.check_replication_lag(self.dcs.cluster.last_leader_operation): + if check_replication_lag and \ + not self.state_handler.check_replication_lag(self.dcs.cluster.last_leader_operation): return False # Too far behind last reported xlog location on master # Prepare list of nodes to run check against diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index a5258eac..68786485 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -91,7 +91,6 @@ class ZooKeeper(AbstractDCS): 'max_tries': -1}, connection_retry={'max_delay': 1, 'max_tries': -1}) self.client.add_listener(self.session_listener) - self.cluster_event = self.client.handler.event_object() self._my_member_data = None self.fetch_cluster = True @@ -105,7 +104,7 @@ class ZooKeeper(AbstractDCS): def cluster_watcher(self, event): self.fetch_cluster = True - self.cluster_event.set() + self.event.set() def get_node(self, key, watch=None): try: @@ -133,7 +132,7 @@ class ZooKeeper(AbstractDCS): return members def _inner_load_cluster(self): - self.cluster_event.clear() + self.event.clear() nodes = set(self.get_children(self.client_path(''), self.cluster_watcher)) # get initialize flag @@ -279,8 +278,5 @@ class ZooKeeper(AbstractDCS): logger.exception("Unable to delete initialize key") def watch(self, timeout): - self.cluster_event.wait(timeout) - if self.cluster_event.isSet(): - self.fetch_cluster = True - return True - return False + self.fetch_cluster = super(ZooKeeper, self).watch(timeout) + return self.fetch_cluster diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 3d2535a7..53c054e5 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -1,4 +1,3 @@ -import datetime import etcd import json import requests @@ -8,7 +7,7 @@ import unittest from dns.exception import DNSException from mock import Mock, patch -from patroni.dcs import Cluster, DCSError, Leader, Member +from patroni.dcs import Cluster, DCSError, Leader from patroni.etcd import Client, Etcd @@ -194,7 +193,6 @@ class TestClient(unittest.TestCase): self.assertRaises(etcd.EtcdException, self.client._load_machines_cache) -@patch('time.sleep', Mock()) @patch('requests.get', requests_get) class TestEtcd(unittest.TestCase): @@ -254,7 +252,7 @@ class TestEtcd(unittest.TestCase): def test_watch(self): self.etcd.client.watch = etcd_watch - self.etcd.watch(100) + self.etcd.watch(0) self.etcd.get_cluster() self.etcd.watch(1.5) self.etcd.watch(4.5) diff --git a/tests/test_ha.py b/tests/test_ha.py index a0a43d3f..392ad663 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -6,7 +6,6 @@ from patroni.etcd import Client, Etcd from patroni.exceptions import DCSError, PostgresException from patroni.ha import Ha from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get -from threading import Thread def true(*args, **kwargs): @@ -84,6 +83,7 @@ class MockPatroni: self.api = Mock() self.api.connection_string = 'http://127.0.0.1:8008' + def run_async(func, args=()): func(args) if args else func() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index b38f36ce..52b10d7a 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -1,4 +1,3 @@ -import datetime import sys import time import unittest @@ -7,9 +6,7 @@ import yaml from mock import Mock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor -from patroni.dcs import Cluster, Member from patroni.etcd import Etcd -from patroni.ha import Ha from patroni import Patroni, main from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 9e14bec2..b4e004be 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -170,5 +170,5 @@ class TestZooKeeper(unittest.TestCase): def test_watch(self): self.zk.watch(0) - self.zk.cluster_event.isSet = lambda: False + self.zk.event.isSet = lambda: False self.zk.watch(0) From 4c444c943e60758f0f84500a5fcb88c752bcf2cc Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 2 Oct 2015 13:17:58 +0200 Subject: [PATCH 10/15] tests for Api.do_GET method --- tests/test_api.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 1a8b59e2..e4b8f87e 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -71,10 +71,20 @@ class MockRestApiServer(RestApiServer): class TestRestApiHandler(unittest.TestCase): def test_do_GET(self): - MockRestApiServer(RestApiHandler, b'GET /master') MockRestApiServer(RestApiHandler, b'GET /replica') + with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={})): + MockRestApiServer(RestApiHandler, b'GET /replica') + with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={'role': 'master'})): + MockRestApiServer(RestApiHandler, b'GET /replica') + MockRestApiServer(RestApiHandler, b'GET /master') + MockPatroni.dcs.cluster.leader.name = MockPostgresql.name + MockRestApiServer(RestApiHandler, b'GET /replica') + MockPatroni.dcs.cluster = None + with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={'role': 'master'})): + MockRestApiServer(RestApiHandler, b'GET /master') with patch.object(MockHa, 'restart_scheduled', Mock(return_value=True)): MockRestApiServer(RestApiHandler, b'GET /master') + MockRestApiServer(RestApiHandler, b'GET /master') def test_do_GET_patroni(self): MockRestApiServer(RestApiHandler, b'GET /patroni') From 601ba7db8da5507f60a4b79df744b49bd64db597 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 5 Oct 2015 14:30:47 +0200 Subject: [PATCH 11/15] Make work with dcs.cluster thread-safe --- patroni/api.py | 5 +-- patroni/dcs.py | 33 +++++++++++++++--- patroni/etcd.py | 13 ++++--- patroni/ha.py | 81 +++++++++++++++++++++++--------------------- patroni/zookeeper.py | 9 +++-- tests/test_ha.py | 44 ++++++++++++------------ 6 files changed, 104 insertions(+), 81 deletions(-) diff --git a/patroni/api.py b/patroni/api.py index 0333e86c..dc83249f 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -48,8 +48,9 @@ class RestApiHandler(BaseHTTPRequestHandler): response = self.get_postgresql_status() patroni = self.server.patroni - if patroni.dcs.cluster: # dcs available - if patroni.dcs.cluster.leader and patroni.dcs.cluster.leader.name == patroni.postgresql.name: # is_leader + cluster = patroni.dcs.cluster + if cluster: # dcs available + if cluster.leader and cluster.leader.name == patroni.postgresql.name: # is_leader status_code = 200 if 'master' in path else 503 elif 'role' not in response: status_code = 503 diff --git a/patroni/dcs.py b/patroni/dcs.py index d007b392..25e44cd1 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -4,7 +4,7 @@ import json from collections import namedtuple from patroni.exceptions import DCSError from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl -from threading import Event +from threading import Event, Lock def parse_connection_string(value): @@ -125,7 +125,8 @@ class AbstractDCS: self._scope = config['scope'] self._base_path = '/service/' + self._scope - self.cluster = None + self._cluster = None + self._cluster_thread_lock = Lock() self.event = Event() def client_path(self, path): @@ -156,10 +157,32 @@ class AbstractDCS: return self.client_path(self._LEADER_OPTIME) @abc.abstractmethod + def _load_cluster(self): + """Internally this method should build `Cluster` object which + represents current state and topology of the cluster in DCS. + this method supposed to be called only by `get_cluster` method. + + raise `~DCSError` in case of communication or other problems with DCS. + If the current node was running as a master and exception raised, + instance would be demoted.""" + def get_cluster(self): - """:returns: `Cluster` object which represent current state and topology of the cluster - raise `~DCSError` in case of communication or other problems with DCS. If current instance was - running as a master and exception raised instance would be demoted.""" + with self._cluster_thread_lock: + try: + self._load_cluster() + except: + self._cluster = None + raise + return self._cluster + + @property + def cluster(self): + with self._cluster_thread_lock: + return self._cluster + + def reset_cluster(self): + with self._cluster_thread_lock: + self._cluster = None @abc.abstractmethod def write_leader_optime(self, last_operation): diff --git a/patroni/etcd.py b/patroni/etcd.py index 76e00554..79379700 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -171,7 +171,7 @@ class Etcd(AbstractDCS): def member(node): return Member.from_node(node.modifiedIndex, os.path.basename(node.key), node.ttl, node.value) - def get_cluster(self): + def _load_cluster(self): try: result = self.retry(self.client.read, self.client_path(''), recursive=True) nodes = {os.path.relpath(node.key, result.key): node for node in result.leaves} @@ -198,14 +198,12 @@ class Etcd(AbstractDCS): if failover: failover = Failover.from_node(failover.modifiedIndex, failover.value) - self.cluster = Cluster(initialize, leader, last_leader_operation, members, failover) + self._cluster = Cluster(initialize, leader, last_leader_operation, members, failover) except etcd.EtcdKeyNotFound: - self.cluster = Cluster(False, None, None, [], None) + self._cluster = Cluster(False, None, None, [], None) except: - self.cluster = None logger.exception('get_cluster') raise EtcdError('Etcd is not responding properly') - return self.cluster @catch_etcd_errors def touch_member(self, connection_string, ttl=None): @@ -249,10 +247,11 @@ class Etcd(AbstractDCS): return self.retry(self.client.delete, self.initialize_path, prevValue=self._name) def watch(self, timeout): + cluster = self.cluster # watch on leader key changes if it is defined and current node is not lock owner - if self.cluster and self.cluster.leader and self.cluster.leader.name != self._name: + if cluster and cluster.leader and cluster.leader.name != self._name: end_time = time.time() + timeout - index = self.cluster.leader.index + index = cluster.leader.index while index and timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect try: diff --git a/patroni/ha.py b/patroni/ha.py index ca79d8e3..9203e153 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -16,13 +16,17 @@ class Ha: self.patroni = patroni self.state_handler = patroni.postgresql self.dcs = patroni.dcs + self.cluster = None self.old_cluster = None self._async_executor = AsyncExecutor() def load_cluster_from_dcs(self): + cluster = self.dcs.get_cluster() + # We want to keep the state of cluster when it was healhy - if not self.dcs.get_cluster().is_unlocked() or not self.old_cluster: - self.old_cluster = self.dcs.cluster + if not cluster.is_unlocked() or not self.old_cluster: + self.old_cluster = cluster + self.cluster = cluster def acquire_lock(self): return self.dcs.attempt_to_acquire_leader() @@ -37,7 +41,7 @@ class Ha: return ret def has_lock(self): - lock_owner = self.dcs.cluster.leader and self.dcs.cluster.leader.name + lock_owner = self.cluster.leader and self.cluster.leader.name logger.info('Lock owner: %s; I am %s', lock_owner, self.state_handler.name) return lock_owner == self.state_handler.name @@ -55,8 +59,8 @@ class Ha: pass self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) - def copy_backup_from_leader(self): - if self.state_handler.bootstrap(self.dcs.cluster.leader): + def copy_backup_from_leader(self, leader): + if self.state_handler.bootstrap(leader): logger.info('bootstrapped from leader') else: self.state_handler.stop('immediate') @@ -64,14 +68,11 @@ class Ha: logger.error('failed to bootstrap from leader') def bootstrap(self): - if not self.dcs.cluster.is_unlocked(): # cluster already has leader - if self._async_executor.busy: - self.copy_backup_from_leader() - else: - self._async_executor.schedule('bootstrap from leader') - self._async_executor.run_async(self.copy_backup_from_leader) + if not self.cluster.is_unlocked(): # cluster already has leader + self._async_executor.schedule('bootstrap from leader') + self._async_executor.run_async(self.copy_backup_from_leader, args=(self.cluster.leader, )) return 'trying to bootstrap from leader' - elif not self.dcs.cluster.initialize: # no initialize key + elif not self.cluster.initialize: # no initialize key if self.dcs.initialize(): # race for initialization try: self.state_handler.bootstrap() @@ -91,11 +92,12 @@ class Ha: def recover(self): has_lock = self.has_lock() - self.state_handler.write_recovery_conf(None if has_lock else self.dcs.cluster.leader) + self.state_handler.write_recovery_conf(None if has_lock else self.cluster.leader) if not self.state_handler.start(): if not has_lock: return 'failed to start postgres' self.dcs.delete_leader() + self.dcs.reset_cluster() return 'removed leader key after trying and failing to start postgres' if not has_lock: return 'started as a secondary' @@ -105,9 +107,11 @@ class Ha: def follow_the_leader(self, demote_reason, follow_reason, refresh=True): refresh and self.load_cluster_from_dcs() ret = demote_reason if self.state_handler.is_leader() else follow_reason - if not self.state_handler.check_recovery_conf(self.dcs.cluster.leader): + if not self.state_handler.check_recovery_conf(self.cluster.leader): self._async_executor.schedule('changing primary_conninfo and restarting') - self._async_executor.run_async(self.state_handler.follow_the_leader, (self.dcs.cluster.leader, )) + leader = self.cluster.leader + leader = None if (leader and leader.name) == self.state_handler.name else leader + self._async_executor.run_async(self.state_handler.follow_the_leader, (leader, )) return ret def enforce_master_role(self, message, promote_message): @@ -150,8 +154,7 @@ class Ha: if self.state_handler.is_leader(): return True - if check_replication_lag and \ - not self.state_handler.check_replication_lag(self.dcs.cluster.last_leader_operation): + if check_replication_lag and not self.state_handler.check_replication_lag(self.cluster.last_leader_operation): return False # Too far behind last reported xlog location on master # Prepare list of nodes to run check against @@ -182,13 +185,13 @@ class Ha: return ret def manual_failover_process_no_leader(self): - failover = self.dcs.cluster.failover + failover = self.cluster.failover if failover.member: # manual failover to specific member if failover.member == self.state_handler.name: # manual failover to me return True # find specific node and check that it is healthy - members = [m for m in self.dcs.cluster.members if m.name == failover.member] + members = [m for m in self.cluster.members if m.name == failover.member] if members: member, reachable, in_recovery, xlog_location = self.fetch_node_status(members[0]) if reachable: # node is healthy @@ -204,7 +207,7 @@ class Ha: if failover.leader: if self.state_handler.name == failover.leader: # I was the leader # exclude me and desired member which is unhealthy (failover.member can be None) - members = [m for m in self.dcs.cluster.members if m.name != failover.member] + members = [m for m in self.cluster.members if m.name != failover.member] if self.is_failover_possible(members): # check that there are healthy members return False else: # I was the leader and it looks like currently I am the only healthy member @@ -213,28 +216,29 @@ class Ha: # at this point we assume that our node is a candidate for a failover among all nodes except former leader # exclude former leader from the list (failover.leader can be None) - members = [m for m in self.dcs.cluster.members if m.name != failover.leader] + members = [m for m in self.cluster.members if m.name != failover.leader] return self._is_healthiest_node(members, check_replication_lag=False) def is_healthiest_node(self): - if self.dcs.cluster.failover: + if self.cluster.failover: return self.manual_failover_process_no_leader() # run usual health check - members = {m.name: m for m in self.dcs.cluster.members + self.old_cluster.members} + members = {m.name: m for m in self.cluster.members + self.old_cluster.members} return self._is_healthiest_node(members.values()) def demote(self, delete_leader=True): if delete_leader: self.state_handler.stop() self.dcs.delete_leader() + self.dcs.reset_cluster() self.state_handler.follow_the_leader(None) def process_manual_failover_from_leader(self): - failover = self.dcs.cluster.failover + failover = self.cluster.failover if not failover.leader or failover.leader == self.state_handler.name: if not failover.member or failover.member != self.state_handler.name: - members = [m for m in self.dcs.cluster.members if not failover.member or m.name == failover.member] + members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] if self.is_failover_possible(members): # check that there are healthy members self._async_executor.schedule('manual failover: demote') self._async_executor.run_async(self.demote) @@ -245,15 +249,15 @@ class Ha: logger.warning('manual failover: I am already the leader, no need to failover') else: logger.warning('manual failover: leader name does not match: %s != %s', - self.dcs.cluster.failover.leader, self.state_handler.name) + self.cluster.failover.leader, self.state_handler.name) logger.info('Trying to clean up failover key') - self.dcs.manual_failover('', '', self.dcs.cluster.failover.index) + self.dcs.manual_failover('', '', self.cluster.failover.index) def process_unhealthy_cluster(self): if self.is_healthiest_node(): if self.acquire_lock(): - if self.dcs.cluster.failover: + if self.cluster.failover: logger.info('Cleanning up failover key after acquiring leader lock...') self.dcs.manual_failover('', '') return self.enforce_master_role('acquired session lock as a leader', @@ -267,7 +271,7 @@ class Ha: def process_healthy_cluster(self): if self.has_lock(): - if self.dcs.cluster.failover: + if self.cluster.failover: msg = self.process_manual_failover_from_leader() if msg is not None: return msg @@ -307,22 +311,21 @@ class Ha: else: return (False, 'restart failed') - def reinitialize(self): + def reinitialize(self, cluster): self.state_handler.stop('immediate') self.state_handler.remove_data_directory() - self.load_cluster_from_dcs() - self.bootstrap() + self.copy_backup_from_leader(cluster.leader) def process_scheduled_action(self): if self.reinitialize_scheduled(): - if self.dcs.cluster.is_unlocked(): + if self.cluster.is_unlocked(): logger.error('Cluster has no leader, can not reinitialize') self._async_executor.reset_scheduled_action() elif self.has_lock(): logger.error('I am the leader, can not reinitialize') self._async_executor.reset_scheduled_action() else: - self._async_executor.run_async(self.reinitialize) + self._async_executor.run_async(self.reinitialize, args=(self.cluster, )) return 'reinitialize started' def handle_long_action_in_progress(self): @@ -331,7 +334,7 @@ class Ha: return 'updated leader lock during ' + self._async_executor.scheduled_action else: return 'failed to update leader lock during ' + self._async_executor.scheduled_action - elif self.dcs.cluster.is_unlocked(): + elif self.cluster.is_unlocked(): return 'not healthy enough for leader race' else: return self._async_executor.scheduled_action + ' in progress' @@ -343,7 +346,7 @@ class Ha: self.touch_member() # cluster has leader key but not initialize key - if not self.dcs.cluster.is_unlocked() and not self.dcs.cluster.initialize: + if not self.cluster.is_unlocked() and not self.cluster.initialize: self.dcs.initialize() # fix it if self._async_executor.busy: @@ -358,7 +361,7 @@ class Ha: if self.state_handler.data_directory_empty(): return self.bootstrap() # new node # "bootstrap", but data directory is not empty - elif not self.dcs.cluster.initialize and self.dcs.cluster.is_unlocked(): + elif not self.cluster.initialize and self.cluster.is_unlocked(): self.dcs.initialize() # try to start dead postgres @@ -368,12 +371,12 @@ class Ha: return msg try: - if self.dcs.cluster.is_unlocked(): + if self.cluster.is_unlocked(): return self.process_unhealthy_cluster() else: return self.process_healthy_cluster() finally: - self.state_handler.sync_replication_slots(self.dcs.cluster) + self.state_handler.sync_replication_slots(self.cluster) except DCSError: logger.error('Error communicating with DCS') if self.state_handler.is_running() and self.state_handler.is_leader(): diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 68786485..c328ae52 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -164,9 +164,9 @@ class ZooKeeper(AbstractDCS): # get last leader operation self.last_leader_operation = self.get_node(self.leader_optime_path) if self.fetch_cluster else None self.last_leader_operation = 0 if self.last_leader_operation is None else int(self.last_leader_operation[0]) - self.cluster = Cluster(initialize, leader, self.last_leader_operation, members, failover) + self._cluster = Cluster(initialize, leader, self.last_leader_operation, members, failover) - def get_cluster(self): + def _load_cluster(self): if self.exhibitor and self.exhibitor.poll(): self.client.set_hosts(self.exhibitor.zookeeper_hosts) @@ -174,11 +174,9 @@ class ZooKeeper(AbstractDCS): try: self.client.retry(self._inner_load_cluster) except: - self.cluster = None logger.exception('get_cluster') self.session_listener(KazooState.LOST) raise ZooKeeperError('ZooKeeper in not responding properly') - return self.cluster def _create(self, path, value, **kwargs): try: @@ -206,7 +204,8 @@ class ZooKeeper(AbstractDCS): return self._create(self.initialize_path, self._name, makepath=True) def touch_member(self, data, ttl=None): - me = self.cluster and ([m for m in self.cluster.members if m.name == self._name] or [None])[0] + cluster = self.cluster + me = cluster and ([m for m in cluster.members if m.name == self._name] or [None])[0] path = self.member_path data = data.encode('utf-8') create = not me diff --git a/tests/test_ha.py b/tests/test_ha.py index 392ad663..0a986280 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -85,7 +85,7 @@ class MockPatroni: def run_async(func, args=()): - func(args) if args else func() + func(*args) if args else func() class TestHa(unittest.TestCase): @@ -101,7 +101,7 @@ class TestHa(unittest.TestCase): self.ha = Ha(MockPatroni(self.p, self.e)) self.ha._async_executor.run_async = run_async self.ha.old_cluster = self.e.get_cluster() - self.e.cluster = get_cluster_not_initialized_without_leader() + self.ha.cluster = get_cluster_not_initialized_without_leader() self.ha.load_cluster_from_dcs = Mock() def test_update_lock(self): @@ -161,28 +161,28 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') def test_promote_because_have_lock(self): - self.e.cluster.is_unlocked = false + self.ha.cluster.is_unlocked = false self.ha.has_lock = true self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock') def test_leader_with_lock(self): - self.e.cluster.is_unlocked = false + self.ha.cluster.is_unlocked = false self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') def test_demote_because_not_having_lock(self): - self.e.cluster.is_unlocked = false + self.ha.cluster.is_unlocked = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i do not have the lock and i was a leader') def test_demote_because_update_lock_failed(self): - self.e.cluster.is_unlocked = false + self.ha.cluster.is_unlocked = false self.ha.has_lock = true self.ha.update_lock = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i do not have the lock and i was a leader') def test_follow_the_leader(self): - self.e.cluster.is_unlocked = false + self.ha.cluster.is_unlocked = false self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'no action. i am a secondary and i am following a leader') @@ -191,27 +191,25 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'demoted self because DCS is not accessible and i was a leader') def test_bootstrap_from_leader(self): - self.e.cluster = get_cluster_initialized_with_leader() + self.ha.cluster = get_cluster_initialized_with_leader() self.p.bootstrap = false self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') - self.ha._async_executor._busy = True - self.assertEquals(self.ha.bootstrap(), 'trying to bootstrap from leader') def test_bootstrap_waiting_for_leader(self): - self.e.cluster = get_cluster_initialized_without_leader() + self.ha.cluster = get_cluster_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'waiting for leader to bootstrap') def test_bootstrap_initialize_lock_failed(self): - self.e.cluster = get_cluster_not_initialized_without_leader() + self.ha.cluster = get_cluster_not_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'failed to acquire initialize lock') def test_bootstrap_initialized_new_cluster(self): - self.e.cluster = get_cluster_not_initialized_without_leader() + self.ha.cluster = get_cluster_not_initialized_without_leader() self.e.initialize = true self.assertEquals(self.ha.bootstrap(), 'initialized a new cluster') def test_bootstrap_release_initialize_key_on_failure(self): - self.e.cluster = get_cluster_not_initialized_without_leader() + self.ha.cluster = get_cluster_not_initialized_without_leader() self.e.initialize = true self.p.bootstrap = Mock(side_effect=PostgresException("Could not bootstrap master PostgreSQL")) self.assertRaises(PostgresException, self.ha.bootstrap) @@ -222,7 +220,7 @@ class TestHa(unittest.TestCase): self.ha.run_cycle() self.assertIsNone(self.ha._async_executor.scheduled_action) - self.e.cluster = get_cluster_initialized_with_leader() + self.ha.cluster = get_cluster_initialized_with_leader() self.ha.has_lock = true self.ha.schedule_reinitialize() self.ha.run_cycle() @@ -244,7 +242,7 @@ class TestHa(unittest.TestCase): self.assertTrue(self.ha.restart_scheduled()) self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race') - self.e.cluster = get_cluster_initialized_with_leader() + self.ha.cluster = get_cluster_initialized_with_leader() self.assertEquals(self.ha.run_cycle(), 'restart in progress') self.ha.has_lock = true @@ -256,26 +254,26 @@ class TestHa(unittest.TestCase): @patch('requests.get', requests_get) def test_manual_failover_from_leader(self): self.ha.has_lock = true - self.e.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '')) + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '')) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') - self.e.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name)) + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name)) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') - self.e.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla')) + self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla')) self.assertEquals(self.ha.run_cycle(), 'no action. i am the leader with the lock') f = Failover(0, MockPostgresql.name, '') - self.e.cluster = get_cluster_initialized_with_leader(f) + self.ha.cluster = get_cluster_initialized_with_leader(f) self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself') @patch('requests.get', requests_get) def test_manual_failover_process_no_leader(self): self.p.is_leader = false - self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name)) + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name)) self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') - self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader')) + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader')) self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') self.ha.fetch_node_status = lambda e: (e, True, True, 0) # accessible, in_recovery self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') - self.e.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '')) + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '')) self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') self.ha.fetch_node_status = lambda e: (e, False, True, 0) # accessible, in_recovery self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') From d48f8384ed3314e634ca26682aaf16f2965fc409 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 6 Oct 2015 10:05:19 +0200 Subject: [PATCH 12/15] leader variable should be None if the leader.name == my name. This check has to be performed even check_recovery_conf call --- patroni/ha.py | 6 +++--- patroni/zookeeper.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 9203e153..d4102e61 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -107,10 +107,10 @@ class Ha: def follow_the_leader(self, demote_reason, follow_reason, refresh=True): refresh and self.load_cluster_from_dcs() ret = demote_reason if self.state_handler.is_leader() else follow_reason - if not self.state_handler.check_recovery_conf(self.cluster.leader): + leader = self.cluster.leader + leader = None if (leader and leader.name) == self.state_handler.name else leader + if not self.state_handler.check_recovery_conf(leader): self._async_executor.schedule('changing primary_conninfo and restarting') - leader = self.cluster.leader - leader = None if (leader and leader.name) == self.state_handler.name else leader self._async_executor.run_async(self.state_handler.follow_the_leader, (leader, )) return ret diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index c328ae52..c6bfe639 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -197,7 +197,7 @@ class ZooKeeper(AbstractDCS): except NoNodeError: return value == '' or (not index and self._create(self.failover_path, value.encode('utf-8'))) except: - logging.exception('foo') + logging.exception('set_failover_value') return False def initialize(self): From 8a844285ff83c5c85c0884dae31f0664dd153022 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 7 Oct 2015 16:48:39 +0200 Subject: [PATCH 13/15] Set fetch_cluster flag to False when _inner_load_cluster called Set the same flag to True if the cluster does not yet exists in ZooKeeper --- patroni/zookeeper.py | 3 +++ tests/test_zookeeper.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index c6bfe639..c7b9a44c 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -132,8 +132,11 @@ class ZooKeeper(AbstractDCS): return members def _inner_load_cluster(self): + self.fetch_cluster = False self.event.clear() nodes = set(self.get_children(self.client_path(''), self.cluster_watcher)) + if not nodes: + self.fetch_cluster = True # get initialize flag initialize = self._INITIALIZE in nodes diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index b4e004be..731eb53f 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -46,7 +46,7 @@ class MockKazooClient(Mock): def get_children(self, path, watch=None, include_data=False): if not isinstance(path, six.string_types): raise TypeError("Invalid type for 'path' (string expected)") - if path == '/no_node': + if path.startswith('/no_node'): raise NoNodeError elif path in ['/service/bla/', '/service/test/']: return ['initialize', 'leader', 'members', 'optime', 'failover'] @@ -121,6 +121,8 @@ class TestZooKeeper(unittest.TestCase): def test__inner_load_cluster(self): self.zk._base_path = self.zk._base_path.replace('test', 'bla') self.zk._inner_load_cluster() + self.zk._base_path = self.zk._base_path = '/no_node' + self.zk._inner_load_cluster() def test_get_cluster(self): self.assertRaises(ZooKeeperError, self.zk.get_cluster) From a6603e8b48647f28a57a45b38c904bb87f75ef47 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 8 Oct 2015 13:07:38 +0200 Subject: [PATCH 14/15] bugfix in zookeeper module: when master node was being attached to patroni/zookeeper (no cluster in zookeeper yet) patroni has never tried to "refetch" cluster from DCS. It was leeding to demote... --- patroni/zookeeper.py | 7 ++++--- tests/test_zookeeper.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index c7b9a44c..6f8ab981 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -165,8 +165,8 @@ class ZooKeeper(AbstractDCS): failover = Failover.from_node(failover[1].version, failover[0]) # get last leader operation - self.last_leader_operation = self.get_node(self.leader_optime_path) if self.fetch_cluster else None - self.last_leader_operation = 0 if self.last_leader_operation is None else int(self.last_leader_operation[0]) + optime = self.get_node(self.leader_optime_path) if self._OPTIME in nodes and self.fetch_cluster else None + self.last_leader_operation = 0 if optime is None else int(optime[0]) self._cluster = Cluster(initialize, leader, self.last_leader_operation, members, failover) def _load_cluster(self): @@ -280,5 +280,6 @@ class ZooKeeper(AbstractDCS): logger.exception("Unable to delete initialize key") def watch(self, timeout): - self.fetch_cluster = super(ZooKeeper, self).watch(timeout) + if super(ZooKeeper, self).watch(timeout): + self.fetch_cluster = True return self.fetch_cluster diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 731eb53f..84807270 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -172,5 +172,5 @@ class TestZooKeeper(unittest.TestCase): def test_watch(self): self.zk.watch(0) - self.zk.event.isSet = lambda: False + self.zk.event.isSet = lambda: True self.zk.watch(0) From cf6be5f58e72ba25ee6b904d90fce69a011fccf7 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 9 Oct 2015 16:02:34 +0200 Subject: [PATCH 15/15] add missing tests for async_executor --- tests/test_async_executor.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tests/test_async_executor.py diff --git a/tests/test_async_executor.py b/tests/test_async_executor.py new file mode 100644 index 00000000..17d7f94a --- /dev/null +++ b/tests/test_async_executor.py @@ -0,0 +1,18 @@ +import unittest + +from mock import Mock, patch +from patroni.async_executor import AsyncExecutor +from threading import Thread + + +class TestAsyncExecutor(unittest.TestCase): + + def setUp(self): + self.a = AsyncExecutor() + + @patch.object(Thread, 'start', Mock()) + def test_run_async(self): + self.a.run_async(Mock(return_value=True)) + + def test_run(self): + self.a.run(Mock(side_effect=Exception()))