mirror of
https://github.com/outbackdingo/patroni.git
synced 2026-08-25 14:53:37 +00:00
Bugfix: pg_rewind can work only with master
This commit is contained in:
+3
-3
@@ -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
|
||||
|
||||
@@ -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
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user