From 4cfbf6669d77c78b95fa7252b5032e33db4bd1a1 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) (cherry picked from commit adb454b2e8a9fc7b69c40c4fa62c21020a68524c) (cherry picked from commit 37b15da4a13da800b991d81e89e15fe1ff216568) --- .../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 ca97ca4eab..a0c8ab1fed 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 UpdatePoolMembersOperatingStatusInDB(BaseDatabaseTask): diff --git a/octavia/db/repositories.py b/octavia/db/repositories.py index 4cdb6486bb..6f1ec17194 100644 --- a/octavia/db/repositories.py +++ b/octavia/db/repositories.py @@ -948,6 +948,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 65c0f08aba..13fc16d957 100644 --- a/octavia/tests/functional/db/test_repositories.py +++ b/octavia/tests/functional/db/test_repositories.py @@ -2098,6 +2098,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 6637120c7b..feb304de32 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 @@ -294,47 +294,23 @@ 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)