From adb454b2e8a9fc7b69c40c4fa62c21020a68524c Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 30 Apr 2021 09:21:45 +0200 Subject: [PATCH] Optimize CountPoolChildrenForQuota task in amphorav2 The CountPoolChildrenForQuota task takes 3 or 4 seconds to execute in amphorav2. When cascade deleting a load balancer with many pools (60), it might take minutes. We don't need to get and to convert all the fields from the object to compute the number of children of a pool, we can optimize this task by creating a new method in the PoolRepository class. Story 2008873 Task 42414 Change-Id: Ie4d40509ae7e6faccecd1d0cc0bc4c7d93959261 (cherry picked from commit 7075d22c4cb0dfc25b4c5c8eff654aa4358f2f25) --- .../worker/v2/tasks/database_tasks.py | 10 ++--- octavia/db/repositories.py | 10 +++++ .../tests/functional/db/test_repositories.py | 44 +++++++++++++++++++ .../v2/tasks/test_database_tasks_quota.py | 36 +++------------ 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/octavia/controller/worker/v2/tasks/database_tasks.py b/octavia/controller/worker/v2/tasks/database_tasks.py index 3985984d8e..9b8e81c098 100644 --- a/octavia/controller/worker/v2/tasks/database_tasks.py +++ b/octavia/controller/worker/v2/tasks/database_tasks.py @@ -2873,14 +2873,10 @@ class CountPoolChildrenForQuota(BaseDatabaseTask): :returns: None """ session = db_apis.get_session() - db_pool = self.pool_repo.get(session, id=pool_id) - LOG.debug("Counting pool children for " - "project: %s ", db_pool.project_id) + hm_count, member_count = ( + self.pool_repo.get_children_count(session, pool_id)) - health_mon_count = 1 if db_pool.health_monitor else 0 - member_count = len(db_pool.members) - - return {'HM': health_mon_count, 'member': member_count} + return {'HM': hm_count, 'member': member_count} class DecrementL7policyQuota(BaseDatabaseTask): diff --git a/octavia/db/repositories.py b/octavia/db/repositories.py index b082599501..776d756ca4 100644 --- a/octavia/db/repositories.py +++ b/octavia/db/repositories.py @@ -1027,6 +1027,16 @@ class PoolRepository(BaseRepository): session, pagination_helper=pagination_helper, query_options=query_options, **filters) + def get_children_count(self, session, pool_id): + hm_count = session.query(models.HealthMonitor).filter( + models.HealthMonitor.pool_id == pool_id, + models.HealthMonitor.provisioning_status != consts.DELETED).count() + member_count = session.query(models.Member).filter( + models.Member.pool_id == pool_id, + models.Member.provisioning_status != consts.DELETED).count() + + return (hm_count, member_count) + class MemberRepository(BaseRepository): model_class = models.Member diff --git a/octavia/tests/functional/db/test_repositories.py b/octavia/tests/functional/db/test_repositories.py index d1a77df012..30032a8d6f 100644 --- a/octavia/tests/functional/db/test_repositories.py +++ b/octavia/tests/functional/db/test_repositories.py @@ -2666,6 +2666,50 @@ class PoolRepositoryTest(BaseRepositoryTest): self.assertIsNone(self.hm_repo.get(self.session, pool_id=hm.pool_id)) self.assertIsNone(self.sp_repo.get(self.session, pool_id=sp.pool_id)) + def test_get_children_count(self): + pool = self.create_pool(pool_id=self.FAKE_UUID_1, + project_id=self.FAKE_UUID_2) + hm_count, member_count = ( + self.pool_repo.get_children_count(self.session, pool.id)) + self.assertEqual(0, hm_count) + self.assertEqual(0, member_count) + + self.hm_repo.create(self.session, pool_id=pool.id, + type=constants.HEALTH_MONITOR_HTTP, + delay=1, timeout=1, fall_threshold=1, + rise_threshold=1, enabled=True, + provisioning_status=constants.ACTIVE, + operating_status=constants.ONLINE) + + hm_count, member_count = ( + self.pool_repo.get_children_count(self.session, pool.id)) + self.assertEqual(1, hm_count) + self.assertEqual(0, member_count) + + self.member_repo.create(self.session, id=self.FAKE_UUID_3, + project_id=self.FAKE_UUID_2, + pool_id=pool.id, + ip_address="192.0.2.1", + protocol_port=80, + provisioning_status=constants.ACTIVE, + operating_status=constants.ONLINE, + enabled=True, + backup=False) + self.member_repo.create(self.session, id=self.FAKE_UUID_4, + project_id=self.FAKE_UUID_2, + pool_id=pool.id, + ip_address="192.0.2.2", + protocol_port=80, + provisioning_status=constants.ACTIVE, + operating_status=constants.ONLINE, + enabled=True, + backup=False) + + hm_count, member_count = ( + self.pool_repo.get_children_count(self.session, pool.id)) + self.assertEqual(1, hm_count) + self.assertEqual(2, member_count) + class MemberRepositoryTest(BaseRepositoryTest): diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks_quota.py b/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks_quota.py index f086e5526a..10953d9684 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks_quota.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks_quota.py @@ -319,48 +319,24 @@ class TestDatabaseTasksQuota(base.TestCase): self.assertEqual(1, mock_lock_session.rollback.call_count) @mock.patch('octavia.db.api.get_session') - @mock.patch('octavia.db.repositories.PoolRepository.get') + @mock.patch('octavia.db.repositories.PoolRepository.get_children_count') def test_count_pool_children_for_quota(self, repo_mock, session_mock): project_id = uuidutils.generate_uuid() - member1 = data_models.Member(id=1, project_id=project_id) - member2 = data_models.Member(id=2, project_id=project_id) - healtmon = data_models.HealthMonitor(id=1, project_id=project_id) - pool_no_children = data_models.Pool(id=1, project_id=project_id) - pool_1_mem = data_models.Pool(id=1, project_id=project_id, - members=[member1]) - pool_hm = data_models.Pool(id=1, project_id=project_id, - health_monitor=healtmon) - pool_hm_2_mem = data_models.Pool(id=1, project_id=project_id, - health_monitor=healtmon, - members=[member1, member2]) + pool = data_models.Pool(id=1, project_id=project_id) task = database_tasks.CountPoolChildrenForQuota() # Test pool with no children repo_mock.reset_mock() - repo_mock.return_value = pool_no_children - result = task.execute(pool_no_children.id) + repo_mock.return_value = (0, 0) + result = task.execute(pool.id) self.assertEqual({'HM': 0, 'member': 0}, result) - # Test pool with one member - repo_mock.reset_mock() - repo_mock.return_value = pool_1_mem - result = task.execute(pool_1_mem.id) - - self.assertEqual({'HM': 0, 'member': 1}, result) - - # Test pool with health monitor and no members - repo_mock.reset_mock() - repo_mock.return_value = pool_hm - result = task.execute(pool_hm.id) - - self.assertEqual({'HM': 1, 'member': 0}, result) - # Test pool with health monitor and two members repo_mock.reset_mock() - repo_mock.return_value = pool_hm_2_mem - result = task.execute(pool_hm_2_mem.id) + repo_mock.return_value = (1, 2) + result = task.execute(pool.id) self.assertEqual({'HM': 1, 'member': 2}, result)