From 3b1efff53e0db0e05b8bb7a2a4443994631293a2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 27 Aug 2015 10:53:22 +0200 Subject: [PATCH 01/66] Refactor `Cluster` object `Cluster.leader` is not reference to `Member` anymore, but to `Leader` `Leader` class contains field `index` (update index). This field is very useful for watching for events which changing leader key. Also `Leader` contains `member` field, which should reference real member. --- helpers/dcs.py | 15 ++++++++++++--- helpers/etcd.py | 7 ++++--- helpers/ha.py | 2 +- helpers/postgresql.py | 8 ++++---- helpers/zookeeper.py | 27 ++++++++++++--------------- tests/test_etcd.py | 12 ++++++++---- tests/test_patroni.py | 12 ++++++++---- tests/test_postgresql.py | 11 ++++++----- tests/test_zookeeper.py | 12 +++++++++--- 9 files changed, 64 insertions(+), 42 deletions(-) diff --git a/helpers/dcs.py b/helpers/dcs.py index c7140c22..f5984fd9 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -39,7 +39,7 @@ class DCSError(Exception): class Member(namedtuple('Member', 'index,name,conn_url,api_url,expiration,ttl')): """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 DCS + :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 @@ -50,17 +50,26 @@ class Member(namedtuple('Member', 'index,name,conn_url,api_url,expiration,ttl')) return calculate_ttl(self.expiration) or -1 +class Leader(namedtuple('Leader', 'index,expiration,ttl,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 member: reference to a `Member` object which represents current leader (see `Cluster.members`)""" + + class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,members')): """Immutable object (namedtuple) which represents PostgreSQL cluster. Consists of the following fields: :param initialize: boolean, shows whether this cluster has initialization key stored in DC or not. - :param leader: `Member` object which represents current leader of the cluster + :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""" def is_unlocked(self): - return not (self.leader and self.leader.name) + return not (self.leader and self.leader.member.name) class AbstractDCS: diff --git a/helpers/etcd.py b/helpers/etcd.py index add736c5..f767175b 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -8,7 +8,7 @@ import socket from dns.exception import DNSException from dns import resolver -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from helpers.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string from helpers.utils import sleep from requests.exceptions import RequestException @@ -170,8 +170,9 @@ class Etcd(AbstractDCS): # get leader leader = nodes.get('leader', None) if leader: - leader = Member(-1, leader.value, None, None, None, None) - leader = ([m for m in members if m.name == leader.name] or [leader])[0] + member = Member(-1, leader.value, None, None, None, None) + member = ([m for m in members if m.name == leader.value] or [member])[0] + leader = Leader(leader.modifiedIndex, leader.expiration, leader.ttl, member) return Cluster(initialize, leader, last_leader_operation, members) except etcd.EtcdKeyNotFound: diff --git a/helpers/ha.py b/helpers/ha.py index 8e283c55..ff67f0e0 100644 --- a/helpers/ha.py +++ b/helpers/ha.py @@ -31,7 +31,7 @@ class Ha: return self.dcs.update_leader(self.state_handler) def has_lock(self): - lock_owner = self.cluster.leader and self.cluster.leader.name + lock_owner = self.cluster.leader and self.cluster.leader.member.name logger.info('Lock owner: %s; I am %s', lock_owner, self.state_handler.name) return lock_owner == self.state_handler.name diff --git a/helpers/postgresql.py b/helpers/postgresql.py index ce8ca18f..ca4ea9c0 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -128,7 +128,7 @@ class Postgresql: os.path.exists(self.trigger_file) and os.unlink(self.trigger_file) def sync_from_leader(self, leader): - r = parseurl(leader.conn_url) + r = parseurl(leader.member.conn_url) pgpass = 'pgpass' with open(pgpass, 'w') as f: @@ -288,7 +288,7 @@ class Postgresql: if not os.path.isfile(self.recovery_conf): return False - pattern = leader and leader.conn_url and self.primary_conninfo(leader.conn_url) + pattern = leader and leader.member.conn_url and self.primary_conninfo(leader.member.conn_url) with open(self.recovery_conf, 'r') as f: for line in f: @@ -304,11 +304,11 @@ class Postgresql: f.write("""standby_mode = 'on' recovery_target_timeline = 'latest' """) - if leader and leader.conn_url: + if leader and leader.member.conn_url: f.write(""" primary_slot_name = '{}' primary_conninfo = '{}' -""".format(self.name, self.primary_conninfo(leader.conn_url))) +""".format(self.name, self.primary_conninfo(leader.member.conn_url))) for name, value in self.config.get('recovery_conf', {}).items(): f.write("{} = '{}'\n".format(name, value)) diff --git a/helpers/zookeeper.py b/helpers/zookeeper.py index cb2918cd..56e755a0 100644 --- a/helpers/zookeeper.py +++ b/helpers/zookeeper.py @@ -3,7 +3,7 @@ import random import requests import time -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from helpers.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string from helpers.utils import sleep from kazoo.client import KazooClient, KazooState from kazoo.exceptions import NoNodeError, NodeExistsError @@ -134,21 +134,18 @@ class ZooKeeper(AbstractDCS): leader = self.get_node('/leader', self.cluster_watcher) self.members = self.load_members() if leader: - if leader[0] == self._name: - client_id = self.client.client_id - if client_id is not None and client_id[0] != leader[1].ephemeralOwner: - logger.info('I am leader but not owner of the session. Removing leader node') - self.client.delete(self.client_path('/leader')) - leader = None + client_id = self.client.client_id + if leader[0] == self._name and client_id is not None and client_id[0] != leader[1].ephemeralOwner: + logger.info('I am leader but not owner of the session. Removing leader node') + self.client.delete(self.client_path('/leader')) + leader = None if leader: - for member in self.members: - if member.name == leader[0]: - leader = member - self.fetch_cluster = False - break - if not isinstance(leader, Member): - leader = Member(-1, leader, None, None, None, None) + member = Member(-1, leader[0], None, None, None, None) + member = ([m for m in self.members if m.name == leader[0]] or [member])[0] + leader = Leader(leader[1].mzxid, None, None, member) + self.fetch_cluster = member.index == -1 + self.leader = leader if self.fetch_cluster: last_leader_operation = self.get_node('/optime/leader') @@ -220,7 +217,7 @@ class ZooKeeper(AbstractDCS): return True def delete_leader(self): - if isinstance(self.leader, Member) and self.leader.name == self._name: + if isinstance(self.leader, Leader) and self.leader.member.name == self._name: self.client.delete(self.client_path('/leader')) def sleep(self, timeout): diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 38692c52..52de036e 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -8,7 +8,7 @@ import time import unittest from dns.exception import DNSException -from helpers.dcs import Cluster, DCSError, Member +from helpers.dcs import Cluster, DCSError, Leader, Member from helpers.etcd import Client, Etcd from mock import Mock, patch @@ -107,8 +107,12 @@ def time_sleep(_): pass +class SleepException(Exception): + pass + + def time_sleep_exception(_): - raise Exception() + raise SleepException() class MockSRV: @@ -204,7 +208,7 @@ class TestEtcd(unittest.TestCase): time.sleep = time_sleep_exception with patch.object(etcd.Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(side_effect=etcd.EtcdException) - self.assertRaises(Exception, self.etcd.get_etcd_client, {'discovery_srv': 'test'}) + self.assertRaises(SleepException, self.etcd.get_etcd_client, {'discovery_srv': 'test'}) def test_get_cluster(self): self.assertIsInstance(self.etcd.get_cluster(), Cluster) @@ -214,7 +218,7 @@ class TestEtcd(unittest.TestCase): self.assertIsNone(cluster.leader) def test_current_leader(self): - self.assertIsInstance(self.etcd.current_leader(), Member) + self.assertIsInstance(self.etcd.current_leader(), Leader) self.etcd._base_path = '/service/noleader' self.assertIsNone(self.etcd.current_leader()) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 67ac1ffd..575c7cf3 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -24,8 +24,12 @@ def nop(*args, **kwargs): pass +class SleepException(Exception): + pass + + def time_sleep(*args): - raise Exception() + raise SleepException() class Mock_BaseServer__is_shut_down: @@ -90,7 +94,7 @@ class TestPatroni(unittest.TestCase): Etcd.delete_leader = nop - self.assertRaises(Exception, main) + self.assertRaises(SleepException, main) Patroni.run = run Patroni.touch_member = touch_member @@ -100,10 +104,10 @@ class TestPatroni(unittest.TestCase): self.p.touch_member = self.touch_member self.p.ha.state_handler.sync_replication_slots = time_sleep self.p.ha.dcs.client.read = etcd_read - self.assertRaises(Exception, self.p.run) + self.assertRaises(SleepException, self.p.run) self.p.ha.state_handler.is_leader = lambda: False self.p.api.start = nop - self.assertRaises(Exception, self.p.run) + self.assertRaises(SleepException, self.p.run) def touch_member(self, ttl=None): if not self.touched: diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 040d1c66..08ba0b33 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -4,7 +4,7 @@ import shutil import subprocess import unittest -from helpers.dcs import Cluster, Member +from helpers.dcs import Cluster, Leader, Member from helpers.postgresql import Postgresql @@ -123,7 +123,8 @@ class TestPostgresql(unittest.TestCase): psycopg2.connect = psycopg2_connect if not os.path.exists(self.p.data_dir): os.makedirs(self.p.data_dir) - self.leader = Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', None, None, 28) + 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) @@ -156,7 +157,7 @@ class TestPostgresql(unittest.TestCase): self.p.follow_the_leader(None) self.p.demote(self.leader) self.p.follow_the_leader(self.leader) - self.p.follow_the_leader(self.other) + self.p.follow_the_leader(Leader(-1, None, 28, self.other)) def test_create_connection_users(self): cfg = self.p.config @@ -166,7 +167,7 @@ class TestPostgresql(unittest.TestCase): def test_create_replication_slots(self): self.p.start() - cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leader]) + cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem]) self.p.create_replication_slots(cluster) def test_query(self): @@ -180,7 +181,7 @@ class TestPostgresql(unittest.TestCase): 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.leader]) + 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)) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 53f5ab82..54adea5e 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -2,6 +2,7 @@ import helpers.zookeeper import requests import unittest +from helpers.dcs import Leader from helpers.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError from kazoo.client import KazooState from kazoo.exceptions import NoNodeError, NodeExistsError @@ -30,6 +31,10 @@ class MockEventHandler: return MockEvent() +class SleepException(Exception): + pass + + class MockKazooClient: def __init__(self, **kwargs): @@ -94,7 +99,7 @@ class MockKazooClient: def exhibitor_sleep(_): - raise Exception + raise SleepException class TestExhibitorEnsembleProvider(unittest.TestCase): @@ -108,7 +113,7 @@ class TestExhibitorEnsembleProvider(unittest.TestCase): helpers.zookeeper.sleep = exhibitor_sleep def test_init(self): - self.assertRaises(Exception, ExhibitorEnsembleProvider, ['localhost'], 8181) + self.assertRaises(SleepException, ExhibitorEnsembleProvider, ['localhost'], 8181) class TestZooKeeper(unittest.TestCase): @@ -136,7 +141,8 @@ class TestZooKeeper(unittest.TestCase): def test_get_cluster(self): self.assertRaises(ZooKeeperError, self.zk.get_cluster) self.zk.exhibitor.poll = lambda: True - self.zk.get_cluster() + cluster = self.zk.get_cluster() + self.assertIsInstance(cluster.leader, Leader) self.zk.touch_member('foo') self.zk.delete_leader() From fab321c6b07c7cd567904fa328f88bf5a1786201 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 1 Sep 2015 09:54:57 +0200 Subject: [PATCH 02/66] Watch for changes of leader key --- helpers/etcd.py | 34 +++++++++++++++++++++++++++++++++- tests/test_etcd.py | 24 ++++++++++++++++++++++++ tests/test_patroni.py | 3 ++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/helpers/etcd.py b/helpers/etcd.py index f767175b..ac83fce8 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -5,6 +5,8 @@ import os import random import requests import socket +import time +import urllib3 from dns.exception import DNSException from dns import resolver @@ -59,6 +61,11 @@ class Client(etcd.Client): logger.exception('Can not resolve SRV for %s', host) return [] + # try to workarond bug in python-etcd: https://github.com/jplana/python-etcd/issues/81 + def _result_from_response(self, response): + response.data.decode('utf-8') + return super(Client, self)._result_from_response(response) + def _get_machines_cache_from_srv(self, discovery_srv): """Fetch list of etcd-cluster member by resolving _etcd-server._tcp. SRV record. This record should contain list of host and peer ports which could be used to run @@ -136,6 +143,7 @@ class Etcd(AbstractDCS): self.ttl = config['ttl'] self.member_ttl = config.get('member_ttl', 3600) self.client = self.get_etcd_client(config) + self.cluster = None def get_etcd_client(self, config): client = None @@ -174,12 +182,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) - return Cluster(initialize, leader, last_leader_operation, members) + self.cluster = Cluster(initialize, leader, last_leader_operation, members) + return self.cluster except etcd.EtcdKeyNotFound: return Cluster(False, None, None, []) except: logger.exception('get_cluster') + self.cluster = None raise EtcdError('Etcd is not responding properly') @catch_etcd_errors @@ -213,3 +223,25 @@ class Etcd(AbstractDCS): @catch_etcd_errors def delete_leader(self): return self.client.delete(self.client_path('/leader'), prevValue=self._name) + + def sleep(self, timeout): + # 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.member.name != self._name: + end_time = time.time() + timeout + index = self.cluster.leader.index + + while index and timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect + try: + res = self.client.watch(self.client_path('/leader'), index=index + 1, timeout=timeout) + if res.action not in ['set', 'compareAndSwap'] or res.value != self.cluster.leader.member.name: + return + index = res.modifiedIndex + except urllib3.exceptions.TimeoutError: + self.client.http.clear() + return + except etcd.EtcdException: + index = None + + timeout = end_time - time.time() + + timeout > 0 and super(Etcd, self).sleep(timeout) diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 52de036e..8de4ca46 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -3,6 +3,7 @@ import dns.resolver import etcd import json import requests +import urllib3 import socket import time import unittest @@ -61,6 +62,20 @@ def requests_get(url, **kwargs): return response +def etcd_watch(key, index=None, timeout=None, recursive=None): + print ('watch', key, index, timeout) + if timeout == 1: + raise urllib3.exceptions.TimeoutError + elif timeout == 5: + return etcd.EtcdResult('delete', {}) + elif timeout == 10: + raise etcd.EtcdException + elif index == 20729: + return etcd.EtcdResult('set', {'value': 'postgresql1', 'modifiedIndex': index + 1}) + elif index == 20731: + return etcd.EtcdResult('set', {'value': 'postgresql2', 'modifiedIndex': index + 1}) + + def etcd_write(key, value, **kwargs): if key == '/service/test/leader': if kwargs.get('prevValue', None) == 'foo' or not kwargs.get('prevExist', True): @@ -237,3 +252,12 @@ class TestEtcd(unittest.TestCase): def test_delete_leader(self): self.etcd.client.delete = etcd_delete self.assertFalse(self.etcd.delete_leader()) + + def test_sleep(self): + self.etcd.client.watch = etcd_watch + self.etcd.sleep(100) + self.etcd.get_cluster() + self.etcd.sleep(1) + self.etcd.sleep(5) + self.etcd.sleep(10) + self.etcd.sleep(100) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 575c7cf3..c0ea2907 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -14,7 +14,7 @@ from helpers.zookeeper import ZooKeeper from mock import Mock, patch from patroni import Patroni, main from six.moves import BaseHTTPServer -from test_etcd import Client, etcd_read, etcd_write +from test_etcd import Client, etcd_read, etcd_write, etcd_watch from test_ha import true, false from test_postgresql import Postgresql, subprocess_call, psycopg2_connect from test_zookeeper import MockKazooClient @@ -104,6 +104,7 @@ class TestPatroni(unittest.TestCase): self.p.touch_member = self.touch_member self.p.ha.state_handler.sync_replication_slots = time_sleep self.p.ha.dcs.client.read = etcd_read + self.p.ha.dcs.sleep = time_sleep self.assertRaises(SleepException, self.p.run) self.p.ha.state_handler.is_leader = lambda: False self.p.api.start = nop From 10c95a23e4ff11cf1871bbb56a66b986a706878a Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 1 Sep 2015 09:59:37 +0200 Subject: [PATCH 03/66] Rename sleep to watch in a AbstractDCS This method suppose to watch for changes of leader key if current node is not leader and also it could watch for changes in a members list if current conde is the leader. --- helpers/dcs.py | 2 +- helpers/etcd.py | 4 ++-- helpers/zookeeper.py | 2 +- tests/test_etcd.py | 12 ++++++------ tests/test_zookeeper.py | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/helpers/dcs.py b/helpers/dcs.py index f5984fd9..63515e81 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -153,5 +153,5 @@ class AbstractDCS: """Voluntarily remove leader key from DCS This method should remove leader key if current instance is the leader""" - def sleep(self, timeout): + def watch(self, timeout): sleep(timeout) diff --git a/helpers/etcd.py b/helpers/etcd.py index ac83fce8..6bea59e7 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -224,7 +224,7 @@ class Etcd(AbstractDCS): def delete_leader(self): return self.client.delete(self.client_path('/leader'), prevValue=self._name) - def sleep(self, timeout): + def watch(self, timeout): # 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.member.name != self._name: end_time = time.time() + timeout @@ -244,4 +244,4 @@ class Etcd(AbstractDCS): timeout = end_time - time.time() - timeout > 0 and super(Etcd, self).sleep(timeout) + timeout > 0 and super(Etcd, self).watch(timeout) diff --git a/helpers/zookeeper.py b/helpers/zookeeper.py index 56e755a0..bf8b281a 100644 --- a/helpers/zookeeper.py +++ b/helpers/zookeeper.py @@ -220,7 +220,7 @@ class ZooKeeper(AbstractDCS): if isinstance(self.leader, Leader) and self.leader.member.name == self._name: self.client.delete(self.client_path('/leader')) - def sleep(self, timeout): + def watch(self, timeout): self.cluster_event.wait(timeout) if self.cluster_event.isSet(): self.fetch_cluster = True diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 8de4ca46..6754a68a 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -253,11 +253,11 @@ class TestEtcd(unittest.TestCase): self.etcd.client.delete = etcd_delete self.assertFalse(self.etcd.delete_leader()) - def test_sleep(self): + def test_watch(self): self.etcd.client.watch = etcd_watch - self.etcd.sleep(100) + self.etcd.watch(100) self.etcd.get_cluster() - self.etcd.sleep(1) - self.etcd.sleep(5) - self.etcd.sleep(10) - self.etcd.sleep(100) + self.etcd.watch(1) + self.etcd.watch(5) + self.etcd.watch(10) + self.etcd.watch(100) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 54adea5e..01100c4e 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -164,5 +164,5 @@ class TestZooKeeper(unittest.TestCase): self.zk.last_leader_operation = -1 self.assertTrue(self.zk.update_leader(MockPostgresql())) - def test_sleep(self): - self.zk.sleep(0) + def test_watch(self): + self.zk.watch(0) From 5a0634d7b0705cfc0ac1d18265e22b96dbecbe9b Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 1 Sep 2015 16:37:03 +0200 Subject: [PATCH 04/66] Create name property in a Cluster object It returns self.member.name and simplifies later usage in code. --- helpers/dcs.py | 9 +++++++-- helpers/etcd.py | 7 ++++--- helpers/ha.py | 2 +- helpers/zookeeper.py | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/helpers/dcs.py b/helpers/dcs.py index 63515e81..22c47e26 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -58,6 +58,10 @@ class Leader(namedtuple('Leader', 'index,expiration,ttl,member')): :param ttl: ttl of the leader key :param member: reference to a `Member` object which represents current leader (see `Cluster.members`)""" + @property + def name(self): + return self.member.name + class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,members')): """Immutable object (namedtuple) which represents PostgreSQL cluster. @@ -69,7 +73,7 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem :param members: list of Member object, all PostgreSQL cluster members including leader""" def is_unlocked(self): - return not (self.leader and self.leader.member.name) + return not (self.leader and self.leader.name) class AbstractDCS: @@ -83,7 +87,8 @@ class AbstractDCS: i.e.: `zookeeper` for zookeeper, `etcd` for etcd, etc... """ self._name = name - self._base_path = '/service/' + config['scope'] + self._scope = config['scope'] + self._base_path = '/service/' + self._scope def client_path(self, path): return self._base_path + path diff --git a/helpers/etcd.py b/helpers/etcd.py index 6bea59e7..d6739e2d 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -185,7 +185,8 @@ class Etcd(AbstractDCS): self.cluster = Cluster(initialize, leader, last_leader_operation, members) return self.cluster except etcd.EtcdKeyNotFound: - return Cluster(False, None, None, []) + self.cluster = Cluster(False, None, None, []) + return self.cluster except: logger.exception('get_cluster') @@ -226,14 +227,14 @@ class Etcd(AbstractDCS): def watch(self, timeout): # 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.member.name != self._name: + if self.cluster and self.cluster.leader and self.cluster.leader.name != self._name: end_time = time.time() + timeout index = self.cluster.leader.index while index and timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect try: res = self.client.watch(self.client_path('/leader'), index=index + 1, timeout=timeout) - if res.action not in ['set', 'compareAndSwap'] or res.value != self.cluster.leader.member.name: + if res.action not in ['set', 'compareAndSwap'] or res.value != self.cluster.leader.name: return index = res.modifiedIndex except urllib3.exceptions.TimeoutError: diff --git a/helpers/ha.py b/helpers/ha.py index ff67f0e0..8e283c55 100644 --- a/helpers/ha.py +++ b/helpers/ha.py @@ -31,7 +31,7 @@ class Ha: return self.dcs.update_leader(self.state_handler) def has_lock(self): - lock_owner = self.cluster.leader and self.cluster.leader.member.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 diff --git a/helpers/zookeeper.py b/helpers/zookeeper.py index bf8b281a..d1ccafa2 100644 --- a/helpers/zookeeper.py +++ b/helpers/zookeeper.py @@ -217,7 +217,7 @@ class ZooKeeper(AbstractDCS): return True def delete_leader(self): - if isinstance(self.leader, Leader) and self.leader.member.name == self._name: + if isinstance(self.leader, Leader) and self.leader.name == self._name: self.client.delete(self.client_path('/leader')) def watch(self, timeout): From 3924a90c7491ee719b94d867046526be09364dc7 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 1 Sep 2015 16:43:28 +0200 Subject: [PATCH 05/66] Create conn_url propery in a `Leader` object --- helpers/dcs.py | 4 ++++ helpers/postgresql.py | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/helpers/dcs.py b/helpers/dcs.py index 22c47e26..f837245c 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -62,6 +62,10 @@ class Leader(namedtuple('Leader', 'index,expiration,ttl,member')): def name(self): return self.member.name + @property + def conn_url(self): + return self.member.conn_url + class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,members')): """Immutable object (namedtuple) which represents PostgreSQL cluster. diff --git a/helpers/postgresql.py b/helpers/postgresql.py index ca4ea9c0..ce8ca18f 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -128,7 +128,7 @@ class Postgresql: os.path.exists(self.trigger_file) and os.unlink(self.trigger_file) def sync_from_leader(self, leader): - r = parseurl(leader.member.conn_url) + r = parseurl(leader.conn_url) pgpass = 'pgpass' with open(pgpass, 'w') as f: @@ -288,7 +288,7 @@ class Postgresql: if not os.path.isfile(self.recovery_conf): return False - pattern = leader and leader.member.conn_url and self.primary_conninfo(leader.member.conn_url) + pattern = leader and leader.conn_url and self.primary_conninfo(leader.conn_url) with open(self.recovery_conf, 'r') as f: for line in f: @@ -304,11 +304,11 @@ class Postgresql: f.write("""standby_mode = 'on' recovery_target_timeline = 'latest' """) - if leader and leader.member.conn_url: + if leader and leader.conn_url: f.write(""" primary_slot_name = '{}' primary_conninfo = '{}' -""".format(self.name, self.primary_conninfo(leader.member.conn_url))) +""".format(self.name, self.primary_conninfo(leader.conn_url))) for name, value in self.config.get('recovery_conf', {}).items(): f.write("{} = '{}'\n".format(name, value)) From e96265e4d157d2490455b6892a6f3c0819f3e07f Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 1 Sep 2015 17:05:05 +0200 Subject: [PATCH 06/66] Make Patroni suitable for packaging. - Restructured the repository: Moved patroni into its own directory. - Changed readme into reStructuredText --- README.md | 129 ------------ README.rst | 220 +++++++++++++++++++++ patroni/__init__.py | 1 + patroni/__main__.py | 4 + {helpers => patroni/helpers}/__init__.py | 0 {helpers => patroni/helpers}/api.py | 0 {helpers => patroni/helpers}/dcs.py | 2 +- {helpers => patroni/helpers}/etcd.py | 4 +- {helpers => patroni/helpers}/ha.py | 2 +- {helpers => patroni/helpers}/postgresql.py | 2 +- {helpers => patroni/helpers}/utils.py | 0 {helpers => patroni/helpers}/zookeeper.py | 4 +- patroni.py => patroni/patroni.py | 16 +- {scripts => patroni/scripts}/__init__.py | 0 {scripts => patroni/scripts}/aws.py | 0 {scripts => patroni/scripts}/restore.py | 0 release.sh | 28 +++ requirements-py3.txt | 4 +- setup.py | 27 ++- tests/test_api.py | 2 +- tests/test_aws.py | 2 +- tests/test_etcd.py | 4 +- tests/test_ha.py | 6 +- tests/test_patroni.py | 14 +- tests/test_postgresql.py | 4 +- tests/test_restore.py | 2 +- tests/test_utils.py | 2 +- tests/test_zookeeper.py | 8 +- 28 files changed, 310 insertions(+), 177 deletions(-) delete mode 100644 README.md create mode 100644 README.rst create mode 100644 patroni/__init__.py create mode 100644 patroni/__main__.py rename {helpers => patroni/helpers}/__init__.py (100%) rename {helpers => patroni/helpers}/api.py (100%) rename {helpers => patroni/helpers}/dcs.py (99%) rename {helpers => patroni/helpers}/etcd.py (98%) rename {helpers => patroni/helpers}/ha.py (99%) rename {helpers => patroni/helpers}/postgresql.py (99%) rename {helpers => patroni/helpers}/utils.py (100%) rename {helpers => patroni/helpers}/zookeeper.py (98%) rename patroni.py => patroni/patroni.py (93%) rename {scripts => patroni/scripts}/__init__.py (100%) rename {scripts => patroni/scripts}/aws.py (100%) rename {scripts => patroni/scripts}/restore.py (100%) create mode 100755 release.sh diff --git a/README.md b/README.md deleted file mode 100644 index 2582eb37..00000000 --- a/README.md +++ /dev/null @@ -1,129 +0,0 @@ -[![Build Status](https://travis-ci.org/zalando/patroni.svg?branch=master)](https://travis-ci.org/zalando/patroni) -[![Coverage Status](https://coveralls.io/repos/zalando/patroni/badge.svg?branch=master)](https://coveralls.io/r/zalando/patroni?branch=master) -# Patroni: A Template for PostgreSQL HA with ZooKeeper or etcd - -Patroni was previously known as Governor. - -*There are many ways to run high availability with PostgreSQL; here we present a template for you to create your own custom fit high availability solution using python and distributed configuration store (like ZooKeeper or etcd) for maximum accessibility.* - -## Getting Started -To get started, do the following from different terminals: - -``` -> etcd --data-dir=data/etcd -> ./patroni.py postgres0.yml -> ./patroni.py postgres1.yml -``` - -From there, you will see a high-availability cluster start up. Test -different settings in the YAML files to see how behavior changes. Kill -some of the different components to see how the system behaves. - -Add more `postgres*.yml` files to create an even larger cluster. - -We provide a haproxy configuration, which will give your application a single endpoint for connecting to the cluster's leader. To configure, run: - -``` -> haproxy -f haproxy.cfg -``` - -``` -> psql --host 127.0.0.1 --port 5000 postgres -``` - -## How Patroni works - -For a diagram of the high availability decision loop, see the included a PDF: [postgres-ha.pdf](https://github.com/zalando/patroni/blob/master/postgres-ha.pdf) - -## YAML Configuration - -For an example file, see `postgres0.yml`. Below is an explanation of settings: - -* *ttl*: the TTL to acquire the leader lock. Think of it as the length of time before automatic failover process is initiated. -* *loop_wait*: the number of seconds the loop will sleep - -* *restapi* - * *listen*: ip address + port that Patroni will listen to provide health-check information for haproxy. - * *connect_address*: ip address + port through which restapi is accessible. - -* *etcd* - * *scope*: the relative path used on etcd's http api for this deployment, thus you can run multiple HA deployments from a single etcd - * *ttl*: the TTL to acquire the leader lock. Think of it as the length of time before automatic failover process is initiated. - * *host*: the host:port for the etcd endpoint - -* *zookeeper* - * *scope*: the relative path used on etcd's http api for this deployment, thus you can run multiple HA deployments from a single etcd - * *session_timeout*: the TTL to acquire the leader lock. Think of it as the length of time before automatic failover process is initiated. - * *reconnects_timeout*: how long we should try to reconnect to ZooKeeper after connection loss. After this timeout we assume that we don't have lock anymore and will restart in read-only mode. - * *hosts*: list of ZooKeeper cluster members in format: [ 'host1:port1', 'host2:port2', 'etc...'] - * *exhibitor*: if you are running ZooKeeper cluster under Exhibitor supervisory the following section could be interesting for you - * *poll_interval*: how often list of ZooKeeper and Exhibitor nodes should be updated from Exhibitor - * *port*: Exhibitor port - * *hosts*: initial list of Exhibitor (ZooKeeper) nodes in format: [ 'host1', 'host2', 'etc...' ]. This list would be updated automatically when Exhibitor (ZooKeeper) cluster topology changes. - -* *postgresql* - * *name*: the name of the Postgres host, must be unique for the cluster - * *listen*: ip address + port that Postgres listening. Must be accessible from other nodes in the cluster if using streaming replication. - * *connect_address*: ip address + port through which Postgres is accessible from other nodes and applications. - * *data_dir*: file path to initialize and store Postgres data files - * *maximum_lag_on_failover*: the maximum bytes a follower may lag before it is not eligible become leader - * *pg_hba*: list of lines which should be added to pg_hba.conf - * *- host all all 0.0.0.0/0 md5* - * *replication* - * *username*: replication username, user will be created during initialization - * *password*: replication password, user will be created during initialization - * *network*: network setting for replication in pg_hba.conf - * *callbacks* callback scripts to run on certain actions. Patroni will pass current action, role and cluster name. See scripts/aws.py as an example on how to write them. - * *on_start*: a script to run when the cluster starts - * *on_stop*: a script to run when the cluster stops - * *on_restart*: a script to run when the cluster restarts - * *on_reload*: a script to run when configuration reload is triggered - * *on_role_change*: a script to run when the cluster is being promoted or demoted - * *superuser* - * *password*: password for postgres user. It would be set during initialization - * *admin*: - * *username*: admin username, user will be created during initialization. It would have CREATEDB and CREATEROLE privileges - * *password*: admin password, user will be created during initialization. - * *recovery_conf*: configuration settings written to recovery.conf when configuring follower - * *parameters*: list of configuration settings for Postgres - -## Replication choices - -Patroni uses Postgres' streaming replication. By default, this replication is asynchronous. For more information, see the [Postgres documentation on streaming replication](http://www.postgresql.org/docs/current/static/warm-standby.html#STREAMING-REPLICATION). - -Patroni's asynchronous replication configuration allows for `maximum_lag_on_failover` settings. This setting ensures failover will not occur if a follower is more than a certain number of bytes behind the follower. This setting should be increased or decreased based on business requirements. - -When asynchronous replication is not best for your use-case, investigate how Postgres's [synchronous replication](http://www.postgresql.org/docs/current/static/warm-standby.html#SYNCHRONOUS-REPLICATION) works. Synchronous replication ensures consistency across a cluster by confirming that writes are written to a secondary before returning to the connecting client with a success. The cost of synchronous replication will be reduced throughput on writes. This throughput will be entirely based on network performance. In hosted datacenter environments (like AWS, Rackspace, or any network you do not control), synchrous replication increases the variability of write performance significantly. If followers become inaccessible from the leader, the leader will becomes effectively readonly. - -To enable a simple synchronous replication test, add the follow lines to the `parameters` section of your YAML configuration files. - -```YAML - synchronous_commit: "on" - synchronous_standby_names: "*" -``` - -When using synchronous replication, use at least a 3-Postgres data nodes to ensure write availability if one host fails. - -Choosing your replication schema is dependent on the many business decisions. Investigate both async and sync replication, as well as other HA solutions, to determine which solution is best for you. - -## Applications should not use superusers - -When connecting from an application, always use a non-superuser. Patroni requires access to the database to function properly. By using a superuser from application, you can potentially use the entire connection pool, including the connections reserved for superusers with the `superuser_reserved_connections` setting. If Patroni cannot access the Primary, because the connection pool is full, behavior will be undesireable. - -## Requirements on a Mac - -Run the following on a Mac to install requirements: - -``` -brew install postgresql etcd haproxy libyaml python -pip install psycopg2 pyyaml -``` - -## Notice - -There are many different ways to do HA with PostgreSQL, see [the -PostgreSQL documentation](https://wiki.postgresql.org/wiki/Replication,_Clustering,_and_Connection_Pooling) for a complete list. - -We call this project a "template" because it is far from a one-size fits -all, or a plug-and-play replication system. It will have it's own -caveats. Use wisely. diff --git a/README.rst b/README.rst new file mode 100644 index 00000000..c8c5eb7c --- /dev/null +++ b/README.rst @@ -0,0 +1,220 @@ +|Build Status| |Coverage Status| # Patroni: A Template for PostgreSQL HA +with ZooKeeper or etcd + +Patroni was previously known as Governor. + +*There are many ways to run high availability with PostgreSQL; here we +present a template for you to create your own custom fit high +availability solution using python and distributed configuration store +(like ZooKeeper or etcd) for maximum accessibility.* + +Getting Started +--------------- + +To get started, do the following from different terminals: + +:: + + > etcd --data-dir=data/etcd + > ./patroni.py postgres0.yml + > ./patroni.py postgres1.yml + +From there, you will see a high-availability cluster start up. Test +different settings in the YAML files to see how behavior changes. Kill +some of the different components to see how the system behaves. + +Add more ``postgres*.yml`` files to create an even larger cluster. + +We provide a haproxy configuration, which will give your application a +single endpoint for connecting to the cluster's leader. To configure, +run: + +:: + + > haproxy -f haproxy.cfg + +:: + + > psql --host 127.0.0.1 --port 5000 postgres + +How Patroni works +----------------- + +For a diagram of the high availability decision loop, see the included a +PDF: +`postgres-ha.pdf `__ + +YAML Configuration +------------------ + +For an example file, see ``postgres0.yml``. Below is an explanation of +settings: + +- *ttl*: the TTL to acquire the leader lock. Think of it as the length + of time before automatic failover process is initiated. +- *loop\_wait*: the number of seconds the loop will sleep + +- *restapi* +- *listen*: ip address + port that Patroni will listen to provide + health-check information for haproxy. +- *connect\_address*: ip address + port through which restapi is + accessible. + +- *etcd* +- *scope*: the relative path used on etcd's http api for this + deployment, thus you can run multiple HA deployments from a single + etcd +- *ttl*: the TTL to acquire the leader lock. Think of it as the length + of time before automatic failover process is initiated. +- *host*: the host:port for the etcd endpoint + +- *zookeeper* +- *scope*: the relative path used on etcd's http api for this + deployment, thus you can run multiple HA deployments from a single + etcd +- *session\_timeout*: the TTL to acquire the leader lock. Think of it + as the length of time before automatic failover process is initiated. +- *reconnects\_timeout*: how long we should try to reconnect to + ZooKeeper after connection loss. After this timeout we assume that we + don't have lock anymore and will restart in read-only mode. +- *hosts*: list of ZooKeeper cluster members in format: [ + 'host1:port1', 'host2:port2', 'etc...'] +- *exhibitor*: if you are running ZooKeeper cluster under Exhibitor + supervisory the following section could be interesting for you + + - *poll\_interval*: how often list of ZooKeeper and Exhibitor nodes + should be updated from Exhibitor + - *port*: Exhibitor port + - *hosts*: initial list of Exhibitor (ZooKeeper) nodes in format: [ + 'host1', 'host2', 'etc...' ]. This list would be updated + automatically when Exhibitor (ZooKeeper) cluster topology changes. + +- *postgresql* +- *name*: the name of the Postgres host, must be unique for the cluster +- *listen*: ip address + port that Postgres listening. Must be + accessible from other nodes in the cluster if using streaming + replication. +- *connect\_address*: ip address + port through which Postgres is + accessible from other nodes and applications. +- *data\_dir*: file path to initialize and store Postgres data files +- *maximum\_lag\_on\_failover*: the maximum bytes a follower may lag + before it is not eligible become leader +- *pg\_hba*: list of lines which should be added to pg\_hba.conf + + - *- host all all 0.0.0.0/0 md5* + +- *replication* + + - *username*: replication username, user will be created during + initialization + - *password*: replication password, user will be created during + initialization + - *network*: network setting for replication in pg\_hba.conf + +- *callbacks* callback scripts to run on certain actions. Patroni will + pass current action, role and cluster name. See scripts/aws.py as an + example on how to write them. + + - *on\_start*: a script to run when the cluster starts + - *on\_stop*: a script to run when the cluster stops + - *on\_restart*: a script to run when the cluster restarts + - *on\_reload*: a script to run when configuration reload is + triggered + - *on\_role\_change*: a script to run when the cluster is being + promoted or demoted + +- *superuser* + + - *password*: password for postgres user. It would be set during + initialization + +- *admin*: + + - *username*: admin username, user will be created during + initialization. It would have CREATEDB and CREATEROLE privileges + - *password*: admin password, user will be created during + initialization. + +- *recovery\_conf*: configuration settings written to recovery.conf + when configuring follower +- *parameters*: list of configuration settings for Postgres + +Replication choices +------------------- + +Patroni uses Postgres' streaming replication. By default, this +replication is asynchronous. For more information, see the `Postgres +documentation on streaming +replication `__. + +Patroni's asynchronous replication configuration allows for +``maximum_lag_on_failover`` settings. This setting ensures failover will +not occur if a follower is more than a certain number of bytes behind +the follower. This setting should be increased or decreased based on +business requirements. + +When asynchronous replication is not best for your use-case, investigate +how Postgres's `synchronous +replication `__ +works. Synchronous replication ensures consistency across a cluster by +confirming that writes are written to a secondary before returning to +the connecting client with a success. The cost of synchronous +replication will be reduced throughput on writes. This throughput will +be entirely based on network performance. In hosted datacenter +environments (like AWS, Rackspace, or any network you do not control), +synchrous replication increases the variability of write performance +significantly. If followers become inaccessible from the leader, the +leader will becomes effectively readonly. + +To enable a simple synchronous replication test, add the follow lines to +the ``parameters`` section of your YAML configuration files. + +.. code:: YAML + + synchronous_commit: "on" + synchronous_standby_names: "*" + +When using synchronous replication, use at least a 3-Postgres data nodes +to ensure write availability if one host fails. + +Choosing your replication schema is dependent on the many business +decisions. Investigate both async and sync replication, as well as other +HA solutions, to determine which solution is best for you. + +Applications should not use superusers +-------------------------------------- + +When connecting from an application, always use a non-superuser. Patroni +requires access to the database to function properly. By using a +superuser from application, you can potentially use the entire +connection pool, including the connections reserved for superusers with +the ``superuser_reserved_connections`` setting. If Patroni cannot access +the Primary, because the connection pool is full, behavior will be +undesireable. + +Requirements on a Mac +--------------------- + +Run the following on a Mac to install requirements: + +:: + + brew install postgresql etcd haproxy libyaml python + pip install psycopg2 pyyaml + +Notice +------ + +There are many different ways to do HA with PostgreSQL, see `the +PostgreSQL +documentation `__ +for a complete list. + +We call this project a "template" because it is far from a one-size fits +all, or a plug-and-play replication system. It will have it's own +caveats. Use wisely. + +.. |Build Status| image:: https://travis-ci.org/zalando/patroni.svg?branch=master + :target: https://travis-ci.org/zalando/patroni +.. |Coverage Status| image:: https://coveralls.io/repos/zalando/patroni/badge.svg?branch=master + :target: https://coveralls.io/r/zalando/patroni?branch=master diff --git a/patroni/__init__.py b/patroni/__init__.py new file mode 100644 index 00000000..a3e6290d --- /dev/null +++ b/patroni/__init__.py @@ -0,0 +1 @@ +__version__ = '0.15' diff --git a/patroni/__main__.py b/patroni/__main__.py new file mode 100644 index 00000000..7390c1f1 --- /dev/null +++ b/patroni/__main__.py @@ -0,0 +1,4 @@ +import patroni + +if __name__ == '__main__': + patroni.main() diff --git a/helpers/__init__.py b/patroni/helpers/__init__.py similarity index 100% rename from helpers/__init__.py rename to patroni/helpers/__init__.py diff --git a/helpers/api.py b/patroni/helpers/api.py similarity index 100% rename from helpers/api.py rename to patroni/helpers/api.py diff --git a/helpers/dcs.py b/patroni/helpers/dcs.py similarity index 99% rename from helpers/dcs.py rename to patroni/helpers/dcs.py index c7140c22..69ccdebd 100644 --- a/helpers/dcs.py +++ b/patroni/helpers/dcs.py @@ -1,7 +1,7 @@ import abc from collections import namedtuple -from helpers.utils import calculate_ttl, sleep +from patroni.helpers.utils import calculate_ttl, sleep from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl diff --git a/helpers/etcd.py b/patroni/helpers/etcd.py similarity index 98% rename from helpers/etcd.py rename to patroni/helpers/etcd.py index add736c5..c58c1a78 100644 --- a/helpers/etcd.py +++ b/patroni/helpers/etcd.py @@ -8,8 +8,8 @@ import socket from dns.exception import DNSException from dns import resolver -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string -from helpers.utils import sleep +from patroni.helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from patroni.helpers.utils import sleep from requests.exceptions import RequestException logger = logging.getLogger(__name__) diff --git a/helpers/ha.py b/patroni/helpers/ha.py similarity index 99% rename from helpers/ha.py rename to patroni/helpers/ha.py index 8e283c55..0496a094 100644 --- a/helpers/ha.py +++ b/patroni/helpers/ha.py @@ -1,6 +1,6 @@ import logging -from helpers.dcs import DCSError +from patroni.helpers.dcs import DCSError from psycopg2 import InterfaceError, OperationalError logger = logging.getLogger(__name__) diff --git a/helpers/postgresql.py b/patroni/helpers/postgresql.py similarity index 99% rename from helpers/postgresql.py rename to patroni/helpers/postgresql.py index ce8ca18f..8593fb52 100644 --- a/helpers/postgresql.py +++ b/patroni/helpers/postgresql.py @@ -6,7 +6,7 @@ import shutil import subprocess import six -from helpers.utils import sleep +from patroni.helpers.utils import sleep from six.moves.urllib_parse import urlparse if six.PY3: diff --git a/helpers/utils.py b/patroni/helpers/utils.py similarity index 100% rename from helpers/utils.py rename to patroni/helpers/utils.py diff --git a/helpers/zookeeper.py b/patroni/helpers/zookeeper.py similarity index 98% rename from helpers/zookeeper.py rename to patroni/helpers/zookeeper.py index cb2918cd..fd5f4f5b 100644 --- a/helpers/zookeeper.py +++ b/patroni/helpers/zookeeper.py @@ -3,8 +3,8 @@ import random import requests import time -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string -from helpers.utils import sleep +from patroni.helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from patroni.helpers.utils import sleep from kazoo.client import KazooClient, KazooState from kazoo.exceptions import NoNodeError, NodeExistsError from requests.exceptions import RequestException diff --git a/patroni.py b/patroni/patroni.py similarity index 93% rename from patroni.py rename to patroni/patroni.py index e26a4059..dd7bd52e 100755 --- a/patroni.py +++ b/patroni/patroni.py @@ -5,12 +5,12 @@ import sys import time import yaml -from helpers.api import RestApiServer -from helpers.etcd import Etcd -from helpers.ha import Ha -from helpers.postgresql import Postgresql -from helpers.utils import setup_signal_handlers, sleep -from helpers.zookeeper import ZooKeeper +from patroni.helpers.api import RestApiServer +from patroni.helpers.etcd import Etcd +from patroni.helpers.ha import Ha +from patroni.helpers.postgresql import Postgresql +from patroni.helpers.utils import setup_signal_handlers, sleep +from patroni.helpers.zookeeper import ZooKeeper logger = logging.getLogger(__name__) @@ -117,7 +117,3 @@ def main(): patroni.touch_member(patroni.shutdown_member_ttl) # schedule member removal patroni.postgresql.stop() patroni.ha.dcs.delete_leader() - - -if __name__ == '__main__': - main() diff --git a/scripts/__init__.py b/patroni/scripts/__init__.py similarity index 100% rename from scripts/__init__.py rename to patroni/scripts/__init__.py diff --git a/scripts/aws.py b/patroni/scripts/aws.py similarity index 100% rename from scripts/aws.py rename to patroni/scripts/aws.py diff --git a/scripts/restore.py b/patroni/scripts/restore.py similarity index 100% rename from scripts/restore.py rename to patroni/scripts/restore.py diff --git a/release.sh b/release.sh new file mode 100755 index 00000000..5f97ba08 --- /dev/null +++ b/release.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +if [ $# -ne 1 ]; then + >&2 echo "usage: $0 " + exit 1 +fi + +set -xe + +python3 --version +git --version + +version=$1 + +sed -i "s/__version__ = .*/__version__ = '${version}'/" __init__.py +python3 setup.py clean +python3 setup.py test +python3 setup.py flake8 + +git add __init__.py + +git commit -m "Bumped version to $version" +git push + +python3 setup.py sdist bdist_wheel upload + +git tag ${version} +git push --tags diff --git a/requirements-py3.txt b/requirements-py3.txt index 0fd9dfb3..544577ad 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -5,5 +5,5 @@ psycopg2 PyYAML requests six -kazoo>=2.2.1 -python-etcd>=0.4.1 +kazoo +python-etcd diff --git a/setup.py b/setup.py index 62d05a69..82ac8de8 100644 --- a/setup.py +++ b/setup.py @@ -18,14 +18,23 @@ if sys.version_info < (2, 7, 0): __location__ = os.path.join(os.getcwd(), os.path.dirname(inspect.getfile(inspect.currentframe()))) +def read_version(package): + data = {} + with open(os.path.join(package, '__init__.py'), 'r') as fd: + exec(fd.read(), data) + return data['__version__'] NAME = 'patroni' -MAIN_PACKAGE = 'patroni.py' +MAIN_PACKAGE = 'patroni' HELPERS = 'helpers' SCRIPTS = 'scripts' -VERSION = '0.1' -DESCRIPTION = 'A Template for PostgreSQL HA with etcd' +VERSION = read_version(MAIN_PACKAGE) +DESCRIPTION = 'PostgreSQL High-Available orchestrator and CLI' LICENSE = 'The MIT License' +URL = 'https://github.com/zalando/patroni' +AUTHOR = 'Alexander Kukushkin, Alexey Klyukin, Feike Steenbergen' +AUTHOR_EMAIL = 'alexander.kukushkin@zalando.de, oleksii.kliukin@zalando.de, feike.steenbergen@zalando.de' +KEYWORDS = 'etcd governor patroni postgresql postgres ha zookeeper streaming replication' COVERAGE_XML = True COVERAGE_HTML = False @@ -38,7 +47,7 @@ CLASSIFIERS = [ 'Environment :: Console', 'Intended Audience :: Developers', 'Intended Audience :: System Administrators', - 'License :: OSI Approved :: The MIT License', + 'License :: OSI Approved :: MIT License', 'Operating System :: POSIX :: Linux', 'Programming Language :: Python', 'Programming Language :: Python :: 2.7', @@ -82,7 +91,8 @@ class PyTest(TestCommand): params['plugins'] = ['cov'] if self.junitxml: params['args'] += self.junitxml - params['args'] += ['--doctest-modules', HELPERS, '--doctest-modules', SCRIPTS, '-s'] + #params['args'] += ['--doctest-modules', MAIN_PACKAGE, '--doctest-modules', HELPERS, '--doctest-modules', SCRIPTS, '-s'] + params['args'] += ['--doctest-modules', MAIN_PACKAGE, '-s', '-vv'] errno = pytest.main(**params) sys.exit(errno) @@ -118,10 +128,13 @@ def setup_package(): setup( name=NAME, version=version, + url=URL, + author=AUTHOR, + author_email=AUTHOR_EMAIL, description=DESCRIPTION, license=LICENSE, - keywords='etcd governor patroni postgresql postgres ha zookeeper', - long_description=read('README.md'), + keywords=KEYWORDS, + long_description=read('README.rst'), classifiers=CLASSIFIERS, test_suite='tests', packages=setuptools.find_packages(exclude=['tests', 'tests.*']), diff --git a/tests/test_api.py b/tests/test_api.py index 91b36943..12d7798b 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,7 +1,7 @@ import psycopg2 import unittest -from helpers.api import RestApiHandler, RestApiServer +from patroni.helpers.api import RestApiHandler, RestApiServer from six import BytesIO as IO from test_postgresql import psycopg2_connect diff --git a/tests/test_aws.py b/tests/test_aws.py index 84d495fe..09c357f4 100644 --- a/tests/test_aws.py +++ b/tests/test_aws.py @@ -2,7 +2,7 @@ import unittest import requests import boto.ec2 from collections import namedtuple -from scripts.aws import AWSConnection +from patroni.scripts.aws import AWSConnection from requests.exceptions import RequestException diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 38692c52..3f09caa5 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -8,8 +8,8 @@ import time import unittest from dns.exception import DNSException -from helpers.dcs import Cluster, DCSError, Member -from helpers.etcd import Client, Etcd +from patroni.helpers.dcs import Cluster, DCSError, Member +from patroni.helpers.etcd import Client, Etcd from mock import Mock, patch diff --git a/tests/test_ha.py b/tests/test_ha.py index abfc4ac8..8f9a9c49 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1,8 +1,8 @@ import unittest -from helpers.dcs import Cluster, DCSError -from helpers.etcd import Client, Etcd -from helpers.ha import Ha +from patroni.helpers.dcs import Cluster, DCSError +from patroni.helpers.etcd import Client, Etcd +from patroni.helpers.ha import Ha from mock import Mock, patch from test_etcd import etcd_read, etcd_write diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 67ac1ffd..fd2e605d 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -1,5 +1,5 @@ import datetime -import helpers.zookeeper +import patroni.helpers.zookeeper import psycopg2 import subprocess import sys @@ -7,12 +7,12 @@ import time import unittest import yaml -from helpers.api import RestApiServer -from helpers.dcs import Cluster, Member -from helpers.etcd import Etcd -from helpers.zookeeper import ZooKeeper +from patroni.helpers.api import RestApiServer +from patroni.helpers.dcs import Cluster, Member +from patroni.helpers.etcd import Etcd +from patroni.helpers.zookeeper import ZooKeeper from mock import Mock, patch -from patroni import Patroni, main +from patroni.patroni import Patroni, main from six.moves import BaseHTTPServer from test_etcd import Client, etcd_read, etcd_write from test_ha import true, false @@ -70,7 +70,7 @@ class TestPatroni(unittest.TestCase): Postgresql.write_recovery_conf = self.write_recovery_conf def test_get_dcs(self): - helpers.zookeeper.KazooClient = MockKazooClient + patroni.helpers.zookeeper.KazooClient = MockKazooClient self.assertIsInstance(self.p.get_dcs('', {'zookeeper': {'scope': '', 'hosts': ''}}), ZooKeeper) self.assertRaises(Exception, self.p.get_dcs, '', {}) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 040d1c66..f79a1438 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -4,8 +4,8 @@ import shutil import subprocess import unittest -from helpers.dcs import Cluster, Member -from helpers.postgresql import Postgresql +from patroni.helpers.dcs import Cluster, Member +from patroni.helpers.postgresql import Postgresql def nop(*args, **kwargs): diff --git a/tests/test_restore.py b/tests/test_restore.py index 38b34c6a..2ffd8a58 100644 --- a/tests/test_restore.py +++ b/tests/test_restore.py @@ -1,7 +1,7 @@ import unittest from mock import MagicMock, patch import os -from scripts.restore import Restore, WALERestore +from patroni.scripts.restore import Restore, WALERestore def fake_cursor_fetchone(*args, **kwargs): diff --git a/tests/test_utils.py b/tests/test_utils.py index 76dbff66..3bb238b0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,7 +2,7 @@ import os import time import unittest -from helpers.utils import sigchld_handler, sigterm_handler, sleep +from patroni.helpers.utils import sigchld_handler, sigterm_handler, sleep def nop(*args, **kwargs): diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 53f5ab82..1333897a 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -1,8 +1,8 @@ -import helpers.zookeeper +import patroni.helpers.zookeeper import requests import unittest -from helpers.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError +from patroni.helpers.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError from kazoo.client import KazooState from kazoo.exceptions import NoNodeError, NodeExistsError from kazoo.protocol.states import ZnodeStat @@ -105,7 +105,7 @@ class TestExhibitorEnsembleProvider(unittest.TestCase): def set_up(self): requests.get = requests_get - helpers.zookeeper.sleep = exhibitor_sleep + patroni.helpers.zookeeper.sleep = exhibitor_sleep def test_init(self): self.assertRaises(Exception, ExhibitorEnsembleProvider, ['localhost'], 8181) @@ -119,7 +119,7 @@ class TestZooKeeper(unittest.TestCase): def set_up(self): requests.get = requests_get - helpers.zookeeper.KazooClient = MockKazooClient + patroni.helpers.zookeeper.KazooClient = MockKazooClient self.zk = ZooKeeper('foo', {'exhibitor': {'hosts': ['localhost', 'exhibitor'], 'port': 8181}, 'scope': 'test'}) def test_session_listener(self): From eb85caa3bd230927803dfc4df8b0dcf1c21d4e4d Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 1 Sep 2015 17:16:15 +0200 Subject: [PATCH 07/66] Remove spurious print --- tests/test_etcd.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 6754a68a..c42394fa 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -63,7 +63,6 @@ def requests_get(url, **kwargs): def etcd_watch(key, index=None, timeout=None, recursive=None): - print ('watch', key, index, timeout) if timeout == 1: raise urllib3.exceptions.TimeoutError elif timeout == 5: From c6abf857400c6f9cc4ca51bdd4205e3d2ef76ac5 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 2 Sep 2015 08:42:47 +0200 Subject: [PATCH 08/66] Bugfix: sleep method was renamed to watch --- patroni.py | 2 +- tests/test_patroni.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patroni.py b/patroni.py index e26a4059..97d34ca2 100755 --- a/patroni.py +++ b/patroni.py @@ -76,7 +76,7 @@ class Patroni: if nap_time <= 0: self.next_run = current_time else: - self.ha.dcs.sleep(nap_time) + self.ha.dcs.watch(nap_time) def run(self): self.api.start() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index c0ea2907..489c4fa6 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -104,7 +104,7 @@ class TestPatroni(unittest.TestCase): self.p.touch_member = self.touch_member self.p.ha.state_handler.sync_replication_slots = time_sleep self.p.ha.dcs.client.read = etcd_read - self.p.ha.dcs.sleep = time_sleep + self.p.ha.dcs.watch = time_sleep self.assertRaises(SleepException, self.p.run) self.p.ha.state_handler.is_leader = lambda: False self.p.api.start = nop From 0beecb97a7d022b2b45a07188eb994b0f5b8ac32 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 2 Sep 2015 13:25:33 +0200 Subject: [PATCH 09/66] Reschedule next ha loop after promote --- patroni.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/patroni.py b/patroni.py index 97d34ca2..a711d5a8 100755 --- a/patroni.py +++ b/patroni.py @@ -70,6 +70,8 @@ class Patroni: self.postgresql.load_replication_slots() def schedule_next_run(self): + if self.postgresql.is_promoted: + self.next_run = time.time() self.next_run += self.nap_time current_time = time.time() nap_time = self.next_run - current_time From 8bc9d003911e1bdc9c6d3ef5d4590ec45682de29 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 2 Sep 2015 13:26:40 +0200 Subject: [PATCH 10/66] Code cleanup --- helpers/etcd.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/helpers/etcd.py b/helpers/etcd.py index d6739e2d..67ac9c92 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -183,15 +183,13 @@ class Etcd(AbstractDCS): leader = Leader(leader.modifiedIndex, leader.expiration, leader.ttl, member) self.cluster = Cluster(initialize, leader, last_leader_operation, members) - return self.cluster except etcd.EtcdKeyNotFound: self.cluster = Cluster(False, None, None, []) - return self.cluster except: + self.cluster = None logger.exception('get_cluster') - - self.cluster = None - raise EtcdError('Etcd is not responding properly') + raise EtcdError('Etcd is not responding properly') + return self.cluster @catch_etcd_errors def touch_member(self, connection_string, ttl=None): From da132ca1fcc6b408652c923bf0fd353f67644cb4 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 2 Sep 2015 15:20:16 +0200 Subject: [PATCH 11/66] Do not rely on env vars for superuser role name. Environment variables may not be set in Docker. Instead, obtain the superuser role from the database. --- helpers/postgresql.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 695e45c1..8c9fd3c9 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -354,7 +354,8 @@ primary_conninfo = '{}' self.query('CREATE ROLE "{0}" WITH LOGIN SUPERUSER PASSWORD %s'.format( self.superuser['username']), self.superuser['password']) else: - self.query('ALTER ROLE "{0}" WITH PASSWORD %s'.format(os.environ['USER']), self.superuser['password']) + rolsuper = self.query("""SELECT rolname FROM pg_authid WHERE rolsuper = 't'""").fetchone()[0] + self.query('ALTER ROLE "{0}" WITH PASSWORD %s'.format(rolsuper), self.superuser['password']) if self.admin: self.query('CREATE ROLE "{0}" WITH LOGIN CREATEDB CREATEROLE PASSWORD %s'.format( self.admin['username']), self.admin['password']) From a670b598f4416b0e99acd7442e5a51295a5daaa3 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 2 Sep 2015 16:48:40 +0200 Subject: [PATCH 12/66] Implement unit test for reap_children function --- tests/test_utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 76dbff66..312277b6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,7 +2,7 @@ import os import time import unittest -from helpers.utils import sigchld_handler, sigterm_handler, sleep +from helpers.utils import reap_children, sigchld_handler, sigterm_handler, sleep def nop(*args, **kwargs): @@ -34,10 +34,11 @@ class TestUtils(unittest.TestCase): def test_sigterm_handler(self): self.assertRaises(SystemExit, sigterm_handler, None, None) - def test_sigchld_handler(self): - sigchld_handler(None, None) + def test_reap_children(self): + reap_children() os.waitpid = os_waitpid sigchld_handler(None, None) + reap_children() def test_sleep(self): time.sleep = time_sleep From b1afd5ddc4b2e756846942349b8312e247348588 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 3 Sep 2015 09:52:01 +0200 Subject: [PATCH 13/66] Refactoring to enable package building and succesfull installation of the Patroni package. Build the first packages of patroni and added them to pypi.python.org Update Dockerfile to use pip to install patroni. --- MANIFEST.in | 3 +++ Dockerfile => docker/Dockerfile | 19 ++++++++----------- docker/Dockerfile.test.patch | 13 +++++++++++++ docker/entrypoint.sh | 2 +- patroni/__init__.py | 2 +- patroni/helpers/__init__.py | 0 patroni/patroni.py | 12 ++++++------ patroni/scripts/__init__.py | 0 requirements-py3.txt | 4 ++-- setup.py | 5 ++++- 10 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 MANIFEST.in rename Dockerfile => docker/Dockerfile (71%) create mode 100644 docker/Dockerfile.test.patch delete mode 100644 patroni/helpers/__init__.py delete mode 100644 patroni/scripts/__init__.py diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 00000000..1a5e91cd --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,3 @@ +include requirements* +include *.rst +recursive-include patroni *.py diff --git a/Dockerfile b/docker/Dockerfile similarity index 71% rename from Dockerfile rename to docker/Dockerfile index 2068d856..1a1cb41c 100644 --- a/Dockerfile +++ b/docker/Dockerfile @@ -8,29 +8,26 @@ RUN apt-get update -y && apt-get install curl -y # Add PGDG repositories RUN echo "deb http://apt.postgresql.org/pub/repos/apt/ $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list -RUN curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add - +RUN curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install python python-psycopg2 python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN pip install python-etcd +RUN apt-get install postgresql-${PGVERSION} -y +RUN apt-get install python python-psycopg2 python-yaml python-requests python-dnspython python-pip python-mock -y +RUN pip install patroni + ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH -RUN mkdir -p /patroni/helpers -RUN mkdir -p /patroni/scripts -ADD patroni.py /patroni/patroni.py -ADD helpers /patroni/helpers -ADD scripts /patroni/scripts - ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl -## Setting up a simple script that will serve as an entrypoint +### Setting up a simple script that will serve as an entrypoint +RUN mkdir /patroni/ RUN mkdir /data/ && touch /var/log/etcd.log /var/log/etcd.err /pgpass /patroni/postgres.yml RUN chown postgres:postgres -R /patroni/ /data/ /pgpass /var/log/etcd.* /patroni/postgres.yml -ADD docker/entrypoint.sh /entrypoint.sh +ADD entrypoint.sh /entrypoint.sh EXPOSE 4001 5432 2380 diff --git a/docker/Dockerfile.test.patch b/docker/Dockerfile.test.patch new file mode 100644 index 00000000..376f3c20 --- /dev/null +++ b/docker/Dockerfile.test.patch @@ -0,0 +1,13 @@ +--- Dockerfile 2015-09-03 09:24:16.355412216 +0200 ++++ Dockerfile.test 2015-09-03 09:25:48.407879693 +0200 +@@ -15,7 +15,9 @@ + ENV PGVERSION 9.4 + RUN apt-get install postgresql-${PGVERSION} -y + RUN apt-get install python python-psycopg2 python-yaml python-requests python-dnspython python-pip python-mock -y +-RUN pip install patroni ++## We install prereqs from pypi, the package from testpypi ++RUN pip install --force-reinstall --upgrade kazoo boto python-etcd ++RUN pip install -i https://testpypi.python.org/pypi patroni + + + ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index ce40f6dc..b28f4ae6 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -133,5 +133,5 @@ then sleep 60 done else - exec /patroni/patroni.py /patroni/postgres.yml + exec patroni /patroni/postgres.yml fi diff --git a/patroni/__init__.py b/patroni/__init__.py index a3e6290d..b970b513 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -1 +1 @@ -__version__ = '0.15' +__version__ = '0.22' diff --git a/patroni/helpers/__init__.py b/patroni/helpers/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/patroni/patroni.py b/patroni/patroni.py index dd7bd52e..2ee32211 100755 --- a/patroni/patroni.py +++ b/patroni/patroni.py @@ -5,12 +5,12 @@ import sys import time import yaml -from patroni.helpers.api import RestApiServer -from patroni.helpers.etcd import Etcd -from patroni.helpers.ha import Ha -from patroni.helpers.postgresql import Postgresql -from patroni.helpers.utils import setup_signal_handlers, sleep -from patroni.helpers.zookeeper import ZooKeeper +from .helpers.api import RestApiServer +from .helpers.etcd import Etcd +from .helpers.ha import Ha +from .helpers.postgresql import Postgresql +from .helpers.utils import setup_signal_handlers, sleep +from .helpers.zookeeper import ZooKeeper logger = logging.getLogger(__name__) diff --git a/patroni/scripts/__init__.py b/patroni/scripts/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/requirements-py3.txt b/requirements-py3.txt index 544577ad..0fd9dfb3 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -5,5 +5,5 @@ psycopg2 PyYAML requests six -kazoo -python-etcd +kazoo>=2.2.1 +python-etcd>=0.4.1 diff --git a/setup.py b/setup.py index 82ac8de8..b90d1084 100644 --- a/setup.py +++ b/setup.py @@ -18,6 +18,7 @@ if sys.version_info < (2, 7, 0): __location__ = os.path.join(os.getcwd(), os.path.dirname(inspect.getfile(inspect.currentframe()))) + def read_version(package): data = {} with open(os.path.join(package, '__init__.py'), 'r') as fd: @@ -56,6 +57,8 @@ CLASSIFIERS = [ 'Programming Language :: Python :: Implementation :: CPython', ] +CONSOLE_SCRIPTS = ['patroni = patroni.patroni:main'] + class PyTest(TestCommand): @@ -91,7 +94,6 @@ class PyTest(TestCommand): params['plugins'] = ['cov'] if self.junitxml: params['args'] += self.junitxml - #params['args'] += ['--doctest-modules', MAIN_PACKAGE, '--doctest-modules', HELPERS, '--doctest-modules', SCRIPTS, '-s'] params['args'] += ['--doctest-modules', MAIN_PACKAGE, '-s', '-vv'] errno = pytest.main(**params) sys.exit(errno) @@ -144,6 +146,7 @@ def setup_package(): cmdclass=cmdclass, tests_require=['pytest-cov', 'pytest'], command_options=command_options, + entry_points={'console_scripts': CONSOLE_SCRIPTS}, ) From 04d6f7b418eb9fcecf87364ef23359b9abbd222a Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 3 Sep 2015 10:34:37 +0200 Subject: [PATCH 14/66] Make scripts into a module, so it can be imported. --- patroni/scripts/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 patroni/scripts/__init__.py diff --git a/patroni/scripts/__init__.py b/patroni/scripts/__init__.py new file mode 100644 index 00000000..e69de29b From 8984d991166ecf0b47c16b2ff6b39e4ff928f62f Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 3 Sep 2015 10:38:21 +0200 Subject: [PATCH 15/66] Make helpers into a module, so it can be imported. --- patroni/helpers/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 patroni/helpers/__init__.py diff --git a/patroni/helpers/__init__.py b/patroni/helpers/__init__.py new file mode 100644 index 00000000..e69de29b From 147d7c8566ba9b8ff2669b421d12e51cc3d63564 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 3 Sep 2015 10:45:06 +0200 Subject: [PATCH 16/66] Fix heading of the README --- README.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index c8c5eb7c..cb06c176 100644 --- a/README.rst +++ b/README.rst @@ -1,5 +1,8 @@ -|Build Status| |Coverage Status| # Patroni: A Template for PostgreSQL HA -with ZooKeeper or etcd +|Build Status| |Coverage Status| + +=== +Patroni: A Template for PostgreSQL HA with ZooKeeper or etcd +=== Patroni was previously known as Governor. From 58410db9dd804597b6c45cb5ec329f09eb362154 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 3 Sep 2015 11:47:08 +0200 Subject: [PATCH 17/66] Implemented Retry class inspired by KazooRetry --- helpers/__init__.py | 16 ++++++++++ helpers/dcs.py | 15 +-------- helpers/utils.py | 77 +++++++++++++++++++++++++++++++++++++++++++++ tests/test_utils.py | 58 +++++++++++++++++++++++++++++++++- 4 files changed, 151 insertions(+), 15 deletions(-) diff --git a/helpers/__init__.py b/helpers/__init__.py index e69de29b..2fe60065 100644 --- a/helpers/__init__.py +++ b/helpers/__init__.py @@ -0,0 +1,16 @@ +class PatroniException(Exception): + pass + + +class DCSError(PatroniException): + """Parent class for all kind of exceptions related to selected distributed configuration store""" + + def __init__(self, value): + self.value = value + + def __str__(self): + """ + >>> str(DCSError('foo')) + "'foo'" + """ + return repr(self.value) diff --git a/helpers/dcs.py b/helpers/dcs.py index f837245c..7ee97ca3 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -1,6 +1,7 @@ import abc from collections import namedtuple +from helpers import DCSError from helpers.utils import calculate_ttl, sleep from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl @@ -22,20 +23,6 @@ def parse_connection_string(value): return conn_url, api_url -class DCSError(Exception): - """Parent class for all kind of exceptions related to selected distributed configuration store""" - - def __init__(self, value): - self.value = value - - def __str__(self): - """ - >>> str(DCSError('foo')) - "'foo'" - """ - return repr(self.value) - - class Member(namedtuple('Member', 'index,name,conn_url,api_url,expiration,ttl')): """Immutable object (namedtuple) which represents single member of PostgreSQL cluster. Consists of the following fields: diff --git a/helpers/utils.py b/helpers/utils.py index c725e6d4..f0555f48 100644 --- a/helpers/utils.py +++ b/helpers/utils.py @@ -1,10 +1,13 @@ import datetime import os +import random import re import signal import sys import time +from helpers import DCSError + interrupted_sleep = False reap_children = False @@ -107,3 +110,77 @@ def reap_children(): pass finally: reap_children = False + + +class RetryFailedError(DCSError): + """Raised when retrying an operation ultimately failed, after retrying the maximum number of attempts.""" + + +class Retry: + """Helper for retrying a method in the face of retry-able exceptions""" + + def __init__(self, max_tries=1, delay=0.1, backoff=2, max_jitter=0.8, max_delay=3600, + sleep_func=time.sleep, deadline=None, retry_exceptions=DCSError): + """Create a :class:`Retry` instance for retrying function calls + + :param max_tries: How many times to retry the command. -1 means infinite tries. + :param delay: Initial delay between retry attempts. + :param backoff: Backoff multiplier between retry attempts. Defaults to 2 for exponential backoff. + :param max_jitter: Additional max jitter period to wait between retry attempts to avoid slamming the server. + :param max_delay: Maximum delay in seconds, regardless of other backoff settings. Defaults to one hour. + :param retry_exceptions: single exception or tuple""" + + self.max_tries = max_tries + self.delay = delay + self.backoff = backoff + self.max_jitter = int(max_jitter * 100) + self.max_delay = float(max_delay) + self._attempts = 0 + self._cur_delay = delay + self.deadline = deadline + self._cur_stoptime = None + self.sleep_func = sleep_func + self.retry_exceptions = retry_exceptions + + def reset(self): + """Reset the attempt counter""" + self._attempts = 0 + self._cur_delay = self.delay + self._cur_stoptime = None + + def copy(self): + """Return a clone of this retry manager""" + return Retry(max_tries=self.max_tries, delay=self.delay, backoff=self.backoff, + max_jitter=self.max_jitter / 100.0, max_delay=self.max_delay, sleep_func=self.sleep_func, + deadline=self.deadline, retry_exceptions=self.retry_exceptions) + + def __call__(self, func, *args, **kwargs): + """Call a function with arguments until it completes without throwing a `retry_exceptions` + + :param func: Function to call + :param args: Positional arguments to call the function with + :params kwargs: Keyword arguments to call the function with + + The function will be called until it doesn't throw one of the retryable exceptions""" + self.reset() + + while True: + try: + if self.deadline is not None and self._cur_stoptime is None: + self._cur_stoptime = time.time() + self.deadline + return func(*args, **kwargs) + except self.retry_exceptions: + # Note: max_tries == -1 means infinite tries. + if self._attempts == self.max_tries: + raise RetryFailedError("Too many retry attempts") + self._attempts += 1 + sleeptime = self._cur_delay + ( + random.randint(0, self.max_jitter) / 100.0) + + if self._cur_stoptime is not None and \ + time.time() + sleeptime >= self._cur_stoptime: + raise RetryFailedError("Exceeded retry deadline") + else: + self.sleep_func(sleeptime) + self._cur_delay = min(self._cur_delay * self.backoff, + self.max_delay) diff --git a/tests/test_utils.py b/tests/test_utils.py index 312277b6..da43ada3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,7 +2,8 @@ import os import time import unittest -from helpers.utils import reap_children, sigchld_handler, sigterm_handler, sleep +from helpers import DCSError +from helpers.utils import Retry, RetryFailedError, reap_children, sigchld_handler, sigterm_handler, sleep def nop(*args, **kwargs): @@ -43,3 +44,58 @@ class TestUtils(unittest.TestCase): def test_sleep(self): time.sleep = time_sleep sleep(0.01) + + +class TestRetrySleeper(unittest.TestCase): + + def _pass(self): + pass + + def _fail(self, times=1): + scope = dict(times=0) + + def inner(): + if scope['times'] >= times: + pass + else: + scope['times'] += 1 + raise DCSError('Failed!') + return inner + + def _makeOne(self, *args, **kwargs): + return Retry(*args, **kwargs) + + def test_reset(self): + retry = self._makeOne(delay=0, max_tries=2) + retry(self._fail()) + self.assertEquals(retry._attempts, 1) + retry.reset() + self.assertEquals(retry._attempts, 0) + + def test_too_many_tries(self): + retry = self._makeOne(delay=0) + self.assertRaises(RetryFailedError, retry, self._fail(times=999)) + self.assertEquals(retry._attempts, 1) + + def test_maximum_delay(self): + def sleep_func(_time): + pass + + retry = self._makeOne(delay=10, max_tries=100, sleep_func=sleep_func) + retry(self._fail(times=10)) + self.assertTrue(retry._cur_delay < 4000, retry._cur_delay) + # gevent's sleep function is picky about the type + self.assertEquals(type(retry._cur_delay), float) + + def test_deadline(self): + def sleep_func(_time): + pass + + retry = self._makeOne(deadline=0.0001, sleep_func=sleep_func) + self.assertRaises(RetryFailedError, retry, self._fail(times=10)) + + def test_copy(self): + _sleep = lambda t: None + retry = self._makeOne(sleep_func=_sleep) + rcopy = retry.copy() + self.assertTrue(rcopy.sleep_func is _sleep) From e8b2fb112b04d009030a3010e5decdbf2f03d9a9 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 3 Sep 2015 11:50:58 +0200 Subject: [PATCH 18/66] It is better to reap childs before running relatively long sleep --- patroni.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni.py b/patroni.py index a15f535b..3be2aa57 100755 --- a/patroni.py +++ b/patroni.py @@ -92,8 +92,8 @@ class Patroni: self.ha.state_handler.drop_replication_slots() except: logger.exception('Exception when changing replication slots') - self.schedule_next_run() reap_children() + self.schedule_next_run() def main(): From 66286733b2df8ed187e09aaa2a0259b86a4067cb Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 3 Sep 2015 15:03:09 +0200 Subject: [PATCH 19/66] Enable retry functionality for etcd with default deadline=10 seconds --- helpers/etcd.py | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/helpers/etcd.py b/helpers/etcd.py index 67ac9c92..226c500a 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -11,7 +11,7 @@ import urllib3 from dns.exception import DNSException from dns import resolver from helpers.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string -from helpers.utils import sleep +from helpers.utils import Retry, RetryFailedError, sleep from requests.exceptions import RequestException logger = logging.getLogger(__name__) @@ -63,7 +63,12 @@ class Client(etcd.Client): # try to workarond bug in python-etcd: https://github.com/jplana/python-etcd/issues/81 def _result_from_response(self, response): - response.data.decode('utf-8') + try: + response.data.decode('utf-8') + except urllib3.exceptions.TimeoutError: + raise + except Exception as e: + raise etcd.EtcdException('Unable to decode server response: %s' % e) return super(Client, self)._result_from_response(response) def _get_machines_cache_from_srv(self, discovery_srv): @@ -131,7 +136,7 @@ def catch_etcd_errors(func): def wrapper(*args, **kwargs): try: return not func(*args, **kwargs) is None - except etcd.EtcdException: + except (RetryFailedError, etcd.EtcdException): return False return wrapper @@ -142,9 +147,17 @@ class Etcd(AbstractDCS): 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, + 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) + def get_etcd_client(self, config): client = None while not client: @@ -162,7 +175,7 @@ class Etcd(AbstractDCS): def get_cluster(self): try: - result = self.client.read(self.client_path(''), recursive=True) + 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} # get initialize flag @@ -193,17 +206,22 @@ class Etcd(AbstractDCS): @catch_etcd_errors def touch_member(self, connection_string, ttl=None): - return self.client.set(self.client_path('/members/' + self._name), connection_string, ttl or self.member_ttl) + return self.retry(self.client.set, self.client_path('/members/' + self._name), + connection_string, ttl or self.member_ttl) @catch_etcd_errors def take_leader(self): return self.client.set(self.client_path('/leader'), self._name, self.ttl) - @catch_etcd_errors def attempt_to_acquire_leader(self): - ret = self.client.write(self.client_path('/leader'), self._name, ttl=self.ttl, prevExist=False) - ret or logger.info('Could not take out TTL lock') - return ret + try: + return not self.retry(self.client.write, self.client_path('/leader'), + self._name, ttl=self.ttl, prevExist=False) is None + except etcd.EtcdAlreadyExist: + logger.info('Could not take out TTL lock') + except (RetryFailedError, etcd.EtcdException): + pass + return False @catch_etcd_errors def write_leader_optime(self, state_handler): @@ -211,13 +229,13 @@ class Etcd(AbstractDCS): @catch_etcd_errors def update_leader(self, state_handler): - ret = self.client.test_and_set(self.client_path('/leader'), self._name, self._name, self.ttl) + ret = self.retry(self.client.test_and_set, self.client_path('/leader'), self._name, self._name, self.ttl) ret and self.write_leader_optime(state_handler) return ret @catch_etcd_errors def race(self, path): - return self.client.write(self.client_path(path), self._name, prevExist=False) + return self.retry(self.client.write, self.client_path(path), self._name, prevExist=False) @catch_etcd_errors def delete_leader(self): From 2f6399de27d2d7e4eb90d20dbfc8d6c9b35c6669 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 3 Sep 2015 16:35:58 +0200 Subject: [PATCH 20/66] More unit-tests for etcd --- tests/test_etcd.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_etcd.py b/tests/test_etcd.py index c42394fa..2f993f85 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -26,6 +26,10 @@ class MockResponse: @property def data(self): + if self.content == 'TimeoutError': + raise urllib3.exceptions.TimeoutError + if self.content == 'Exception': + raise Exception return self.content @property @@ -76,6 +80,8 @@ def etcd_watch(key, index=None, timeout=None, recursive=None): def etcd_write(key, value, **kwargs): + if key == '/service/exists/leader': + raise etcd.EtcdAlreadyExist if key == '/service/test/leader': if kwargs.get('prevValue', None) == 'foo' or not kwargs.get('prevExist', True): return True @@ -190,6 +196,15 @@ class TestClient(unittest.TestCase): self.assertEquals(self.client.get_srv_record('blabla'), []) self.assertEquals(self.client.get_srv_record('exception'), []) + def test__result_from_response(self): + response = MockResponse() + response.content = 'TimeoutError' + self.assertRaises(urllib3.exceptions.TimeoutError, self.client._result_from_response, response) + response.content = 'Exception' + self.assertRaises(etcd.EtcdException, self.client._result_from_response, response) + response.content = '{}' + self.assertRaises(etcd.EtcdException, self.client._result_from_response, response) + def test__get_machines_cache_from_srv(self): self.client.get_srv_record = lambda e: [('localhost', 2380)] self.client._get_machines_cache_from_srv('blabla') @@ -242,6 +257,12 @@ class TestEtcd(unittest.TestCase): def test_take_leader(self): self.assertFalse(self.etcd.take_leader()) + def testattempt_to_acquire_leader(self): + self.etcd._base_path = '/service/exists' + self.assertFalse(self.etcd.attempt_to_acquire_leader()) + self.etcd._base_path = '/service/failed' + self.assertFalse(self.etcd.attempt_to_acquire_leader()) + def test_update_leader(self): self.assertTrue(self.etcd.update_leader(MockPostgresql())) From 50420771ce3531248c779e52dbf61a2e4672f41c Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 3 Sep 2015 16:53:15 +0200 Subject: [PATCH 21/66] Retry when executing take_leader method. --- helpers/etcd.py | 2 +- tests/test_etcd.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helpers/etcd.py b/helpers/etcd.py index 226c500a..74ad36f8 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -211,7 +211,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def take_leader(self): - return self.client.set(self.client_path('/leader'), self._name, self.ttl) + return self.retry(self.client.set, self.client_path('/leader'), self._name, self.ttl) def attempt_to_acquire_leader(self): try: diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 2f993f85..f5d65e5f 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -202,7 +202,7 @@ class TestClient(unittest.TestCase): self.assertRaises(urllib3.exceptions.TimeoutError, self.client._result_from_response, response) response.content = 'Exception' self.assertRaises(etcd.EtcdException, self.client._result_from_response, response) - response.content = '{}' + response.content = b'{}' self.assertRaises(etcd.EtcdException, self.client._result_from_response, response) def test__get_machines_cache_from_srv(self): From 330f9023eecd0f5153a749a8208ac5324f2943df Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 17:46:41 -0700 Subject: [PATCH 22/66] Interim commit to make replication slots optional for 9.3 users. --- helpers/postgresql.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index ce8ca18f..a8d749be 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -43,6 +43,7 @@ class Postgresql: def __init__(self, config): self.config = config self.name = config['name'] + self.scope = config['scope'] self.listen_addresses, self.port = config['listen'].split(':') self.data_dir = config['data_dir'] @@ -50,6 +51,7 @@ class Postgresql: self.superuser = config['superuser'] self.admin = config['admin'] self.callback = config.get('callbacks', {}) + self.use_slots = config['use_slots'] self.recovery_conf = os.path.join(self.data_dir, 'recovery.conf') self.configuration_to_save = (os.path.join(self.data_dir, 'pg_hba.conf'), os.path.join(self.data_dir, 'postgresql.conf')) @@ -365,22 +367,24 @@ primary_conninfo = '{}' ELSE pg_current_xlog_location() - '0/00000'::pg_lsn END""").fetchone()[0] def load_replication_slots(self): - cursor = self.query("SELECT slot_name FROM pg_replication_slots WHERE slot_type='physical'") - self.members = [r[0] for r in cursor] + if self.use_slots: + cursor = self.query("SELECT slot_name FROM pg_replication_slots WHERE slot_type='physical'") + self.members = [r[0] for r in cursor] def sync_replication_slots(self, members): - # drop unused slots - for slot in set(self.members) - set(members): - self.query("""SELECT pg_drop_replication_slot(%s) - WHERE EXISTS(SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + if self.use_slots: + # drop unused slots + for slot in set(self.members) - set(members): + 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): - 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 + # create new slots + for slot in set(members) - set(self.members): + 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 def create_replication_slots(self, cluster): self.sync_replication_slots([m.name for m in cluster.members if m.name != self.name]) From 2b801a3cccc5ebc634889366bd5565a7f3be3604 Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 18:03:25 -0700 Subject: [PATCH 23/66] Next 9.3 compatibility commit. Removed pg_lsn, since it's not available in 9.3. --- helpers/postgresql.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index a8d749be..78a77174 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -258,7 +258,7 @@ class Postgresql: member_conn.autocommit = True member_cursor = member_conn.cursor() member_cursor.execute( - "SELECT pg_is_in_recovery(), %s - (pg_last_xlog_replay_location() - '0/0000000'::pg_lsn)", + "SELECT pg_is_in_recovery(), %s - pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000')", (self.xlog_position(), )) row = member_cursor.fetchone() member_cursor.close() @@ -307,10 +307,9 @@ class Postgresql: recovery_target_timeline = 'latest' """) if leader and leader.conn_url: - f.write(""" -primary_slot_name = '{}' -primary_conninfo = '{}' -""".format(self.name, self.primary_conninfo(leader.conn_url))) + f.write("""primary_conninfo = '{}'\n""".format(self.primary_conninfo(leader.conn_url))) + if self.use_slots: + f.write("""primary_slot_name = '{}'\n""".format(self.name)) for name, value in self.config.get('recovery_conf', {}).items(): f.write("{} = '{}'\n".format(name, value)) @@ -363,8 +362,8 @@ primary_conninfo = '{}' def xlog_position(self): return self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_last_xlog_replay_location() - '0/0000000'::pg_lsn - ELSE pg_current_xlog_location() - '0/00000'::pg_lsn END""").fetchone()[0] + THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000') + ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/00000') END""").fetchone()[0] def load_replication_slots(self): if self.use_slots: From f2338e074c006942c73b0f3f6b280895c6f0b65f Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 18:16:44 -0700 Subject: [PATCH 24/66] Added documentation, sample config for optional replication slots. --- README.md | 5 +++-- postgres0.yml | 1 + postgres1.yml | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2582eb37..cfa141f5 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,7 @@ For an example file, see `postgres0.yml`. Below is an explanation of settings: * *connect_address*: ip address + port through which Postgres is accessible from other nodes and applications. * *data_dir*: file path to initialize and store Postgres data files * *maximum_lag_on_failover*: the maximum bytes a follower may lag before it is not eligible become leader + * *use_slots*: whether or not to use replication_slots. Must be False for PostgreSQL 9.3. * *pg_hba*: list of lines which should be added to pg_hba.conf * *- host all all 0.0.0.0/0 md5* * *replication* @@ -84,8 +85,8 @@ For an example file, see `postgres0.yml`. Below is an explanation of settings: * *admin*: * *username*: admin username, user will be created during initialization. It would have CREATEDB and CREATEROLE privileges * *password*: admin password, user will be created during initialization. - * *recovery_conf*: configuration settings written to recovery.conf when configuring follower - * *parameters*: list of configuration settings for Postgres + * *recovery_conf*: additional configuration settings written to recovery.conf when configuring follower + * *parameters*: list of configuration settings for Postgres. Many of these are required for replication to work. ## Replication choices diff --git a/postgres0.yml b/postgres0.yml index a2a7ce44..15dff82c 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -30,6 +30,7 @@ postgresql: connect_address: 127.0.0.1:5432 data_dir: data/postgresql0 maximum_lag_on_failover: 1048576 # 1 megabyte in bytes + use_slots: True pg_hba: - host all all 0.0.0.0/0 md5 - hostssl all all 0.0.0.0/0 md5 diff --git a/postgres1.yml b/postgres1.yml index 6ef6b1c9..90731fe0 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -30,6 +30,7 @@ postgresql: connect_address: 127.0.0.1:5433 data_dir: data/postgresql1 maximum_lag_on_failover: 1048576 # 1 megabyte in bytes + use_slots: True pg_hba: - host all all 0.0.0.0/0 md5 - hostssl all all 0.0.0.0/0 md5 From f5627a498e4e46f5480b38020f8a23194314285e Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 18:41:30 -0700 Subject: [PATCH 25/66] Fixed test_postgresql.py to include use_slots. --- README.md | 2 +- tests/test_postgresql.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cfa141f5..345c12c3 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ For an example file, see `postgres0.yml`. Below is an explanation of settings: * *connect_address*: ip address + port through which Postgres is accessible from other nodes and applications. * *data_dir*: file path to initialize and store Postgres data files * *maximum_lag_on_failover*: the maximum bytes a follower may lag before it is not eligible become leader - * *use_slots*: whether or not to use replication_slots. Must be False for PostgreSQL 9.3. + * *use_slots*: whether or not to use replication_slots. Must be False for PostgreSQL 9.3, and you should comment out max_replication_slots. * *pg_hba*: list of lines which should be added to pg_hba.conf * *- host all all 0.0.0.0/0 md5* * *replication* diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index c4a3d6df..882886df 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -114,6 +114,7 @@ class TestPostgresql(unittest.TestCase): 'pg_hba': ['hostssl all all 0.0.0.0/0 md5', 'host all all 0.0.0.0/0 md5'], 'superuser': {'password': ''}, 'admin': {'username': 'admin', 'password': 'admin'}, + 'use_slots' : True, 'replication': {'username': 'replicator', 'password': 'rep-pass', 'network': '127.0.0.1/32'}, From 5f4a9ffabb77499f1565a1fa8e45eb2532d033dc Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 21:04:42 -0700 Subject: [PATCH 26/66] Text spacing changes in an attempt to get flake8 to stop complaining. --- helpers/postgresql.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 78a77174..2a6af912 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -43,7 +43,6 @@ class Postgresql: def __init__(self, config): self.config = config self.name = config['name'] - self.scope = config['scope'] self.listen_addresses, self.port = config['listen'].split(':') self.data_dir = config['data_dir'] @@ -257,8 +256,8 @@ class Postgresql: 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/0000000')", + member_cursor.execute("""SELECT pg_is_in_recovery(), + %s - pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000')""", (self.xlog_position(), )) row = member_cursor.fetchone() member_cursor.close() @@ -362,8 +361,8 @@ recovery_target_timeline = 'latest' def xlog_position(self): return self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000') - ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/00000') END""").fetchone()[0] + THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000') + ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/00000') END""").fetchone()[0] def load_replication_slots(self): if self.use_slots: From bdb1454e3505031ac609a26c228fa179288613fb Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 21:10:30 -0700 Subject: [PATCH 27/66] Another commit because flake8 is a huge waste of time. --- helpers/postgresql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 2a6af912..24f903e9 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -256,9 +256,9 @@ class Postgresql: 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/0000000')""", - (self.xlog_position(), )) + member_cursor.execute("""SELECT pg_is_in_recovery(), + %s - pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000')""", + (self.xlog_position(), )) row = member_cursor.fetchone() member_cursor.close() member_conn.close() From 2b62adae210f487ddf667f6787a81679a9b55a8c Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 21:22:59 -0700 Subject: [PATCH 28/66] Fixed inherited bug in is_healthiest_node() with LSN position. --- helpers/postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 24f903e9..62c7bb7e 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -257,7 +257,7 @@ class Postgresql: 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/0000000')""", + %s - pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/00000')""", (self.xlog_position(), )) row = member_cursor.fetchone() member_cursor.close() From 6df56fc6ccfe68259bd0d1556843e72d7e9b65e6 Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 21:41:33 -0700 Subject: [PATCH 29/66] Another try at the is_healthiest_node bugfix. --- helpers/postgresql.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 62c7bb7e..bc6aed5b 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -382,7 +382,8 @@ recovery_target_timeline = 'latest' 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.members = members def create_replication_slots(self, cluster): self.sync_replication_slots([m.name for m in cluster.members if m.name != self.name]) From aeea7196bd1eebac4411c414786cf2175bad61da Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 21:46:06 -0700 Subject: [PATCH 30/66] Fixing flake8 issue. --- helpers/postgresql.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index bc6aed5b..dbdecb8f 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -382,8 +382,8 @@ recovery_target_timeline = 'latest' 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.members = members def create_replication_slots(self, cluster): self.sync_replication_slots([m.name for m in cluster.members if m.name != self.name]) From 5612cd0280ec016831efa0e461f7825d1bac3400 Mon Sep 17 00:00:00 2001 From: Josh Berkus Date: Thu, 3 Sep 2015 22:20:27 -0700 Subject: [PATCH 31/66] Changed use_slots to be backwards compatible by using config.get() per PR feedback. --- helpers/postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index dbdecb8f..03e4f4ef 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -50,7 +50,7 @@ class Postgresql: self.superuser = config['superuser'] self.admin = config['admin'] self.callback = config.get('callbacks', {}) - self.use_slots = config['use_slots'] + self.use_slots = config.get('use_slots',True) self.recovery_conf = os.path.join(self.data_dir, 'recovery.conf') self.configuration_to_save = (os.path.join(self.data_dir, 'pg_hba.conf'), os.path.join(self.data_dir, 'postgresql.conf')) From 936173272260288023dade904324067dbcc7e311 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 4 Sep 2015 08:29:16 +0200 Subject: [PATCH 32/66] Fix pep8 formatting and unit-tests --- helpers/postgresql.py | 20 ++++++++++---------- tests/test_postgresql.py | 3 +-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index ffe93be3..db4d3cd8 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -50,7 +50,7 @@ class Postgresql: self.superuser = config['superuser'] self.admin = config['admin'] self.callback = config.get('callbacks', {}) - self.use_slots = config.get('use_slots',True) + self.use_slots = config.get('use_slots', True) self.recovery_conf = os.path.join(self.data_dir, 'recovery.conf') self.configuration_to_save = (os.path.join(self.data_dir, 'pg_hba.conf'), os.path.join(self.data_dir, 'postgresql.conf')) @@ -256,9 +256,9 @@ class Postgresql: 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/00000')""", - (self.xlog_position(), )) + 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() @@ -362,8 +362,8 @@ recovery_target_timeline = 'latest' def xlog_position(self): return self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0000000') - ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/00000') END""").fetchone()[0] + THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0') + ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/0') END""").fetchone()[0] def load_replication_slots(self): if self.use_slots: @@ -375,14 +375,14 @@ recovery_target_timeline = 'latest' # drop unused slots for slot in set(self.members) - set(members): self.query("""SELECT pg_drop_replication_slot(%s) - WHERE EXISTS(SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + 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): self.query("""SELECT pg_create_physical_replication_slot(%s) - WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots + WHERE slot_name = %s)""", slot, slot) self.members = members diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 30278f87..ade68d3a 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -111,7 +111,6 @@ class TestPostgresql(unittest.TestCase): 'pg_hba': ['hostssl all all 0.0.0.0/0 md5', 'host all all 0.0.0.0/0 md5'], 'superuser': {'password': ''}, 'admin': {'username': 'admin', 'password': 'admin'}, - 'use_slots' : True, 'replication': {'username': 'replicator', 'password': 'rep-pass', 'network': '127.0.0.1/32'}, @@ -120,7 +119,7 @@ class TestPostgresql(unittest.TestCase): 'on_restart': 'true', 'on_role_change': 'true', 'on_reload': 'true' }, - 'restore': '/usr/bin/true'}) + 'restore': 'true'}) psycopg2.connect = psycopg2_connect if not os.path.exists(self.p.data_dir): os.makedirs(self.p.data_dir) From 8b9e99090fc2cf2c8808fea711c9e8f9db9e588e Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 4 Sep 2015 10:41:59 +0200 Subject: [PATCH 33/66] Move lsn_to_bytes and bytes_to_lsn into Postgresql class and make their behavior version specific --- helpers/postgresql.py | 33 +++++++++++++++++++++++++++++---- helpers/utils.py | 26 -------------------------- tests/test_postgresql.py | 1 + 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index db4d3cd8..f84f87a0 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -4,14 +4,10 @@ import psycopg2 import shlex import shutil import subprocess -import six from helpers.utils import sleep from six.moves.urllib_parse import urlparse -if six.PY3: - long = int - logger = logging.getLogger(__name__) ACTION_ON_START = "on_start" @@ -39,6 +35,7 @@ def parseurl(url): class Postgresql: + _SERVER_VERSION = 90400 def __init__(self, config): self.config = config @@ -85,6 +82,7 @@ class Postgresql: r = parseurl('postgres://{}/postgres'.format(self.local_address)) self._connection = psycopg2.connect(**r) self._connection.autocommit = True + self._SERVER_VERSION = self._connection.server_version return self._connection def _cursor(self): @@ -394,3 +392,30 @@ recovery_target_timeline = 'latest' def last_operation(self): return str(self.xlog_position()) + + @staticmethod + def lsn_to_bytes(value): + """ + >>> Postgresql.lsn_to_bytes('1/66000060') + 6006243424 + >>> Postgresql.lsn_to_bytes('j/66000060') + 0 + """ + try: + multiplier = 0xFF000000 if Postgresql._SERVER_VERSION < 90300 else 0x100000000 + e = value.split('/') + if len(e) == 2 and len(e[0]) > 0 and len(e[1]) > 0: + return int(e[0], 16) * multiplier + int(e[1], 16) + except ValueError: + return 0 + + @staticmethod + def bytes_to_lsn(value): + """ + >>> Postgresql.bytes_to_lsn(6006243424) + '1/66000060' + """ + divider = 0xFF000000 if Postgresql._SERVER_VERSION < 90300 else 0x100000000 + segment = value / divider + offset = value % divider + return '%X/%X' % (segment, offset) diff --git a/helpers/utils.py b/helpers/utils.py index c725e6d4..c0553374 100644 --- a/helpers/utils.py +++ b/helpers/utils.py @@ -42,32 +42,6 @@ def calculate_ttl(expiration): return int((expiration - now).total_seconds()) -def lsn_to_bytes(value): - """ - >>> lsn_to_bytes('1/66000060') - 6006243424 - >>> lsn_to_bytes('j/66000060') - 0 - """ - try: - e = value.split('/') - if len(e) == 2 and len(e[0]) > 0 and len(e[1]) > 0: - return (int(e[0], 16) << 32) | int(e[1], 16) - except ValueError: - pass - return 0 - - -def bytes_to_lsn(value): - """ - >>> bytes_to_lsn(6006243424) - '1/66000060' - """ - id = value >> 32 - off = value & 0xffffffff - return '%x/%x' % (id, off) - - def sigterm_handler(signo, stack_frame): sys.exit() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index ade68d3a..f8187c9f 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -79,6 +79,7 @@ class MockConnect: def __init__(self): self.autocommit = False self.closed = 0 + self.server_version = 90400 def cursor(self): return MockCursor() From c913c8ad9270ee3ae100ac3912d06d2ace6d395b Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 4 Sep 2015 12:11:55 +0200 Subject: [PATCH 34/66] Calculate xlog bytes with using lsn_to_bytes method This method behaves differently depending on server version and will allow to use patroni with postgres older than 9.3 --- helpers/postgresql.py | 27 +++++++++++++++------------ tests/test_postgresql.py | 19 +++++++++++-------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index f84f87a0..642c48f4 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -82,7 +82,7 @@ class Postgresql: r = parseurl('postgres://{}/postgres'.format(self.local_address)) self._connection = psycopg2.connect(**r) self._connection.autocommit = True - self._SERVER_VERSION = self._connection.server_version + Postgresql._SERVER_VERSION = self._connection.server_version return self._connection def _cursor(self): @@ -254,9 +254,7 @@ class Postgresql: 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(), )) + member_cursor.execute("SELECT pg_is_in_recovery(), COALESCE(pg_last_xlog_replay_location(), '0/0')") row = member_cursor.fetchone() member_cursor.close() member_conn.close() @@ -264,7 +262,7 @@ class Postgresql: if not row[0]: logger.warning('Master (%s) is still alive', member.name) return False - if row[1] < 0: + if self.xlog_position() < self.lsn_to_bytes(row[1], member_conn.server_version): return False except psycopg2.Error: continue @@ -359,9 +357,10 @@ recovery_target_timeline = 'latest' self.admin['username']), self.admin['password']) def xlog_position(self): - return self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_xlog_location_diff(pg_last_xlog_replay_location(),'0/0') - ELSE pg_xlog_location_diff(pg_current_xlog_location(),'0/0') END""").fetchone()[0] + lsn = self.query("""SELECT CASE WHEN pg_is_in_recovery() + THEN pg_last_xlog_replay_location() + ELSE pg_current_xlog_location() END""").fetchone()[0] + return self.lsn_to_bytes(lsn) def load_replication_slots(self): if self.use_slots: @@ -394,15 +393,17 @@ recovery_target_timeline = 'latest' return str(self.xlog_position()) @staticmethod - def lsn_to_bytes(value): + def lsn_to_bytes(value, version=None): """ >>> Postgresql.lsn_to_bytes('1/66000060') 6006243424 >>> Postgresql.lsn_to_bytes('j/66000060') 0 """ + if version is None: + version = Postgresql._SERVER_VERSION try: - multiplier = 0xFF000000 if Postgresql._SERVER_VERSION < 90300 else 0x100000000 + multiplier = 0xFF000000 if version < 90300 else 0x100000000 e = value.split('/') if len(e) == 2 and len(e[0]) > 0 and len(e[1]) > 0: return int(e[0], 16) * multiplier + int(e[1], 16) @@ -410,12 +411,14 @@ recovery_target_timeline = 'latest' return 0 @staticmethod - def bytes_to_lsn(value): + def bytes_to_lsn(value, version=None): """ >>> Postgresql.bytes_to_lsn(6006243424) '1/66000060' """ - divider = 0xFF000000 if Postgresql._SERVER_VERSION < 90300 else 0x100000000 + if version is None: + version = Postgresql._SERVER_VERSION + divider = 0xFF000000 if version < 90300 else 0x100000000 segment = value / divider offset = value % divider return '%X/%X' % (segment, offset) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index f8187c9f..a63c240b 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -21,13 +21,14 @@ def false(*args, **kwargs): class MockCursor: + _count = 0 def __init__(self): self.closed = False - self.current = 0 self.results = [] def execute(self, sql, *params): + MockCursor._count += 1 if sql.startswith('blabla'): raise psycopg2.OperationalError() elif sql.startswith('InterfaceError'): @@ -36,15 +37,15 @@ class MockCursor: 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: + elif sql.startswith('SELECT pg_is_in_recovery(), COALESCE'): + if MockCursor._count == 1: raise psycopg2.OperationalError() - elif params[0][0] == 2: - self.results = [(True, -1)] + elif MockCursor._count == 2: + self.results = [(True, '0/1')] else: - self.results = [(False, 0)] + self.results = [(False, '0/1')] elif sql.startswith('SELECT CASE WHEN pg_is_in_recovery()'): - self.results = [(0,)] + self.results = [('0/0', )] elif sql.startswith('SELECT pg_is_in_recovery()'): self.results = [(False, )] elif sql.startswith('SELECT to_char(pg_postmaster_start_time'): @@ -184,12 +185,14 @@ class TestPostgresql(unittest.TestCase): cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leader]) self.assertTrue(self.p.is_healthiest_node(cluster)) self.p.is_leader = false + MockCursor._count = 0 self.assertFalse(self.p.is_healthiest_node(cluster)) + MockCursor._count = 0 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'] = -2 + self.p.config['maximum_lag_on_failover'] = -3 self.assertFalse(self.p.is_healthiest_node(cluster)) def test_is_leader(self): From a69565fc5f6a002c97faea8e6e544ecb17aeae73 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 4 Sep 2015 13:22:44 +0200 Subject: [PATCH 35/66] Set Postgresql._SERVER_VERSION only after real connect. --- helpers/postgresql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 642c48f4..3fc7983a 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -82,7 +82,7 @@ class Postgresql: r = parseurl('postgres://{}/postgres'.format(self.local_address)) self._connection = psycopg2.connect(**r) self._connection.autocommit = True - Postgresql._SERVER_VERSION = self._connection.server_version + Postgresql._SERVER_VERSION = self._connection.server_version return self._connection def _cursor(self): @@ -358,8 +358,8 @@ recovery_target_timeline = 'latest' def xlog_position(self): lsn = self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_last_xlog_replay_location() - ELSE pg_current_xlog_location() END""").fetchone()[0] + THEN pg_last_xlog_replay_location() + ELSE pg_current_xlog_location() END""").fetchone()[0] return self.lsn_to_bytes(lsn) def load_replication_slots(self): From 650e2449042763ccac6bd923f4549db9a5b31253 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 4 Sep 2015 16:06:44 +0200 Subject: [PATCH 36/66] Refactor directory structure in preparation for building pypi-package --- patroni.py | 120 +---------------------- patroni/__init__.py | 119 ++++++++++++++++++++++ patroni/__main__.py | 5 + {helpers => patroni}/api.py | 0 {helpers => patroni}/dcs.py | 2 +- {helpers => patroni}/etcd.py | 4 +- {helpers => patroni}/ha.py | 2 +- {helpers => patroni}/postgresql.py | 2 +- {helpers => patroni/scripts}/__init__.py | 0 {scripts => patroni/scripts}/aws.py | 0 {scripts => patroni/scripts}/restore.py | 0 {helpers => patroni}/utils.py | 0 patroni/version.py | 1 + {helpers => patroni}/zookeeper.py | 4 +- scripts/__init__.py | 0 setup.py | 18 ++-- tests/test_api.py | 2 +- tests/test_aws.py | 2 +- tests/test_etcd.py | 4 +- tests/test_ha.py | 6 +- tests/test_patroni.py | 12 +-- tests/test_postgresql.py | 4 +- tests/test_restore.py | 2 +- tests/test_utils.py | 2 +- tests/test_zookeeper.py | 8 +- 25 files changed, 166 insertions(+), 153 deletions(-) create mode 100644 patroni/__init__.py create mode 100644 patroni/__main__.py rename {helpers => patroni}/api.py (100%) rename {helpers => patroni}/dcs.py (99%) rename {helpers => patroni}/etcd.py (98%) rename {helpers => patroni}/ha.py (99%) rename {helpers => patroni}/postgresql.py (99%) rename {helpers => patroni/scripts}/__init__.py (100%) rename {scripts => patroni/scripts}/aws.py (100%) rename {scripts => patroni/scripts}/restore.py (100%) rename {helpers => patroni}/utils.py (100%) create mode 100644 patroni/version.py rename {helpers => patroni}/zookeeper.py (98%) delete mode 100644 scripts/__init__.py diff --git a/patroni.py b/patroni.py index 3be2aa57..f34ef133 100755 --- a/patroni.py +++ b/patroni.py @@ -1,123 +1,5 @@ #!/usr/bin/env python -import logging -import os -import sys -import time -import yaml - -from helpers.api import RestApiServer -from helpers.etcd import Etcd -from helpers.ha import Ha -from helpers.postgresql import Postgresql -from helpers.utils import setup_signal_handlers, sleep, reap_children -from helpers.zookeeper import ZooKeeper - -logger = logging.getLogger(__name__) - - -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)) - host, port = config['restapi']['listen'].split(':') - self.api = RestApiServer(self, config['restapi']) - self.next_run = time.time() - self.shutdown_member_ttl = 300 - - @staticmethod - def get_dcs(name, config): - if 'etcd' in config: - return Etcd(name, config['etcd']) - if 'zookeeper' in config: - 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 initialize(self): - # wait for etcd to be available - while not self.touch_member(): - logger.info('waiting on DCS') - sleep(5) - - # is data directory empty? - if self.postgresql.data_directory_empty(): - # racing to initialize - if self.ha.dcs.race('/initialize'): - self.postgresql.initialize() - self.ha.dcs.take_leader() - self.postgresql.start() - self.postgresql.create_replication_user() - self.postgresql.create_connection_users() - else: - while True: - leader = self.ha.dcs.current_leader() - if leader and self.postgresql.sync_from_leader(leader): - self.postgresql.write_recovery_conf(leader) - self.postgresql.start() - break - sleep(5) - elif self.postgresql.is_running(): - self.postgresql.load_replication_slots() - - 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 - else: - self.ha.dcs.sleep(nap_time) - - def run(self): - self.api.start() - self.next_run = time.time() - - while True: - self.touch_member() - logger.info(self.ha.run_cycle()) - try: - if self.ha.state_handler.is_leader(): - self.ha.cluster and self.ha.state_handler.create_replication_slots(self.ha.cluster) - else: - self.ha.state_handler.drop_replication_slots() - except: - logger.exception('Exception when changing replication slots') - reap_children() - self.schedule_next_run() - - -def main(): - logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO) - logging.getLogger('requests').setLevel(logging.WARNING) - setup_signal_handlers() - - if len(sys.argv) < 2 or not os.path.isfile(sys.argv[1]): - print('Usage: {} config.yml'.format(sys.argv[0])) - return - - with open(sys.argv[1], 'r') as f: - config = yaml.load(f) - - patroni = Patroni(config) - try: - patroni.initialize() - patroni.run() - except KeyboardInterrupt: - pass - finally: - patroni.touch_member(patroni.shutdown_member_ttl) # schedule member removal - patroni.postgresql.stop() - patroni.ha.dcs.delete_leader() +from patroni import main if __name__ == '__main__': diff --git a/patroni/__init__.py b/patroni/__init__.py new file mode 100644 index 00000000..572f4263 --- /dev/null +++ b/patroni/__init__.py @@ -0,0 +1,119 @@ +import logging +import os +import sys +import time +import yaml + +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.zookeeper import ZooKeeper + +logger = logging.getLogger(__name__) + + +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)) + host, port = config['restapi']['listen'].split(':') + self.api = RestApiServer(self, config['restapi']) + self.next_run = time.time() + self.shutdown_member_ttl = 300 + + @staticmethod + def get_dcs(name, config): + if 'etcd' in config: + return Etcd(name, config['etcd']) + if 'zookeeper' in config: + 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 initialize(self): + # wait for etcd to be available + while not self.touch_member(): + logger.info('waiting on DCS') + sleep(5) + + # is data directory empty? + if self.postgresql.data_directory_empty(): + # racing to initialize + if self.ha.dcs.race('/initialize'): + self.postgresql.initialize() + self.ha.dcs.take_leader() + self.postgresql.start() + self.postgresql.create_replication_user() + self.postgresql.create_connection_users() + else: + while True: + leader = self.ha.dcs.current_leader() + if leader and self.postgresql.sync_from_leader(leader): + self.postgresql.write_recovery_conf(leader) + self.postgresql.start() + break + sleep(5) + elif self.postgresql.is_running(): + self.postgresql.load_replication_slots() + + 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 + else: + self.ha.dcs.sleep(nap_time) + + def run(self): + self.api.start() + self.next_run = time.time() + + while True: + self.touch_member() + logger.info(self.ha.run_cycle()) + try: + if self.ha.state_handler.is_leader(): + self.ha.cluster and self.ha.state_handler.create_replication_slots(self.ha.cluster) + else: + self.ha.state_handler.drop_replication_slots() + except: + logger.exception('Exception when changing replication slots') + reap_children() + self.schedule_next_run() + + +def main(): + logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO) + logging.getLogger('requests').setLevel(logging.WARNING) + setup_signal_handlers() + + if len(sys.argv) < 2 or not os.path.isfile(sys.argv[1]): + print('Usage: {} config.yml'.format(sys.argv[0])) + return + + with open(sys.argv[1], 'r') as f: + config = yaml.load(f) + + patroni = Patroni(config) + try: + patroni.initialize() + patroni.run() + except KeyboardInterrupt: + pass + finally: + patroni.touch_member(patroni.shutdown_member_ttl) # schedule member removal + patroni.postgresql.stop() + patroni.ha.dcs.delete_leader() diff --git a/patroni/__main__.py b/patroni/__main__.py new file mode 100644 index 00000000..3abcbfc3 --- /dev/null +++ b/patroni/__main__.py @@ -0,0 +1,5 @@ +from patroni import main + + +if __name__ == '__main__': + main() diff --git a/helpers/api.py b/patroni/api.py similarity index 100% rename from helpers/api.py rename to patroni/api.py diff --git a/helpers/dcs.py b/patroni/dcs.py similarity index 99% rename from helpers/dcs.py rename to patroni/dcs.py index c7140c22..f5eb4d2c 100644 --- a/helpers/dcs.py +++ b/patroni/dcs.py @@ -1,7 +1,7 @@ import abc from collections import namedtuple -from helpers.utils import calculate_ttl, sleep +from patroni.utils import calculate_ttl, sleep from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl diff --git a/helpers/etcd.py b/patroni/etcd.py similarity index 98% rename from helpers/etcd.py rename to patroni/etcd.py index add736c5..0ebb99f4 100644 --- a/helpers/etcd.py +++ b/patroni/etcd.py @@ -8,8 +8,8 @@ import socket from dns.exception import DNSException from dns import resolver -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string -from helpers.utils import sleep +from patroni.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from patroni.utils import sleep from requests.exceptions import RequestException logger = logging.getLogger(__name__) diff --git a/helpers/ha.py b/patroni/ha.py similarity index 99% rename from helpers/ha.py rename to patroni/ha.py index 8e283c55..f31b2b26 100644 --- a/helpers/ha.py +++ b/patroni/ha.py @@ -1,6 +1,6 @@ import logging -from helpers.dcs import DCSError +from patroni.dcs import DCSError from psycopg2 import InterfaceError, OperationalError logger = logging.getLogger(__name__) diff --git a/helpers/postgresql.py b/patroni/postgresql.py similarity index 99% rename from helpers/postgresql.py rename to patroni/postgresql.py index 3fc7983a..2c86800d 100644 --- a/helpers/postgresql.py +++ b/patroni/postgresql.py @@ -5,7 +5,7 @@ import shlex import shutil import subprocess -from helpers.utils import sleep +from patroni.utils import sleep from six.moves.urllib_parse import urlparse logger = logging.getLogger(__name__) diff --git a/helpers/__init__.py b/patroni/scripts/__init__.py similarity index 100% rename from helpers/__init__.py rename to patroni/scripts/__init__.py diff --git a/scripts/aws.py b/patroni/scripts/aws.py similarity index 100% rename from scripts/aws.py rename to patroni/scripts/aws.py diff --git a/scripts/restore.py b/patroni/scripts/restore.py similarity index 100% rename from scripts/restore.py rename to patroni/scripts/restore.py diff --git a/helpers/utils.py b/patroni/utils.py similarity index 100% rename from helpers/utils.py rename to patroni/utils.py diff --git a/patroni/version.py b/patroni/version.py new file mode 100644 index 00000000..11d27f8c --- /dev/null +++ b/patroni/version.py @@ -0,0 +1 @@ +__version__ = '0.1' diff --git a/helpers/zookeeper.py b/patroni/zookeeper.py similarity index 98% rename from helpers/zookeeper.py rename to patroni/zookeeper.py index cb2918cd..bbc05002 100644 --- a/helpers/zookeeper.py +++ b/patroni/zookeeper.py @@ -3,10 +3,10 @@ import random import requests import time -from helpers.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string -from helpers.utils import sleep from kazoo.client import KazooClient, KazooState from kazoo.exceptions import NoNodeError, NodeExistsError +from patroni.dcs import AbstractDCS, Cluster, DCSError, Member, parse_connection_string +from patroni.utils import sleep from requests.exceptions import RequestException logger = logging.getLogger(__name__) diff --git a/scripts/__init__.py b/scripts/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/setup.py b/setup.py index 62d05a69..4ed8ca37 100644 --- a/setup.py +++ b/setup.py @@ -19,13 +19,20 @@ if sys.version_info < (2, 7, 0): __location__ = os.path.join(os.getcwd(), os.path.dirname(inspect.getfile(inspect.currentframe()))) +def read_version(package): + data = {} + with open(os.path.join(package, 'version.py'), 'r') as fd: + exec(fd.read(), data) + return data['__version__'] + + NAME = 'patroni' -MAIN_PACKAGE = 'patroni.py' -HELPERS = 'helpers' +MAIN_PACKAGE = NAME SCRIPTS = 'scripts' +VERSION = read_version(MAIN_PACKAGE) VERSION = '0.1' DESCRIPTION = 'A Template for PostgreSQL HA with etcd' -LICENSE = 'The MIT License' +LICENSE = 'MIT License' COVERAGE_XML = True COVERAGE_HTML = False @@ -62,8 +69,7 @@ class PyTest(TestCommand): def finalize_options(self): TestCommand.finalize_options(self) if self.cov_xml or self.cov_html: - self.cov = ['--cov', MAIN_PACKAGE, '--cov', HELPERS, '--cov', SCRIPTS, '--cov-report', - 'term-missing'] + self.cov = ['--cov', MAIN_PACKAGE, '--cov', MAIN_PACKAGE, '--cov-report', 'term-missing'] if self.cov_xml: self.cov.extend(['--cov-report', 'xml']) if self.cov_html: @@ -82,7 +88,7 @@ class PyTest(TestCommand): params['plugins'] = ['cov'] if self.junitxml: params['args'] += self.junitxml - params['args'] += ['--doctest-modules', HELPERS, '--doctest-modules', SCRIPTS, '-s'] + params['args'] += ['--doctest-modules', MAIN_PACKAGE, '-s', '-vv'] errno = pytest.main(**params) sys.exit(errno) diff --git a/tests/test_api.py b/tests/test_api.py index 91b36943..aecf3df6 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,7 +1,7 @@ import psycopg2 import unittest -from helpers.api import RestApiHandler, RestApiServer +from patroni.api import RestApiHandler, RestApiServer from six import BytesIO as IO from test_postgresql import psycopg2_connect diff --git a/tests/test_aws.py b/tests/test_aws.py index 84d495fe..09c357f4 100644 --- a/tests/test_aws.py +++ b/tests/test_aws.py @@ -2,7 +2,7 @@ import unittest import requests import boto.ec2 from collections import namedtuple -from scripts.aws import AWSConnection +from patroni.scripts.aws import AWSConnection from requests.exceptions import RequestException diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 38692c52..bc88e6a7 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -8,9 +8,9 @@ import time import unittest from dns.exception import DNSException -from helpers.dcs import Cluster, DCSError, Member -from helpers.etcd import Client, Etcd from mock import Mock, patch +from patroni.dcs import Cluster, DCSError, Member +from patroni.etcd import Client, Etcd class MockResponse: diff --git a/tests/test_ha.py b/tests/test_ha.py index abfc4ac8..46aa9a0a 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1,9 +1,9 @@ import unittest -from helpers.dcs import Cluster, DCSError -from helpers.etcd import Client, Etcd -from helpers.ha import Ha from mock import Mock, patch +from patroni.dcs import Cluster, DCSError +from patroni.etcd import Client, Etcd +from patroni.ha import Ha from test_etcd import etcd_read, etcd_write diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 67ac1ffd..979e603c 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -1,5 +1,5 @@ import datetime -import helpers.zookeeper +import patroni.zookeeper import psycopg2 import subprocess import sys @@ -7,12 +7,12 @@ import time import unittest import yaml -from helpers.api import RestApiServer -from helpers.dcs import Cluster, Member -from helpers.etcd import Etcd -from helpers.zookeeper import ZooKeeper from mock import Mock, patch +from patroni.api import RestApiServer +from patroni.dcs import Cluster, Member +from patroni.etcd import Etcd from patroni import Patroni, main +from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer from test_etcd import Client, etcd_read, etcd_write from test_ha import true, false @@ -70,7 +70,7 @@ class TestPatroni(unittest.TestCase): Postgresql.write_recovery_conf = self.write_recovery_conf def test_get_dcs(self): - helpers.zookeeper.KazooClient = MockKazooClient + patroni.zookeeper.KazooClient = MockKazooClient self.assertIsInstance(self.p.get_dcs('', {'zookeeper': {'scope': '', 'hosts': ''}}), ZooKeeper) self.assertRaises(Exception, self.p.get_dcs, '', {}) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a63c240b..23fd9d74 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -4,8 +4,8 @@ import shutil import subprocess import unittest -from helpers.dcs import Cluster, Member -from helpers.postgresql import Postgresql +from patroni.dcs import Cluster, Member +from patroni.postgresql import Postgresql def nop(*args, **kwargs): diff --git a/tests/test_restore.py b/tests/test_restore.py index 38b34c6a..2ffd8a58 100644 --- a/tests/test_restore.py +++ b/tests/test_restore.py @@ -1,7 +1,7 @@ import unittest from mock import MagicMock, patch import os -from scripts.restore import Restore, WALERestore +from patroni.scripts.restore import Restore, WALERestore def fake_cursor_fetchone(*args, **kwargs): diff --git a/tests/test_utils.py b/tests/test_utils.py index 312277b6..9a383ca0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,7 +2,7 @@ import os import time import unittest -from helpers.utils import reap_children, sigchld_handler, sigterm_handler, sleep +from patroni.utils import reap_children, sigchld_handler, sigterm_handler, sleep def nop(*args, **kwargs): diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 53f5ab82..bf06fd19 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -1,8 +1,8 @@ -import helpers.zookeeper +import patroni.zookeeper import requests import unittest -from helpers.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError +from patroni.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError from kazoo.client import KazooState from kazoo.exceptions import NoNodeError, NodeExistsError from kazoo.protocol.states import ZnodeStat @@ -105,7 +105,7 @@ class TestExhibitorEnsembleProvider(unittest.TestCase): def set_up(self): requests.get = requests_get - helpers.zookeeper.sleep = exhibitor_sleep + patroni.zookeeper.sleep = exhibitor_sleep def test_init(self): self.assertRaises(Exception, ExhibitorEnsembleProvider, ['localhost'], 8181) @@ -119,7 +119,7 @@ class TestZooKeeper(unittest.TestCase): def set_up(self): requests.get = requests_get - helpers.zookeeper.KazooClient = MockKazooClient + patroni.zookeeper.KazooClient = MockKazooClient self.zk = ZooKeeper('foo', {'exhibitor': {'hosts': ['localhost', 'exhibitor'], 'port': 8181}, 'scope': 'test'}) def test_session_listener(self): From 7cce02ae958a499aca707c87131db051710bb269 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Sat, 5 Sep 2015 15:19:17 +0200 Subject: [PATCH 37/66] Always use pg_xlog_location_diff to calculate bytes written to xlog Drop unused functionality (lsn_to_bytes, bytes_to_lsn) Revert some changes. --- helpers/postgresql.py | 47 +++++++--------------------------------- tests/test_postgresql.py | 18 ++++++--------- 2 files changed, 15 insertions(+), 50 deletions(-) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index 3fc7983a..e451d844 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -35,7 +35,6 @@ def parseurl(url): class Postgresql: - _SERVER_VERSION = 90400 def __init__(self, config): self.config = config @@ -82,7 +81,6 @@ class Postgresql: r = parseurl('postgres://{}/postgres'.format(self.local_address)) self._connection = psycopg2.connect(**r) self._connection.autocommit = True - Postgresql._SERVER_VERSION = self._connection.server_version return self._connection def _cursor(self): @@ -254,7 +252,9 @@ class Postgresql: member_conn = psycopg2.connect(**r) member_conn.autocommit = True member_cursor = member_conn.cursor() - member_cursor.execute("SELECT pg_is_in_recovery(), COALESCE(pg_last_xlog_replay_location(), '0/0')") + 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() @@ -262,7 +262,7 @@ class Postgresql: if not row[0]: logger.warning('Master (%s) is still alive', member.name) return False - if self.xlog_position() < self.lsn_to_bytes(row[1], member_conn.server_version): + if row[1] < 0: return False except psycopg2.Error: continue @@ -357,10 +357,10 @@ recovery_target_timeline = 'latest' self.admin['username']), self.admin['password']) def xlog_position(self): - lsn = self.query("""SELECT CASE WHEN pg_is_in_recovery() - THEN pg_last_xlog_replay_location() - ELSE pg_current_xlog_location() END""").fetchone()[0] - return self.lsn_to_bytes(lsn) + 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] def load_replication_slots(self): if self.use_slots: @@ -391,34 +391,3 @@ recovery_target_timeline = 'latest' def last_operation(self): return str(self.xlog_position()) - - @staticmethod - def lsn_to_bytes(value, version=None): - """ - >>> Postgresql.lsn_to_bytes('1/66000060') - 6006243424 - >>> Postgresql.lsn_to_bytes('j/66000060') - 0 - """ - if version is None: - version = Postgresql._SERVER_VERSION - try: - multiplier = 0xFF000000 if version < 90300 else 0x100000000 - e = value.split('/') - if len(e) == 2 and len(e[0]) > 0 and len(e[1]) > 0: - return int(e[0], 16) * multiplier + int(e[1], 16) - except ValueError: - return 0 - - @staticmethod - def bytes_to_lsn(value, version=None): - """ - >>> Postgresql.bytes_to_lsn(6006243424) - '1/66000060' - """ - if version is None: - version = Postgresql._SERVER_VERSION - divider = 0xFF000000 if version < 90300 else 0x100000000 - segment = value / divider - offset = value % divider - return '%X/%X' % (segment, offset) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a63c240b..ca33f3e5 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -21,14 +21,12 @@ def false(*args, **kwargs): class MockCursor: - _count = 0 def __init__(self): self.closed = False self.results = [] def execute(self, sql, *params): - MockCursor._count += 1 if sql.startswith('blabla'): raise psycopg2.OperationalError() elif sql.startswith('InterfaceError'): @@ -37,15 +35,15 @@ class MockCursor: self.results = [('blabla',), ('foobar',)] elif sql.startswith('SELECT pg_current_xlog_location()'): self.results = [(0,)] - elif sql.startswith('SELECT pg_is_in_recovery(), COALESCE'): - if MockCursor._count == 1: + elif sql.startswith('SELECT pg_is_in_recovery(), %s'): + if params[0][0] == 1: raise psycopg2.OperationalError() - elif MockCursor._count == 2: - self.results = [(True, '0/1')] + elif params[0][0] == 2: + self.results = [(True, -1)] else: - self.results = [(False, '0/1')] - elif sql.startswith('SELECT CASE WHEN pg_is_in_recovery()'): - self.results = [('0/0', )] + self.results = [(False, 0)] + elif sql.startswith('SELECT pg_xlog_location_diff'): + self.results = [(0,)] elif sql.startswith('SELECT pg_is_in_recovery()'): self.results = [(False, )] elif sql.startswith('SELECT to_char(pg_postmaster_start_time'): @@ -185,9 +183,7 @@ class TestPostgresql(unittest.TestCase): cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leader]) self.assertTrue(self.p.is_healthiest_node(cluster)) self.p.is_leader = false - MockCursor._count = 0 self.assertFalse(self.p.is_healthiest_node(cluster)) - MockCursor._count = 0 self.p.xlog_position = lambda: 1 self.assertTrue(self.p.is_healthiest_node(cluster)) self.p.xlog_position = lambda: 2 From ac2740eeb91813ebd9d470a79d79c165aa5b5078 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Sat, 5 Sep 2015 15:22:38 +0200 Subject: [PATCH 38/66] Drop unused variable server_version --- tests/test_postgresql.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index ca33f3e5..b3c91f73 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -78,7 +78,6 @@ class MockConnect: def __init__(self): self.autocommit = False self.closed = 0 - self.server_version = 90400 def cursor(self): return MockCursor() From 5ea0ab70f558198869bf25f069e64f7c23a21eb8 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 7 Sep 2015 10:37:10 +0200 Subject: [PATCH 39/66] Add patroni.py executable file --- patroni.py | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100755 patroni.py diff --git a/patroni.py b/patroni.py new file mode 100755 index 00000000..f34ef133 --- /dev/null +++ b/patroni.py @@ -0,0 +1,6 @@ +#!/usr/bin/env python +from patroni import main + + +if __name__ == '__main__': + main() From 348e8e80864bb4d0acfe05f64e51fdf665a36dec Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 7 Sep 2015 13:47:05 +0200 Subject: [PATCH 40/66] Reverted pip installation in Dockerfile As the Dockerfile is there mainly to support developers, we want to build the Dockerfile using the current working directory instead of a previously released version. --- docker/Dockerfile => Dockerfile | 12 ++++++------ docker/Dockerfile.test.patch | 13 ------------- docker/entrypoint.sh | 2 +- patroni/version.py | 2 +- release.sh | 15 +++++++++------ 5 files changed, 17 insertions(+), 27 deletions(-) rename docker/Dockerfile => Dockerfile (86%) delete mode 100644 docker/Dockerfile.test.patch diff --git a/docker/Dockerfile b/Dockerfile similarity index 86% rename from docker/Dockerfile rename to Dockerfile index 1a1cb41c..61d85a74 100644 --- a/docker/Dockerfile +++ b/Dockerfile @@ -13,21 +13,21 @@ RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install postgresql-${PGVERSION} -y -RUN apt-get install python python-psycopg2 python-yaml python-requests python-dnspython python-pip python-mock -y -RUN pip install patroni - +RUN apt-get install python python-psycopg2 python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y +RUN pip install python-etcd ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH +ADD patroni.py /patroni.py +ADD patroni/ /patroni + ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl ### Setting up a simple script that will serve as an entrypoint -RUN mkdir /patroni/ RUN mkdir /data/ && touch /var/log/etcd.log /var/log/etcd.err /pgpass /patroni/postgres.yml RUN chown postgres:postgres -R /patroni/ /data/ /pgpass /var/log/etcd.* /patroni/postgres.yml -ADD entrypoint.sh /entrypoint.sh +ADD docker/entrypoint.sh /entrypoint.sh EXPOSE 4001 5432 2380 diff --git a/docker/Dockerfile.test.patch b/docker/Dockerfile.test.patch deleted file mode 100644 index 376f3c20..00000000 --- a/docker/Dockerfile.test.patch +++ /dev/null @@ -1,13 +0,0 @@ ---- Dockerfile 2015-09-03 09:24:16.355412216 +0200 -+++ Dockerfile.test 2015-09-03 09:25:48.407879693 +0200 -@@ -15,7 +15,9 @@ - ENV PGVERSION 9.4 - RUN apt-get install postgresql-${PGVERSION} -y - RUN apt-get install python python-psycopg2 python-yaml python-requests python-dnspython python-pip python-mock -y --RUN pip install patroni -+## We install prereqs from pypi, the package from testpypi -+RUN pip install --force-reinstall --upgrade kazoo boto python-etcd -+RUN pip install -i https://testpypi.python.org/pypi patroni - - - ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index b28f4ae6..f4851a36 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -133,5 +133,5 @@ then sleep 60 done else - exec patroni /patroni/postgres.yml + exec python /patroni.py /patroni/postgres.yml fi diff --git a/patroni/version.py b/patroni/version.py index 11d27f8c..d0cc8295 100644 --- a/patroni/version.py +++ b/patroni/version.py @@ -1 +1 @@ -__version__ = '0.1' +__version__ = '0.45' diff --git a/release.sh b/release.sh index 316ca908..bf6f7482 100755 --- a/release.sh +++ b/release.sh @@ -5,6 +5,9 @@ if [ $# -ne 1 ]; then exit 1 fi +readonly VERSIONFILE="patroni/version.py" + +## Bail out on any non-zero exitcode from the called processes set -xe python3 --version @@ -12,17 +15,17 @@ git --version version=$1 -sed -i "s/__version__ = .*/__version__ = '${version}'/" version.py +sed -i "s/__version__ = .*/__version__ = '${version}'/" "${VERSIONFILE}" python3 setup.py clean python3 setup.py test python3 setup.py flake8 -git add __init__.py +git add "${VERSIONFILE}" -git commit -m "Bumped version to $version" -git push +#git commit -m "Bumped version to $version" +#git push python3 setup.py sdist bdist_wheel upload -git tag ${version} -git push --tags +#git tag ${version} +#git push --tags From cdccebd2d14c07ac50b574f834d82a6ce88432cc Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 7 Sep 2015 13:53:48 +0200 Subject: [PATCH 41/66] Point to api for listing of Docker images for Patroni (Issue #14) --- docker/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docker/README.md b/docker/README.md index d17e77a1..74288a98 100644 --- a/docker/README.md +++ b/docker/README.md @@ -1,6 +1,7 @@ # Patroni Dockerfile -You can run Patroni in a docker container using this Dockerfile, or by using the Docker image at - https://os-registry.stups.zalan.do/acid/patroni-1.0-SNAPSHOT +You can run Patroni in a docker container using this Dockerfile, or by using one of the Docker image at + + https://os-registry.stups.zalan.do/v1/repositories/acid/patroni/tags This Dockerfile is meant in aiding development of Patroni and quick testing of features. It is not a production-worthy Dockerfile From 28f5839ac2fa4c8e730ec4e6a1c93ae0ce86775a Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 7 Sep 2015 14:00:05 +0200 Subject: [PATCH 42/66] Make git add part of the release.sh again. --- release.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/release.sh b/release.sh index bf6f7482..e219603c 100755 --- a/release.sh +++ b/release.sh @@ -22,10 +22,10 @@ python3 setup.py flake8 git add "${VERSIONFILE}" -#git commit -m "Bumped version to $version" -#git push +git commit -m "Bumped version to $version" +git push python3 setup.py sdist bdist_wheel upload -#git tag ${version} -#git push --tags +git tag ${version} +git push --tags From 496f91fea077a450754dd049faf77c3d5287044f Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 8 Sep 2015 11:59:08 +0200 Subject: [PATCH 43/66] Update README.rst Formatting issue with the header --- README.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 0798ad00..3e19f0a2 100644 --- a/README.rst +++ b/README.rst @@ -1,8 +1,7 @@ |Build Status| |Coverage Status| -=== Patroni: A Template for PostgreSQL HA with ZooKeeper or etcd -=== +------------------------------------------------------------ Patroni was previously known as Governor. From b842ed478b6bf03cddf4da80385dedbd25104da7 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 8 Sep 2015 12:03:34 +0200 Subject: [PATCH 44/66] Make sure initialize flag is reset on failure. Cleanup the initialize flag if the initializing node fails to bootstrap its PostgreSQL database. Rename dcs.race to initialize, since we only call it for the initialize flag. Factored out PostgreSQL bootstrapping code into a separate function. --- helpers/dcs.py | 7 ++++++- helpers/etcd.py | 8 ++++++-- helpers/postgresql.py | 24 ++++++++++++++++++++++++ helpers/zookeeper.py | 9 +++++++-- patroni.py | 21 +++++++++++++-------- tests/test_etcd.py | 4 ++-- tests/test_patroni.py | 23 ++++++++++++++++++++--- tests/test_zookeeper.py | 2 +- 8 files changed, 79 insertions(+), 19 deletions(-) diff --git a/helpers/dcs.py b/helpers/dcs.py index c7140c22..fa812a08 100644 --- a/helpers/dcs.py +++ b/helpers/dcs.py @@ -66,6 +66,7 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem class AbstractDCS: __metaclass__ = abc.ABCMeta + initialize_key = '/initialize' def __init__(self, name, config): """ @@ -131,7 +132,7 @@ class AbstractDCS: overwriting the key if necessary.""" @abc.abstractmethod - def race(self, path): + def initialize(self): """Race for cluster initialization. :param path: usually this is just '/initialize' :returns: `!True` if key has been created successfully. @@ -144,5 +145,9 @@ class AbstractDCS: """Voluntarily remove leader key from DCS This method should remove leader key if current instance is the leader""" + @abc.abstractmethod + def cancel_initialization(self): + """ Removes the initialize key for a cluster """ + def sleep(self, timeout): sleep(timeout) diff --git a/helpers/etcd.py b/helpers/etcd.py index add736c5..93be5a05 100644 --- a/helpers/etcd.py +++ b/helpers/etcd.py @@ -206,9 +206,13 @@ class Etcd(AbstractDCS): return ret @catch_etcd_errors - def race(self, path): - return self.client.write(self.client_path(path), self._name, prevExist=False) + def initialize(self): + return self.client.write(self.client_path(self.initialize_key), self._name, prevExist=False) @catch_etcd_errors def delete_leader(self): return self.client.delete(self.client_path('/leader'), prevValue=self._name) + + @catch_etcd_errors + def cancel_initialization(self): + return self.client.delete(self.client_path(self.initialize_key), prevValue=self._name) diff --git a/helpers/postgresql.py b/helpers/postgresql.py index e451d844..4e425dec 100644 --- a/helpers/postgresql.py +++ b/helpers/postgresql.py @@ -391,3 +391,27 @@ recovery_target_timeline = 'latest' def last_operation(self): return str(self.xlog_position()) + + def bootstrap(self, current_leader=None): + """ + Initially bootstrap PostgreSQL, either by creating a data + directory with initdb, or by initalizing a replica from an + exiting leader. Failure in the first case always leads to + exception, since there is no point in continuing if initdb failed. + In the second case, however, a False is returned on failure, since + it is normal for the replica to retry a failed attempt to initialize + from the master. + """ + ret = False + if not current_leader: + ret = self.initialize() and self.start() + if ret: + self.create_replication_user() + self.create_connection_users() + else: + raise Exception("Could not bootstrap master PostgreSQL") + else: + if self.sync_from_leader(current_leader): + self.write_recovery_conf(current_leader) + ret = self.start() + return ret diff --git a/helpers/zookeeper.py b/helpers/zookeeper.py index cb2918cd..98d68ad1 100644 --- a/helpers/zookeeper.py +++ b/helpers/zookeeper.py @@ -180,8 +180,8 @@ class ZooKeeper(AbstractDCS): ret or logger.info('Could not take out TTL lock') return ret - def race(self, path): - return self._create(path, self._name, makepath=True) + def initialize(self): + return self._create(self.initialize_key, self._name, makepath=True) def touch_member(self, connection_string, ttl=None): for m in self.members: @@ -223,6 +223,11 @@ class ZooKeeper(AbstractDCS): if isinstance(self.leader, Member) and self.leader.name == self._name: self.client.delete(self.client_path('/leader')) + def cancel_initialization(self): + node = self.get_node(self.initialize_key) + if node and node == self._name: + self.client.delete(self.client_path(self.initialize_key)) + def sleep(self, timeout): self.cluster_event.wait(timeout) if self.cluster_event.isSet(): diff --git a/patroni.py b/patroni.py index 3be2aa57..64430318 100755 --- a/patroni.py +++ b/patroni.py @@ -43,6 +43,11 @@ class Patroni: return True return self.ha.dcs.touch_member(connection_string, ttl) + def cleanup_on_failed_initialization(self): + """ cleanup the DCS if initialization was not successfull """ + logger.info("removing initialize key after failed attempt to initialize the cluster") + self.ha.dcs.cancel_initialization() + def initialize(self): # wait for etcd to be available while not self.touch_member(): @@ -52,18 +57,18 @@ class Patroni: # is data directory empty? if self.postgresql.data_directory_empty(): # racing to initialize - if self.ha.dcs.race('/initialize'): - self.postgresql.initialize() + if self.ha.dcs.initialize(): + try: + self.postgresql.bootstrap() + except: + # bail out and clean the initialize flag. + self.cleanup_on_failed_initialization() + raise self.ha.dcs.take_leader() - self.postgresql.start() - self.postgresql.create_replication_user() - self.postgresql.create_connection_users() else: while True: leader = self.ha.dcs.current_leader() - if leader and self.postgresql.sync_from_leader(leader): - self.postgresql.write_recovery_conf(leader) - self.postgresql.start() + if leader and self.postgresql.bootstrap(leader): break sleep(5) elif self.postgresql.is_running(): diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 38692c52..fa534d15 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -227,8 +227,8 @@ class TestEtcd(unittest.TestCase): def test_update_leader(self): self.assertTrue(self.etcd.update_leader(MockPostgresql())) - def test_race(self): - self.assertFalse(self.etcd.race('')) + def test_initialize(self): + self.assertFalse(self.etcd.initialize()) def test_delete_leader(self): self.etcd.client.delete = etcd_delete diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 67ac1ffd..eb41591c 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -46,6 +46,7 @@ class TestPatroni(unittest.TestCase): def set_up(self): self.touched = False + self.init_cancelled = False subprocess.call = subprocess_call psycopg2.connect = psycopg2_connect self.time_sleep = time.sleep @@ -121,14 +122,15 @@ class TestPatroni(unittest.TestCase): self.p.touch_member() def test_patroni_initialize(self): - self.p.postgresql.should_use_s3_to_create_replica = false self.p.ha.dcs.client.write = etcd_write self.p.touch_member = self.touch_member self.p.postgresql.data_directory_empty = true - self.p.ha.dcs.race = true + self.p.ha.dcs.initialize = true + self.p.postgresql.initialize = true + self.p.postgresql.start = true self.p.initialize() - self.p.ha.dcs.race = false + self.p.ha.dcs.initialize = false time.sleep = time_sleep self.p.ha.dcs.client.read = etcd_read self.p.initialize() @@ -142,3 +144,18 @@ class TestPatroni(unittest.TestCase): def test_schedule_next_run(self): self.p.next_run = time.time() - self.p.nap_time - 1 self.p.schedule_next_run() + + def cancel_initialization(self): + self.init_cancelled = True + + def test_cleanup_on_initialization(self): + self.p.ha.dcs.client.write = etcd_write + self.p.touch_member = self.touch_member + self.p.postgresql.data_directory_empty = true + self.p.ha.dcs.initialize = true + self.p.postgresql.initialize = true + self.p.postgresql.start = false + + self.p.ha.dcs.cancel_initialization = self.cancel_initialization + self.assertRaises(Exception, self.p.initialize) + self.assertTrue(self.init_cancelled) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 53f5ab82..28b337a9 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -141,7 +141,7 @@ class TestZooKeeper(unittest.TestCase): self.zk.delete_leader() def test_race(self): - self.assertFalse(self.zk.race('/initialize')) + self.assertFalse(self.zk.initialize()) def test_touch_member(self): self.zk.touch_member('new') From fa22d91e053f301498d9d09a950558758bf9b40f Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 8 Sep 2015 12:41:48 +0200 Subject: [PATCH 45/66] Move some basic methods implementation into parent exception class --- patroni/exceptions.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 5c13f3e0..e6159d47 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,8 +1,4 @@ class PatroniException(Exception): - pass - - -class DCSError(PatroniException): """Parent class for all kind of exceptions related to selected distributed configuration store""" @@ -15,3 +11,7 @@ class DCSError(PatroniException): "'foo'" """ return repr(self.value) + + +class DCSError(PatroniException): + pass From 61d1d5a098112e72aa1c1cfb657673b98428a12e Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 8 Sep 2015 12:42:12 +0200 Subject: [PATCH 46/66] Set current version to 0.2 --- patroni/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/version.py b/patroni/version.py index d0cc8295..b650ceb0 100644 --- a/patroni/version.py +++ b/patroni/version.py @@ -1 +1 @@ -__version__ = '0.45' +__version__ = '0.2' From 763e4db94915c44bbc646961b16647501db18a4d Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 8 Sep 2015 13:09:58 +0200 Subject: [PATCH 47/66] Bumped version to 0.20 --- patroni/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/version.py b/patroni/version.py index b650ceb0..606ae0aa 100644 --- a/patroni/version.py +++ b/patroni/version.py @@ -1 +1 @@ -__version__ = '0.2' +__version__ = '0.20' From 4d334061b05bb584553eb719cc22c88f00c28334 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 8 Sep 2015 13:10:19 +0200 Subject: [PATCH 48/66] Bumped version to 0.2 --- patroni/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/version.py b/patroni/version.py index 606ae0aa..b650ceb0 100644 --- a/patroni/version.py +++ b/patroni/version.py @@ -1 +1 @@ -__version__ = '0.20' +__version__ = '0.2' From dd8472f6393e2181c23773883e52f00babb526cd Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 8 Sep 2015 13:17:36 +0200 Subject: [PATCH 49/66] Tag on github is prefixed with v. --- release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release.sh b/release.sh index e219603c..d61ae054 100755 --- a/release.sh +++ b/release.sh @@ -27,5 +27,5 @@ git push python3 setup.py sdist bdist_wheel upload -git tag ${version} +git tag v${version} git push --tags From ff499604f0cfc5ce1b9e581ce3838ef943d78860 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 8 Sep 2015 16:04:54 +0200 Subject: [PATCH 50/66] Act on removal of initialization flag. If initializer node suddenly dies before the initialization is complete, other nodes should try to take over. Fix some unittests for etcd and zookeeper and add couple of new ones. --- patroni/__init__.py | 23 ++++++++++++----------- tests/test_etcd.py | 4 ++++ tests/test_zookeeper.py | 9 ++++++++- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 46c030b6..b55c0dfc 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -55,17 +55,18 @@ class Patroni: # is data directory empty? if self.postgresql.data_directory_empty(): - # racing to initialize - if self.ha.dcs.initialize(): - try: - self.postgresql.bootstrap() - except: - # bail out and clean the initialize flag. - self.cleanup_on_failed_initialization() - raise - self.ha.dcs.take_leader() - else: - while True: + while True: + # racing to initialize + if self.ha.dcs.initialize(): + try: + self.postgresql.bootstrap() + except: + # bail out and clean the initialize flag. + self.cleanup_on_failed_initialization() + raise + self.ha.dcs.take_leader() + break + else: leader = self.ha.dcs.current_leader() if leader and self.postgresql.bootstrap(leader): break diff --git a/tests/test_etcd.py b/tests/test_etcd.py index b5c014f4..dbcd4b08 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -269,6 +269,10 @@ class TestEtcd(unittest.TestCase): def test_initialize(self): self.assertFalse(self.etcd.initialize()) + def test_cancel_initializion(self): + self.etcd.client.delete = etcd_delete + self.assertFalse(self.etcd.cancel_initialization()) + def test_delete_leader(self): self.etcd.client.delete = etcd_delete self.assertFalse(self.etcd.delete_leader()) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index d138e4b2..83c97b7f 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -71,6 +71,8 @@ class MockKazooClient: if self.leader: return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, -1, 0, 0, 0)) return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) + elif path.endswith(patroni.zookeeper.ZooKeeper.initialize_key): + return 'foo' def get_children(self, path, watch=None, include_data=False): return ['foo', 'bar', 'buzz'] @@ -93,6 +95,8 @@ class MockKazooClient: return self.leader = True raise Exception + elif path.endswith(patroni.zookeeper.ZooKeeper.initialize_key): + raise Exception def set_hosts(self, hosts, randomize_hosts=None): pass @@ -146,9 +150,12 @@ class TestZooKeeper(unittest.TestCase): self.zk.touch_member('foo') self.zk.delete_leader() - def test_race(self): + def test_initialize(self): self.assertFalse(self.zk.initialize()) + def test_cancel_initialization(self): + self.assertRaises(Exception, self.zk.cancel_initialization) + def test_touch_member(self): self.zk.touch_member('new') self.zk.touch_member('exists') From 1c61280d70efe523c44e628e191c522a2ea54ad1 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 8 Sep 2015 16:10:42 +0200 Subject: [PATCH 51/66] Fix path to scripts subdirectory in configuration files. --- postgres0.yml | 2 +- postgres1.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/postgres0.yml b/postgres0.yml index ce90da14..659a4db2 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -47,7 +47,7 @@ postgresql: env_dir: /home/postgres/etc/wal-e.d/env threshold_megabytes: 10240 threshold_backup_size_percentage: 30 - restore: scripts/restore.py + restore: patroni/scripts/restore.py #recovery_conf: #restore_command: cp ../wal_archive/%f %p parameters: diff --git a/postgres1.yml b/postgres1.yml index 763444e8..bc8b6fd1 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -49,7 +49,7 @@ postgresql: env_dir: /home/postgres/etc/wal-e.d/env threshold_megabytes: 10240 threshold_backup_size_percentage: 30 - restore: scripts/restore.py + restore: patroni/scripts/restore.py parameters: archive_mode: "on" wal_level: hot_standby From 5bdb18761b19a9d041c3e4d968f9407d55f994e1 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 9 Sep 2015 15:10:45 +0200 Subject: [PATCH 52/66] Define initialize, leader, optime and members string constansts in AbstractDCS Also define following properties: * initialize_path * members_path * member_path * leader_path * leader_optime_path And replace any occurrences of these strings or client_path calls in a etcd and zookeeper implementations with given constants and properties. --- patroni/dcs.py | 28 +++++++++++++++++++++++++++- patroni/etcd.py | 24 +++++++++++------------- patroni/zookeeper.py | 26 +++++++++++++------------- tests/test_etcd.py | 4 ++-- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index 6fb7aea8..a33dea29 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -74,6 +74,12 @@ class AbstractDCS: __metaclass__ = abc.ABCMeta + _INITIALIZE = 'initialize' + _LEADER = 'leader' + _MEMBERS = 'members/' + _OPTIME = 'optime' + _LEADER_OPTIME = _OPTIME + '/' + _LEADER + def __init__(self, name, config): """ :param name: name of current instance (the same value as `~Postgresql.name`) @@ -85,7 +91,27 @@ class AbstractDCS: self._base_path = '/service/' + self._scope def client_path(self, path): - return self._base_path + path + return '/'.join([self._base_path, path.lstrip('/')]) + + @property + def initialize_path(self): + return self.client_path(self._INITIALIZE) + + @property + def members_path(self): + return self.client_path(self._MEMBERS) + + @property + def member_path(self): + return self.client_path(self._MEMBERS + self._name) + + @property + def leader_path(self): + return self.client_path(self._LEADER) + + @property + def leader_optime_path(self): + return self.client_path(self._LEADER_OPTIME) @abc.abstractmethod def get_cluster(self): diff --git a/patroni/etcd.py b/patroni/etcd.py index 97bd2cf7..7a527f6e 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -179,17 +179,17 @@ class Etcd(AbstractDCS): nodes = {os.path.relpath(node.key, result.key): node for node in result.leaves} # get initialize flag - initialize = bool(nodes.get('initialize', False)) + initialize = bool(nodes.get(self._INITIALIZE, False)) # get last leader operation - last_leader_operation = nodes.get('optime/leader', None) + last_leader_operation = nodes.get(self._LEADER_OPTIME, None) last_leader_operation = 0 if last_leader_operation is None else int(last_leader_operation.value) # get list of members - members = [self.member(n) for k, n in nodes.items() if k.startswith('members/') and len(k.split('/')) == 2] + members = [self.member(n) for k, n in nodes.items() if k.startswith(self._MEMBERS) and k.count('/') == 1] # get leader - leader = nodes.get('leader', None) + leader = nodes.get(self._LEADER, None) if leader: member = Member(-1, leader.value, None, None, None, None) member = ([m for m in members if m.name == leader.value] or [member])[0] @@ -206,17 +206,15 @@ class Etcd(AbstractDCS): @catch_etcd_errors def touch_member(self, connection_string, ttl=None): - return self.retry(self.client.set, self.client_path('/members/' + self._name), - connection_string, ttl or self.member_ttl) + return self.retry(self.client.set, self.member_path, connection_string, ttl or self.member_ttl) @catch_etcd_errors def take_leader(self): - return self.retry(self.client.set, self.client_path('/leader'), self._name, self.ttl) + return self.retry(self.client.set, self.leader_path, self._name, self.ttl) def attempt_to_acquire_leader(self): try: - return not self.retry(self.client.write, self.client_path('/leader'), - self._name, ttl=self.ttl, prevExist=False) is None + return bool(self.retry(self.client.write, self.leader_path, self._name, ttl=self.ttl, prevExist=False)) except etcd.EtcdAlreadyExist: logger.info('Could not take out TTL lock') except (RetryFailedError, etcd.EtcdException): @@ -225,11 +223,11 @@ class Etcd(AbstractDCS): @catch_etcd_errors def write_leader_optime(self, state_handler): - return self.client.set(self.client_path('/optime/leader'), state_handler.last_operation()) + return self.client.set(self.leader_optime_path, state_handler.last_operation()) @catch_etcd_errors def update_leader(self, state_handler): - ret = self.retry(self.client.test_and_set, self.client_path('/leader'), self._name, self._name, self.ttl) + 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 @@ -239,7 +237,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def delete_leader(self): - return self.client.delete(self.client_path('/leader'), prevValue=self._name) + return self.client.delete(self.leader_path, prevValue=self._name) def watch(self, timeout): # watch on leader key changes if it is defined and current node is not lock owner @@ -249,7 +247,7 @@ class Etcd(AbstractDCS): while index and timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect try: - res = self.client.watch(self.client_path('/leader'), index=index + 1, timeout=timeout) + res = self.client.watch(self.leader_path, index=index + 1, timeout=timeout) if res.action not in ['set', 'compareAndSwap'] or res.value != self.cluster.leader.name: return index = res.modifiedIndex diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 29b8c7f1..3901e89d 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -107,9 +107,9 @@ class ZooKeeper(AbstractDCS): self.fetch_cluster = True self.cluster_event.set() - def get_node(self, name, watch=None): + def get_node(self, key, watch=None): try: - return self.client.get(self.client_path(name), watch) + return self.client.get(key, watch) except NoNodeError: pass except: @@ -123,21 +123,21 @@ class ZooKeeper(AbstractDCS): def load_members(self): members = [] - for member in self.client.get_children(self.client_path('/members'), self.cluster_watcher): - data = self.get_node('/members/' + member) + for member in self.client.get_children(self.members_path, self.cluster_watcher): + data = self.get_node(self.member_path) if data is not None: members.append(self.member(member, *data)) return members def _inner_load_cluster(self): self.cluster_event.clear() - leader = self.get_node('/leader', self.cluster_watcher) + leader = self.get_node(self.leader_path, self.cluster_watcher) self.members = self.load_members() 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: logger.info('I am leader but not owner of the session. Removing leader node') - self.client.delete(self.client_path('/leader')) + self.client.delete(self.leader_path) leader = None if leader: @@ -148,7 +148,7 @@ class ZooKeeper(AbstractDCS): self.leader = leader if self.fetch_cluster: - last_leader_operation = self.get_node('/optime/leader') + last_leader_operation = self.get_node(self.leader_optime_path) if last_leader_operation: self.last_leader_operation = int(last_leader_operation[0]) @@ -167,24 +167,24 @@ class ZooKeeper(AbstractDCS): def _create(self, path, value, **kwargs): try: - self.client.retry(self.client.create, self.client_path(path), value, **kwargs) + self.client.retry(self.client.create, path, value, **kwargs) return True except: return False def attempt_to_acquire_leader(self): - ret = self._create('/leader', self._name, makepath=True, ephemeral=True) + ret = self._create(self.leader_path, self._name, makepath=True, ephemeral=True) ret or logger.info('Could not take out TTL lock') return ret def race(self, path): - return self._create(path, self._name, makepath=True) + return self._create(self.client_path(path), self._name, makepath=True) def touch_member(self, connection_string, ttl=None): for m in self.members: if m.name == self._name: return True - path = self.client_path('/members/' + self._name) + path = self.member_path try: self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) return True @@ -204,7 +204,7 @@ class ZooKeeper(AbstractDCS): last_operation = state_handler.last_operation() if last_operation != self.last_leader_operation: self.last_leader_operation = last_operation - path = self.client_path('/optime/leader') + path = self.leader_optime_path try: self.client.retry(self.client.set, path, last_operation) except NoNodeError: @@ -218,7 +218,7 @@ class ZooKeeper(AbstractDCS): def delete_leader(self): if isinstance(self.leader, Leader) and self.leader.name == self._name: - self.client.delete(self.client_path('/leader')) + self.client.delete(self.leader_path) def watch(self, timeout): self.cluster_event.wait(timeout) diff --git a/tests/test_etcd.py b/tests/test_etcd.py index ab5717ca..94b7f807 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -93,9 +93,9 @@ def etcd_delete(key, **kwargs): def etcd_read(key, **kwargs): - if key == '/service/noleader': + if key == '/service/noleader/': raise DCSError('noleader') - elif key == '/service/nocluster': + elif key == '/service/nocluster/': raise etcd.EtcdKeyNotFound response = {"action": "get", "node": {"key": "/service/batman5", "dir": True, "nodes": [ From 36cbd34ffc2ff3caf3db9ea6d27992f670b4f319 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 9 Sep 2015 15:59:02 +0200 Subject: [PATCH 53/66] Fix zookeeper test coverage --- tests/test_zookeeper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index af2115ca..d123932e 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -56,9 +56,9 @@ class MockKazooClient: func(*args, **kwargs) def get(self, path, watch=None): - if path == '/service/test/no_node': + if path == '/no_node': raise NoNodeError - elif path == '/service/test/other_exception': + elif path == '/other_exception': raise Exception() elif '/members/' in path: return ( From 30a9e0f7f5da186fdbc855f8f26d18bc6fde80b9 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 10 Sep 2015 15:34:29 +0200 Subject: [PATCH 54/66] Move PostgreSQL data directory if init had failed. Prevent treating the incompletely-initialized PostgreSQL cluster as a valid on restart by forcefully moving the data directory. I don't want to remove it altogether, since a DBA might decide to analyze the failed PG cluster in order to resolve the init issue. --- patroni/__init__.py | 2 ++ patroni/postgresql.py | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index b55c0dfc..6a4bf6aa 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -46,6 +46,8 @@ class Patroni: """ cleanup the DCS if initialization was not successfull """ logger.info("removing initialize key after failed attempt to initialize the cluster") self.ha.dcs.cancel_initialization() + self.postgresql.stop() + self.postgresql.move_data_directory() def initialize(self): # wait for etcd to be available diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 8754bb65..3c1630a8 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -4,6 +4,7 @@ import psycopg2 import shlex import shutil import subprocess +import time from patroni.utils import sleep from six.moves.urllib_parse import urlparse @@ -159,7 +160,7 @@ class Postgresql: return ret def is_running(self): - return subprocess.call(' '.join(self._pg_ctl) + ' status > /dev/null', shell=True) == 0 + return subprocess.call(' '.join(self._pg_ctl) + ' status > /dev/null 2>&1', shell=True) == 0 def call_nowait(self, cb_name, is_leader=None): """ pick a callback command and call it without waiting for it to finish """ @@ -415,3 +416,9 @@ recovery_target_timeline = 'latest' self.write_recovery_conf(current_leader) ret = self.start() return ret + + def move_data_directory(self): + if os.path.isdir(self.data_dir) and not self.is_running(): + os.rename(self.data_dir, '{0}_{1}'.format(self.data_dir, str(long(time.time())))) + return True + return False From 2377c417e40bbbcc64173a64ab989dc8c894c10b Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 10 Sep 2015 16:05:10 +0200 Subject: [PATCH 55/66] Fix etcd and zookeper interactions with initialize key. Fix unittests as well. --- patroni/dcs.py | 1 - patroni/etcd.py | 4 ++-- patroni/zookeeper.py | 9 ++++++--- tests/test_zookeeper.py | 10 +++++----- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index 0c2bb9f7..bdba4004 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -73,7 +73,6 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem class AbstractDCS: __metaclass__ = abc.ABCMeta - initialize_key = '/initialize' _INITIALIZE = 'initialize' _LEADER = 'leader' diff --git a/patroni/etcd.py b/patroni/etcd.py index d5730ffe..e24fa66b 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -233,7 +233,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def initialize(self): - return self.client.write(self.client_path(self.initialize_key), self._name, prevExist=False) + return self.client.write(self.initialize_path, self._name, prevExist=False) @catch_etcd_errors def delete_leader(self): @@ -241,7 +241,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def cancel_initialization(self): - return self.client.delete(self.client_path(self.initialize_key), prevValue=self._name) + return 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/zookeeper.py b/patroni/zookeeper.py index 7245b1a3..7c01f278 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -4,7 +4,7 @@ import requests import time from kazoo.client import KazooClient, KazooState -from kazoo.exceptions import NoNodeError, NodeExistsError +from kazoo.exceptions import NoNodeError, NodeExistsError, KazooException from patroni.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string from patroni.utils import sleep from requests.exceptions import RequestException @@ -222,8 +222,11 @@ class ZooKeeper(AbstractDCS): def cancel_initialization(self): node = self.get_node(self.initialize_path) - if node and node == self._name: - self.client.delete(self.initialize_path) + if node and node[0] == self._name: + try: + self.client.retry(self.client.delete, self.initialize_path, version=node[1].mzxid) + except KazooException: + logger.exception("Unable to delete initialize key") def watch(self, timeout): self.cluster_event.wait(timeout) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 3da71f02..0a95d0bc 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -71,8 +71,8 @@ class MockKazooClient: if self.leader: return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, -1, 0, 0, 0)) return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) - elif path.endswith(patroni.zookeeper.ZooKeeper.initialize_key): - return 'foo' + elif path.endswith('/initialize'): + return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) def get_children(self, path, watch=None, include_data=False): return ['foo', 'bar', 'buzz'] @@ -95,8 +95,8 @@ class MockKazooClient: return self.leader = True raise Exception - elif path.endswith(patroni.zookeeper.ZooKeeper.initialize_key): - raise Exception + elif path.endswith('/initialize'): + raise NoNodeError def set_hosts(self, hosts, randomize_hosts=None): pass @@ -154,7 +154,7 @@ class TestZooKeeper(unittest.TestCase): self.assertFalse(self.zk.initialize()) def test_cancel_initialization(self): - self.assertRaises(Exception, self.zk.cancel_initialization) + self.zk.cancel_initialization() def test_touch_member(self): self.zk.touch_member('new') From cd312de2527e0d168ff0fad27978120c17919383 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 10 Sep 2015 17:15:43 +0200 Subject: [PATCH 56/66] Fix a flake8 warning. Improve some unit tests by expecting specific exceptions. --- patroni/exceptions.py | 4 ++++ patroni/postgresql.py | 10 ++++++---- tests/test_patroni.py | 5 +++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index e6159d47..507edfc3 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -13,5 +13,9 @@ class PatroniException(Exception): return repr(self.value) +class PostgresException(PatroniException): + pass + + class DCSError(PatroniException): pass diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 3c1630a8..b7f76fc3 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -6,6 +6,7 @@ import shutil import subprocess import time +from patroni.exceptions import PostgresException from patroni.utils import sleep from six.moves.urllib_parse import urlparse @@ -410,7 +411,7 @@ recovery_target_timeline = 'latest' self.create_replication_user() self.create_connection_users() else: - raise Exception("Could not bootstrap master PostgreSQL") + raise PostgresException("Could not bootstrap master PostgreSQL") else: if self.sync_from_leader(current_leader): self.write_recovery_conf(current_leader) @@ -419,6 +420,7 @@ recovery_target_timeline = 'latest' def move_data_directory(self): if os.path.isdir(self.data_dir) and not self.is_running(): - os.rename(self.data_dir, '{0}_{1}'.format(self.data_dir, str(long(time.time())))) - return True - return False + try: + os.rename(self.data_dir, '{0}_{1}'.format(self.data_dir, time.strftime('%Y-%m-%d-%H-%M-%S'))) + except: + logger.exception("Could not rename data directory {0}".format(self.data_dir)) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 75d47fe4..d5740e9c 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -11,6 +11,7 @@ from mock import Mock, patch from patroni.api import RestApiServer from patroni.dcs import Cluster, Member from patroni.etcd import Etcd +from patroni.exceptions import PostgresException from patroni import Patroni, main from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer @@ -141,7 +142,7 @@ class TestPatroni(unittest.TestCase): self.p.initialize() self.p.ha.dcs.current_leader = nop - self.assertRaises(Exception, self.p.initialize) + self.assertRaises(SleepException, self.p.initialize) self.p.postgresql.data_directory_empty = false self.p.initialize() @@ -162,5 +163,5 @@ class TestPatroni(unittest.TestCase): self.p.postgresql.start = false self.p.ha.dcs.cancel_initialization = self.cancel_initialization - self.assertRaises(Exception, self.p.initialize) + self.assertRaises(PostgresException, self.p.initialize) self.assertTrue(self.init_cancelled) From 15cd10669dc632a78c8ff7d27c6c3c25f452df32 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 10 Sep 2015 18:04:47 +0200 Subject: [PATCH 57/66] Change an outdated comment. --- patroni/dcs.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index bdba4004..9017da7f 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -167,10 +167,9 @@ class AbstractDCS: @abc.abstractmethod def initialize(self): """Race for cluster initialization. - :param path: usually this is just '/initialize' :returns: `!True` if key has been created successfully. - this method should create atomically `path` key and return `!True` + this method should create atomically initialize key and return `!True` otherwise it should return `!False`""" @abc.abstractmethod From be110c4ba0036056297de02e3fc38d7c2975bfbd Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 14 Sep 2015 09:20:45 +0200 Subject: [PATCH 58/66] Do not try to stop postgres twice if initialization had failed. --- patroni/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 6a4bf6aa..6406143b 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -46,6 +46,7 @@ class Patroni: """ cleanup the DCS if initialization was not successfull """ logger.info("removing initialize key after failed attempt to initialize the cluster") self.ha.dcs.cancel_initialization() + self.touch_member(self.shutdown_member_ttl) self.postgresql.stop() self.postgresql.move_data_directory() @@ -118,8 +119,8 @@ def main(): config = yaml.load(f) patroni = Patroni(config) + patroni.initialize() try: - patroni.initialize() patroni.run() except KeyboardInterrupt: pass From f494d2ce646dfdc35771eb8472dfb543e264829e Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 11:19:46 +0200 Subject: [PATCH 59/66] Build Cluster object for ZooKeeper the same way as for Etcd Previous implementation was always setting Cluster.initialize to True. Also it was throwing ZooKeeperError when there were no members in a cluster. Plus BUGFIX of a bug introduced with https://github.com/zalando/patroni/pull/34 in a `load_members` method. - data = self.get_node(self.member_path) + data = self.get_node(self.members_path + member) It was always fetching the same node for all cluster members. Fortunately Etcd doesn't have such problem because we are fetching the whole cluster directory with one recursive API call. --- patroni/zookeeper.py | 50 ++++++++++++++++++++++++++--------------- tests/test_zookeeper.py | 10 +++++++++ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 7c01f278..5f9625ac 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -92,9 +92,8 @@ class ZooKeeper(AbstractDCS): self.client.add_listener(self.session_listener) self.cluster_event = self.client.handler.event_object() + self.cluster = None self.fetch_cluster = True - self.members = [] - self.leader = None self.last_leader_operation = 0 self.client.start(None) @@ -121,18 +120,35 @@ class ZooKeeper(AbstractDCS): conn_url, api_url = parse_connection_string(value) return Member(znode.mzxid, name, conn_url, api_url, None, None) + def get_children(self, key, watch=None): + try: + return self.client.get_children(key, watch) + except NoNodeError: + pass + except: + logger.exception('get_children') + return [] + def load_members(self): members = [] - for member in self.client.get_children(self.members_path, self.cluster_watcher): - data = self.get_node(self.member_path) + for member in self.get_children(self.members_path, self.cluster_watcher): + data = self.get_node(self.members_path + member) if data is not None: members.append(self.member(member, *data)) return members def _inner_load_cluster(self): self.cluster_event.clear() - leader = self.get_node(self.leader_path, self.cluster_watcher) - self.members = self.load_members() + nodes = set(self.get_children(self.client_path(''))) + + # get initialize flag + initialize = self._INITIALIZE in nodes + + # get list of members + 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 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: @@ -142,15 +158,14 @@ class ZooKeeper(AbstractDCS): if leader: member = Member(-1, leader[0], None, None, None, None) - member = ([m for m in self.members if m.name == leader[0]] or [member])[0] + member = ([m for m in members if m.name == leader[0]] or [member])[0] leader = Leader(leader[1].mzxid, None, None, member) self.fetch_cluster = member.index == -1 - self.leader = leader - if self.fetch_cluster: - last_leader_operation = self.get_node(self.leader_optime_path) - if last_leader_operation: - self.last_leader_operation = int(last_leader_operation[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) def get_cluster(self): if self.exhibitor and self.exhibitor.poll(): @@ -163,7 +178,7 @@ class ZooKeeper(AbstractDCS): logger.exception('get_cluster') self.session_listener(KazooState.LOST) raise ZooKeeperError('ZooKeeper in not responding properly') - return Cluster(True, self.leader, self.last_leader_operation, self.members) + return self.cluster def _create(self, path, value, **kwargs): try: @@ -181,9 +196,8 @@ class ZooKeeper(AbstractDCS): return self._create(self.initialize_path, self._name, makepath=True) def touch_member(self, connection_string, ttl=None): - for m in self.members: - if m.name == self._name: - return True + if self.cluster and any(m.name == self._name for m in self.cluster.members): + return True path = self.member_path try: self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) @@ -217,8 +231,8 @@ class ZooKeeper(AbstractDCS): return True def delete_leader(self): - if isinstance(self.leader, Leader) and self.leader.name == self._name: - self.client.delete(self.leader_path) + if isinstance(self.cluster, Cluster) and self.cluster.leader.name == self._name: + self.client.delete(self.leader_path, version=self.cluster.leader.index) def cancel_initialization(self): node = self.get_node(self.initialize_path) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 0a95d0bc..19435c6e 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -75,6 +75,12 @@ class MockKazooClient: return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) def get_children(self, path, watch=None, include_data=False): + if path == '/no_node': + raise NoNodeError + elif path == '/other_exception': + raise Exception() + elif path in ['/service/bla/', '/service/test/']: + return ['initialize', 'leader', 'members', 'optime'] return ['foo', 'bar', 'buzz'] def create(self, path, value="", acl=None, ephemeral=False, sequence=False, makepath=False): @@ -138,6 +144,10 @@ class TestZooKeeper(unittest.TestCase): self.assertIsNone(self.zk.get_node('/no_node')) self.assertIsNone(self.zk.get_node('/other_exception')) + def test_get_children(self): + self.assertListEqual(self.zk.get_children('/no_node'), []) + self.assertListEqual(self.zk.get_children('/other_exception'), []) + def test__inner_load_cluster(self): self.zk._base_path = self.zk._base_path.replace('test', 'bla') self.zk._inner_load_cluster() From 209c985420b5b437b8afc9dfedfabf7366a01944 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 11:45:00 +0200 Subject: [PATCH 60/66] get_node and get_children should catch only NoNodeError exception. All other exceptions are needed to have retry functionality working correctly. --- patroni/zookeeper.py | 10 ++-------- tests/test_zookeeper.py | 6 ------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 5f9625ac..f2ef2e99 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -110,10 +110,7 @@ class ZooKeeper(AbstractDCS): try: return self.client.get(key, watch) except NoNodeError: - pass - except: - logger.exception('get_node') - return None + return None @staticmethod def member(name, value, znode): @@ -124,10 +121,7 @@ class ZooKeeper(AbstractDCS): try: return self.client.get_children(key, watch) except NoNodeError: - pass - except: - logger.exception('get_children') - return [] + return [] def load_members(self): members = [] diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 19435c6e..4b31bad2 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -58,8 +58,6 @@ class MockKazooClient: def get(self, path, watch=None): if path == '/no_node': raise NoNodeError - elif path == '/other_exception': - raise Exception() elif '/members/' in path: return ( 'postgres://repuser:rep-pass@localhost:5434/postgres?application_name=http://127.0.0.1:8009/patroni', @@ -77,8 +75,6 @@ class MockKazooClient: def get_children(self, path, watch=None, include_data=False): if path == '/no_node': raise NoNodeError - elif path == '/other_exception': - raise Exception() elif path in ['/service/bla/', '/service/test/']: return ['initialize', 'leader', 'members', 'optime'] return ['foo', 'bar', 'buzz'] @@ -142,11 +138,9 @@ class TestZooKeeper(unittest.TestCase): def test_get_node(self): self.assertIsNone(self.zk.get_node('/no_node')) - self.assertIsNone(self.zk.get_node('/other_exception')) def test_get_children(self): self.assertListEqual(self.zk.get_children('/no_node'), []) - self.assertListEqual(self.zk.get_children('/other_exception'), []) def test__inner_load_cluster(self): self.zk._base_path = self.zk._base_path.replace('test', 'bla') From 4a081bcb7179ad00f3771113176c27706ca43a76 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 11:58:10 +0200 Subject: [PATCH 61/66] Run cancel_initialization with retry --- patroni/zookeeper.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index f2ef2e99..38891c58 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -228,13 +228,16 @@ class ZooKeeper(AbstractDCS): if isinstance(self.cluster, Cluster) and self.cluster.leader.name == self._name: self.client.delete(self.leader_path, version=self.cluster.leader.index) - def cancel_initialization(self): + def _cancel_initialization(self): node = self.get_node(self.initialize_path) if node and node[0] == self._name: - try: - self.client.retry(self.client.delete, self.initialize_path, version=node[1].mzxid) - except KazooException: - logger.exception("Unable to delete initialize key") + self.client.delete(self.initialize_path, version=node[1].mzxid) + + def cancel_initialization(self): + try: + self.client.retry(self._cancel_initialization) + except: + logger.exception("Unable to delete initialize key") def watch(self, timeout): self.cluster_event.wait(timeout) From 98488a00a232d0294a2c4f5edc6414bf5a271d2b Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 12:00:24 +0200 Subject: [PATCH 62/66] Remove unused import of KazooException --- patroni/zookeeper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 38891c58..708cbafa 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -4,7 +4,7 @@ import requests import time from kazoo.client import KazooClient, KazooState -from kazoo.exceptions import NoNodeError, NodeExistsError, KazooException +from kazoo.exceptions import NoNodeError, NodeExistsError from patroni.dcs import AbstractDCS, Cluster, DCSError, Leader, Member, parse_connection_string from patroni.utils import sleep from requests.exceptions import RequestException From 51eacc5042bb4548a9bd5339497631f4c46ba90a Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 14 Sep 2015 12:36:28 +0200 Subject: [PATCH 63/66] Handle the case when initialize flag is not set and leader is present. --- patroni/__init__.py | 34 ++++++++++++++++++++-------------- tests/test_patroni.py | 37 +++++++++++++++++++++++++++++++++++-- tests/test_postgresql.py | 7 +++++++ 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 6406143b..cc008fda 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -6,6 +6,7 @@ import yaml from patroni.api import RestApiServer from patroni.etcd import Etcd +from patroni.exceptions import DCSError from patroni.ha import Ha from patroni.postgresql import Postgresql from patroni.utils import setup_signal_handlers, sleep, reap_children @@ -59,21 +60,26 @@ class Patroni: # is data directory empty? if self.postgresql.data_directory_empty(): while True: - # racing to initialize - if self.ha.dcs.initialize(): - try: - self.postgresql.bootstrap() - except: - # bail out and clean the initialize flag. - self.cleanup_on_failed_initialization() - raise - self.ha.dcs.take_leader() - break - else: - leader = self.ha.dcs.current_leader() - if leader and self.postgresql.bootstrap(leader): + try: + cluster = self.ha.dcs.get_cluster() + if not cluster.is_unlocked(): # the leader already exists + if not cluster.initialize: + self.ha.dcs.initialize() + self.postgresql.bootstrap(cluster.leader) break - sleep(5) + # racing to initialize + elif not cluster.initialize and self.ha.dcs.initialize(): + try: + self.postgresql.bootstrap() + except: + # bail out and clean the initialize flag. + self.cleanup_on_failed_initialization() + raise + self.ha.dcs.take_leader() + break + except DCSError: + logger.info('waiting on DCS') + sleep(5) elif self.postgresql.is_running(): self.postgresql.load_replication_slots() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index d5740e9c..317e1e5c 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -9,7 +9,7 @@ import yaml from mock import Mock, patch from patroni.api import RestApiServer -from patroni.dcs import Cluster, Member +from patroni.dcs import Cluster, Member, Leader from patroni.etcd import Etcd from patroni.exceptions import PostgresException from patroni import Patroni, main @@ -42,6 +42,30 @@ class Mock_BaseServer__is_shut_down: pass +def get_cluster(initialize, leader): + return Cluster(initialize, leader, None, None) + + +def get_cluster_not_initialized_without_leader(): + return get_cluster(None, None) + + +def get_cluster_initialized_without_leader(): + return get_cluster(True, None) + + +def get_cluster_not_initialized_with_leader(): + return get_cluster(False, 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(): + return get_cluster(True, Leader(0, 0, 0, + Member(0, 'leader', 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres', + None, None, 28))) + + class TestPatroni(unittest.TestCase): def __init__(self, method_name='runTest'): @@ -129,24 +153,31 @@ class TestPatroni(unittest.TestCase): def test_patroni_initialize(self): self.p.ha.dcs.client.write = etcd_write + self.p.ha.dcs.client.read = etcd_read self.p.touch_member = self.touch_member self.p.postgresql.data_directory_empty = true self.p.ha.dcs.initialize = true self.p.postgresql.initialize = true self.p.postgresql.start = true + self.p.ha.dcs.get_cluster = get_cluster_not_initialized_without_leader self.p.initialize() self.p.ha.dcs.initialize = false + self.p.ha.dcs.get_cluster = get_cluster_initialized_with_leader time.sleep = time_sleep self.p.ha.dcs.client.read = etcd_read self.p.initialize() - self.p.ha.dcs.current_leader = nop + self.p.ha.dcs.get_cluster = get_cluster_initialized_without_leader self.assertRaises(SleepException, self.p.initialize) self.p.postgresql.data_directory_empty = false self.p.initialize() + self.p.ha.dcs.get_cluster = get_cluster_not_initialized_with_leader + self.p.postgresql.data_directory_empty = true + self.p.initialize() + def test_schedule_next_run(self): self.p.next_run = time.time() - self.p.nap_time - 1 self.p.schedule_next_run() @@ -156,6 +187,8 @@ class TestPatroni(unittest.TestCase): def test_cleanup_on_initialization(self): self.p.ha.dcs.client.write = etcd_write + self.p.ha.dcs.client.read = etcd_read + self.p.ha.dcs.get_cluster = get_cluster_not_initialized_without_leader self.p.touch_member = self.touch_member self.p.postgresql.data_directory_empty = true self.p.ha.dcs.initialize = true diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 273d077d..56dbc557 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -6,6 +6,7 @@ import unittest from patroni.dcs import Cluster, Leader, Member from patroni.postgresql import Postgresql +from test_ha import true, false def nop(*args, **kwargs): @@ -217,3 +218,9 @@ class TestPostgresql(unittest.TestCase): self.p.start() self.p.query = self.mock_query self.assertTrue(self.p.stop()) + + def test_move_data_directory(self): + self.p.is_running = is_running + os.rename = nop + os.path.isdir = true + self.p.move_data_directory() From 44a20f12a4275ad0a3706c4740c6b11b4bd33949 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 16:32:45 +0200 Subject: [PATCH 64/66] version field is znode is just version, not mzxid --- patroni/zookeeper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 708cbafa..9e4ec8d8 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -115,7 +115,7 @@ class ZooKeeper(AbstractDCS): @staticmethod def member(name, value, znode): conn_url, api_url = parse_connection_string(value) - return Member(znode.mzxid, name, conn_url, api_url, None, None) + return Member(znode.version, name, conn_url, api_url, None, None) def get_children(self, key, watch=None): try: @@ -153,7 +153,7 @@ class ZooKeeper(AbstractDCS): if leader: member = Member(-1, leader[0], None, None, None, None) member = ([m for m in members if m.name == leader[0]] or [member])[0] - leader = Leader(leader[1].mzxid, None, None, member) + leader = Leader(leader[1].version, None, None, member) self.fetch_cluster = member.index == -1 # get last leader operation @@ -231,7 +231,7 @@ class ZooKeeper(AbstractDCS): def _cancel_initialization(self): node = self.get_node(self.initialize_path) if node and node[0] == self._name: - self.client.delete(self.initialize_path, version=node[1].mzxid) + self.client.delete(self.initialize_path, version=node[1].version) def cancel_initialization(self): try: From 90cfcf0c14d54163b249bf223f683445dcc163c2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 14 Sep 2015 17:14:39 +0200 Subject: [PATCH 65/66] Make zookeeper module compatible with python3 --- patroni/zookeeper.py | 8 +++++--- tests/test_etcd.py | 2 +- tests/test_zookeeper.py | 29 ++++++++++++++++++++++------- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 9e4ec8d8..eae810ed 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -108,7 +108,8 @@ class ZooKeeper(AbstractDCS): def get_node(self, key, watch=None): try: - return self.client.get(key, watch) + ret = self.client.get(key, watch) + return (ret[0].decode('utf-8'), ret[1]) except NoNodeError: return None @@ -176,7 +177,7 @@ class ZooKeeper(AbstractDCS): def _create(self, path, value, **kwargs): try: - self.client.retry(self.client.create, path, value, **kwargs) + self.client.retry(self.client.create, path, value.encode('utf-8'), **kwargs) return True except: return False @@ -193,6 +194,7 @@ class ZooKeeper(AbstractDCS): if 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') try: self.client.retry(self.client.create, path, connection_string, makepath=True, ephemeral=True) return True @@ -209,7 +211,7 @@ class ZooKeeper(AbstractDCS): return self.attempt_to_acquire_leader() def update_leader(self, state_handler): - last_operation = state_handler.last_operation() + last_operation = state_handler.last_operation().encode('utf-8') if last_operation != self.last_leader_operation: self.last_leader_operation = last_operation path = self.leader_optime_path diff --git a/tests/test_etcd.py b/tests/test_etcd.py index 66261eeb..10a602db 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -44,7 +44,7 @@ class MockPostgresql: name = '' def last_operation(self): - return 0 + return '0' def requests_get(url, **kwargs): diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 4b31bad2..b9defcb3 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -1,5 +1,6 @@ import patroni.zookeeper import requests +import six import unittest from patroni.dcs import Leader @@ -56,41 +57,55 @@ class MockKazooClient: func(*args, **kwargs) def get(self, path, watch=None): + if not isinstance(path, six.string_types): + raise TypeError("Invalid type for 'path' (string expected)") if path == '/no_node': raise NoNodeError elif '/members/' in path: return ( - 'postgres://repuser:rep-pass@localhost:5434/postgres?application_name=http://127.0.0.1:8009/patroni', + 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) ) elif path.endswith('/optime/leader'): - return '1' + return (b'1', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) elif path.endswith('/leader'): if self.leader: - return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, -1, 0, 0, 0)) - return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) + return (b'foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, -1, 0, 0, 0)) + return (b'foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) elif path.endswith('/initialize'): - return ('foo', ZnodeStat(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)) + return (b'foo', 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): + raise TypeError("Invalid type for 'path' (string expected)") if path == '/no_node': raise NoNodeError elif path in ['/service/bla/', '/service/test/']: return ['initialize', 'leader', 'members', 'optime'] return ['foo', 'bar', 'buzz'] - def create(self, path, value="", acl=None, ephemeral=False, sequence=False, makepath=False): + def create(self, path, value=b"", acl=None, ephemeral=False, sequence=False, makepath=False): + if not isinstance(path, six.string_types): + raise TypeError("Invalid type for 'path' (string expected)") + if not isinstance(value, (six.binary_type,)): + raise TypeError("Invalid type for 'value' (must be a byte string)") if path.endswith('/initialize') or path == '/service/test/optime/leader': raise Exception - elif value == 'retry' or (value == 'exists' and self.exists): + elif value == b'retry' or (value == b'exists' and self.exists): raise NodeExistsError def set(self, path, value, version=-1): + if not isinstance(path, six.string_types): + raise TypeError("Invalid type for 'path' (string expected)") + if not isinstance(value, (six.binary_type,)): + raise TypeError("Invalid type for 'value' (must be a byte string)") if path == '/service/bla/optime/leader': raise Exception raise NoNodeError def delete(self, path, version=-1, recursive=False): + if not isinstance(path, six.string_types): + raise TypeError("Invalid type for 'path' (string expected)") self.exists = False if path == '/service/test/leader': if self.leader: From 0435e36cad55701198fa7f70d0d33b7084c08699 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 16 Sep 2015 15:14:02 +0200 Subject: [PATCH 66/66] self.cluster = None if unexpected exception occured --- patroni/zookeeper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index eae810ed..bc16a8ea 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -170,6 +170,7 @@ 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')