Merge pull request #203 from zalando/bugfix/pg_rewind

Bugfix: pg_rewind can work only with master
This commit is contained in:
Alexander Kukushkin
2016-05-30 11:01:49 +02:00
3 changed files with 20 additions and 22 deletions
+3 -3
View File
@@ -71,7 +71,7 @@ class Ha(object):
logger.info('bootstrapped %s', msg)
cluster = self.dcs.get_cluster()
node_to_follow = self._get_node_to_follow(cluster)
self.state_handler.follow(node_to_follow, True)
self.state_handler.follow(node_to_follow, cluster.leader, True)
else:
logger.error('failed to bootstrap %s', msg)
self.state_handler.remove_data_directory()
@@ -134,7 +134,7 @@ class Ha(object):
if not self.state_handler.check_recovery_conf(node_to_follow) or recovery:
self._async_executor.schedule('changing primary_conninfo and restarting')
self._async_executor.run_async(self.state_handler.follow, (node_to_follow, recovery))
self._async_executor.run_async(self.state_handler.follow, (node_to_follow, self.cluster.leader, recovery))
return ret
def enforce_master_role(self, message, promote_message):
@@ -278,7 +278,7 @@ class Ha(object):
sleep(2) # Give a time to somebody to promote
self.recover()
else:
self.state_handler.follow(None)
self.state_handler.follow(None, None)
def process_manual_failover_from_leader(self):
failover = self.cluster.failover
+7 -9
View File
@@ -511,12 +511,9 @@ class Postgresql(object):
logger.info("running pg_rewind from %s", pc)
pg_rewind = ['pg_rewind', '-D', self._data_dir, '--source-server', pc]
try:
ret = subprocess.call(pg_rewind, env=env) == 0
return subprocess.call(pg_rewind, env=env) == 0
except OSError:
ret = False
if ret:
self.write_recovery_conf(leader)
return ret
return False
def controldata(self):
""" return the contents of pg_controldata, or non-True value if pg_controldata call failed """
@@ -561,14 +558,13 @@ class Postgresql(object):
except OSError:
logger.exception("Unable to list %s", status_dir)
def follow(self, leader, recovery=False):
if self.check_recovery_conf(leader) and not recovery:
def follow(self, member, leader, recovery=False):
if self.check_recovery_conf(member) and not recovery:
return True
change_role = self.role == 'master'
need_rewind = change_role and self.can_rewind
if need_rewind:
logger.info("set the rewind flag after demote")
self.write_recovery_conf(leader)
if leader and need_rewind: # we have a leader and need to rewind
if self.is_running():
self.stop()
@@ -578,7 +574,7 @@ class Postgresql(object):
# XXX: if recovery.conf is linked, it will be written anew as a normal file.
if os.path.islink(self._recovery_conf):
os.unlink(self._recovery_conf)
else:
elif os.path.isfile(self._recovery_conf):
os.remove(self._recovery_conf)
# Archived segments might be useful to pg_rewind,
# clean the flags that tell we should remove them.
@@ -586,12 +582,14 @@ class Postgresql(object):
# Start in a single user mode and stop to produce a clean shutdown
self.single_user_mode(options={'archive_mode': 'on', 'archive_command': 'false'})
if self.rewind(leader):
self.write_recovery_conf(member)
ret = self.start()
else:
logger.error("unable to rewind the former master")
self.remove_data_directory()
ret = True
else: # do not rewind until the leader becomes available
self.write_recovery_conf(member)
ret = self.restart()
if change_role and ret:
self.call_nowait(ACTION_ON_ROLE_CHANGE)
+10 -10
View File
@@ -248,22 +248,22 @@ class TestPostgresql(unittest.TestCase):
@patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
@patch('subprocess.check_output', Mock(return_value=0, side_effect=pg_controldata_string))
def test_follow(self, mock_pg_rewind):
self.p.follow(None)
self.p.follow(self.leader)
self.p.follow(Leader(-1, 28, self.other))
self.p.follow(None, None)
self.p.follow(self.leader, self.leader)
self.p.follow(Leader(-1, 28, self.other), self.leader)
self.p.rewind = mock_pg_rewind
self.p.follow(self.leader)
self.p.follow(self.leader, self.leader)
with mock.patch('os.path.islink', MagicMock(return_value=True)):
with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)):
with mock.patch('os.unlink', MagicMock(return_value=True)):
self.p.follow(self.leader, recovery=True)
self.p.follow(self.leader, self.leader, recovery=True)
with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)):
self.p.rewind.return_value = True
self.p.follow(self.leader, recovery=True)
self.p.follow(self.leader, self.leader, recovery=True)
self.p.rewind.return_value = False
self.p.follow(self.leader, recovery=True)
self.p.follow(self.leader, self.leader, recovery=True)
with mock.patch('patroni.postgresql.Postgresql.check_recovery_conf', MagicMock(return_value=True)):
self.assertTrue(self.p.follow(None))
self.assertTrue(self.p.follow(None, None))
@patch('subprocess.check_output', Mock(return_value=0, side_effect=pg_controldata_string))
def test_can_rewind(self):
@@ -423,7 +423,7 @@ class TestPostgresql(unittest.TestCase):
def test_cleanup_archive_status(self, mock_file, mock_link, mock_remove, mock_unlink):
ap = os.path.join(self.data_dir, 'pg_xlog', 'archive_status/')
self.p.cleanup_archive_status()
mock_remove.assert_has_calls([mock.call(ap+'a'), mock.call(ap+'b'), mock.call(ap+'c')])
mock_remove.assert_has_calls([mock.call(ap + 'a'), mock.call(ap + 'b'), mock.call(ap + 'c')])
mock_unlink.assert_not_called()
mock_remove.reset_mock()
@@ -431,7 +431,7 @@ class TestPostgresql(unittest.TestCase):
mock_file.return_value = False
mock_link.return_value = True
self.p.cleanup_archive_status()
mock_unlink.assert_has_calls([mock.call(ap+'a'), mock.call(ap+'b'), mock.call(ap+'c')])
mock_unlink.assert_has_calls([mock.call(ap + 'a'), mock.call(ap + 'b'), mock.call(ap + 'c')])
mock_remove.assert_not_called()
mock_unlink.reset_mock()