From e2feac87bcb8bf0c4ef64e44024a32301c907a3c Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 24 Jul 2017 14:19:19 +0200 Subject: [PATCH] Block callbacks during bootstrap (#483) It wasn't a big issue when on_start was called during normal boostrap with initdb, because usually such process is very fast. But situation is changing when we run custom bootstrap, becuase it might be a long time between cluster become connectable and end of recovery and promote. Actually situation was even worse than that, on_start was called with the `replica` argument and later on_role_changes was never called, because promote wasn't performed by Patroni. As a solution for this problem we will block any callbacks during bootstrap and explicitly call on_start after leader lock was taken. --- patroni/ha.py | 11 ++++++----- patroni/postgresql.py | 3 +++ tests/test_postgresql.py | 3 ++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index a14913ce..999589a1 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -102,7 +102,6 @@ class Ha(object): self.cluster = None self.old_cluster = None self.recovering = False - self._bootstrapping = False self._post_bootstrap_task = None self._start_timeout = None self._async_executor = AsyncExecutor(self.wakeup) @@ -206,7 +205,7 @@ class Ha(object): # no initialize key and node is allowed to be master and has 'bootstrap' section in a configuration file elif self.cluster.initialize is None and not self.patroni.nofailover and 'bootstrap' in self.patroni.config: if self.dcs.initialize(create_new=True): # race for initialization - self._bootstrapping = True + self.state_handler.bootstrapping = True self._post_bootstrap_task = CriticalTask() self._async_executor.schedule('bootstrap') self._async_executor.run_async(self.state_handler.bootstrap, args=(self.patroni.config['bootstrap'],)) @@ -890,7 +889,7 @@ class Ha(object): try: if self.has_lock() and self.update_lock(): return 'updated leader lock during ' + self._async_executor.scheduled_action - elif not self._bootstrapping: + elif not self.state_handler.bootstrapping: # Don't have lock, make sure we are not starting up a master in the background if self.state_handler.role == 'master': logger.info("Demoting master during " + self._async_executor.scheduled_action) @@ -946,14 +945,16 @@ class Ha(object): if not self.state_handler.is_leader(): return 'waiting for end of recovery after bootstrap' + self.state_handler.set_role('master') self._async_executor.schedule('post_bootstrap') self._async_executor.run_async(self.state_handler.post_bootstrap, args=(self.patroni.config['bootstrap'], self._post_bootstrap_task)) return 'running post_bootstrap' - self._bootstrapping = False + self.state_handler.bootstrapping = False self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration, separators=(',', ':'))) self.dcs.take_leader() + self.state_handler.call_nowait(ACTION_ON_START) self.load_cluster_from_dcs() return 'initialized a new cluster' @@ -1030,7 +1031,7 @@ class Ha(object): return msg # we've got here, so any async action has finished. - if self._bootstrapping: + if self.state_handler.bootstrapping: return self.post_bootstrap() if self.recovering and not self.state_handler.need_rewind: diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 7b37b39c..61868d7d 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -104,6 +104,7 @@ class Postgresql(object): self._data_dir = config['data_dir'] self._config_dir = os.path.abspath(config.get('config_dir') or self._data_dir) self._pending_restart = False + self.bootstrapping = False self._running_custom_bootstrap = False self.__thread_ident = current_thread().ident @@ -742,6 +743,8 @@ class Postgresql(object): def call_nowait(self, cb_name): """ pick a callback command and call it without waiting for it to finish """ + if self.bootstrapping: + return if cb_name in (ACTION_ON_START, ACTION_ON_STOP, ACTION_ON_RESTART, ACTION_ON_ROLE_CHANGE): self.__cb_called = True diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 4b1342df..5e613cb6 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -487,10 +487,11 @@ class TestPostgresql(unittest.TestCase): self.assertFalse(self.p.is_running()) @patch('shlex.split', Mock(side_effect=OSError)) - @patch.object(Postgresql, 'can_rewind', PropertyMock(return_value=True)) def test_call_nowait(self): self.p.set_role('replica') self.assertIsNone(self.p.call_nowait('on_start')) + self.p.bootstrapping = True + self.assertIsNone(self.p.call_nowait('on_start')) def test_non_existing_callback(self): self.assertFalse(self.p.call_nowait('foobar'))