Abstract CitusHandler and decouple it from configuration (#2950)

the main issue was that the configuration for Citus handler and for DCS existed in two places, while ideally AbstractDCS should not know many details about what kind of MPP is in use.

To solve the problem we first dynamically create an object implementing AbstractMPP interfaces, which is a configuration for DCS. Later this object is used to instantiate the class implementing AbstractMPPHandler interface.

This is just a starting point, which does some heavy lifting. As a next steps all kind of variables named after Citus in files different from patroni/postgres/mpp/citus.py should be renamed.

In other words this commit takes over the most complex part of #2940, which was never implemented.

Co-authored-by: zhjwpku <[email protected]>
This commit is contained in:
Alexander Kukushkin
2023-12-21 08:58:26 +01:00
committed by GitHub
co-authored by zhjwpku
parent 5c3e1a693e
commit bcfd8438a5
28 changed files with 602 additions and 185 deletions
+11 -11
View File
@@ -197,7 +197,7 @@ def run_async(self, func, args=()):
@patch('patroni.async_executor.AsyncExecutor.busy', PropertyMock(return_value=False))
@patch('patroni.async_executor.AsyncExecutor.run_async', run_async)
@patch('patroni.postgresql.rewind.Thread', Mock())
@patch('patroni.postgresql.citus.CitusHandler.start', Mock())
@patch('patroni.postgresql.mpp.citus.CitusHandler.start', Mock())
@patch('subprocess.call', Mock(return_value=0))
@patch('time.sleep', Mock())
class TestHa(PostgresInit):
@@ -593,8 +593,8 @@ class TestHa(PostgresInit):
self.assertEqual(self.ha.bootstrap(), 'failed to acquire initialize lock')
@patch('patroni.psycopg.connect', psycopg_connect)
@patch('patroni.postgresql.citus.connect', psycopg_connect)
@patch('patroni.postgresql.citus.quote_ident', Mock())
@patch('patroni.postgresql.mpp.citus.connect', psycopg_connect)
@patch('patroni.postgresql.mpp.citus.quote_ident', Mock())
@patch.object(Postgresql, 'connection', Mock(return_value=None))
def test_bootstrap_initialized_new_cluster(self):
self.ha.cluster = get_cluster_not_initialized_without_leader()
@@ -615,8 +615,8 @@ class TestHa(PostgresInit):
self.assertRaises(PatroniFatalException, self.ha.post_bootstrap)
@patch('patroni.psycopg.connect', psycopg_connect)
@patch('patroni.postgresql.citus.connect', psycopg_connect)
@patch('patroni.postgresql.citus.quote_ident', Mock())
@patch('patroni.postgresql.mpp.citus.connect', psycopg_connect)
@patch('patroni.postgresql.mpp.citus.quote_ident', Mock())
@patch.object(Postgresql, 'connection', Mock(return_value=None))
def test_bootstrap_release_initialize_key_on_watchdog_failure(self):
self.ha.cluster = get_cluster_not_initialized_without_leader()
@@ -659,7 +659,7 @@ class TestHa(PostgresInit):
@patch.object(ConfigHandler, 'replace_pg_hba', Mock())
@patch.object(ConfigHandler, 'replace_pg_ident', Mock())
@patch.object(PostmasterProcess, 'start', Mock(return_value=MockPostmaster()))
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_worker_restart(self):
self.ha.has_lock = true
self.ha.patroni.request = Mock()
@@ -694,7 +694,7 @@ class TestHa(PostgresInit):
self.ha.is_paused = true
self.assertEqual(self.ha.run_cycle(), 'PAUSE: restart in progress')
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_manual_failover_from_leader(self):
self.ha.has_lock = true # I am the leader
@@ -733,7 +733,7 @@ class TestHa(PostgresInit):
('Member %s exceeds maximum replication lag', 'b'))
self.ha.cluster.members.pop()
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_manual_switchover_from_leader(self):
self.ha.has_lock = true # I am the leader
@@ -774,7 +774,7 @@ class TestHa(PostgresInit):
self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
self.assertEqual(mock_info.call_args_list[0][0], ('Member %s exceeds maximum replication lag', 'leader'))
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_scheduled_switchover_from_leader(self):
self.ha.has_lock = true # I am the leader
@@ -1544,7 +1544,7 @@ class TestHa(PostgresInit):
self.ha.is_failover_possible = true
self.ha.shutdown()
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_shutdown_citus_worker(self):
self.ha.is_leader = true
self.p.is_running = Mock(side_effect=[Mock(), False])
@@ -1656,7 +1656,7 @@ class TestHa(PostgresInit):
self.assertRaises(DCSError, self.ha.acquire_lock)
self.assertFalse(self.ha.acquire_lock())
@patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
@patch('patroni.postgresql.mpp.AbstractMPPHandler.is_coordinator', Mock(return_value=False))
def test_notify_citus_coordinator(self):
self.ha.patroni.request = Mock()
self.ha.notify_citus_coordinator('before_demote')