From e7347d15c1fcba482012580a22f1052694e3a361 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Mon, 11 May 2020 16:58:13 -0400 Subject: [PATCH 1/4] drastically optimize job host summary creation see: https://github.com/ansible/awx/issues/6991 --- awx/main/models/events.py | 33 +++++++------ awx/main/tasks.py | 2 + .../tests/functional/models/test_events.py | 48 ++++++++++++++++++- 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/awx/main/models/events.py b/awx/main/models/events.py index d5cd2d76e7..80d530a2c3 100644 --- a/awx/main/models/events.py +++ b/awx/main/models/events.py @@ -7,7 +7,7 @@ from collections import defaultdict from django.db import models, DatabaseError, connection from django.utils.dateparse import parse_datetime from django.utils.text import Truncator -from django.utils.timezone import utc +from django.utils.timezone import utc, now from django.utils.translation import ugettext_lazy as _ from django.utils.encoding import force_text @@ -407,11 +407,14 @@ class BasePlaybookEvent(CreatedModifiedModel): except (KeyError, ValueError): kwargs.pop('created', None) + host_map = kwargs.pop('host_map', {}) + sanitize_event_keys(kwargs, cls.VALID_KEYS) workflow_job_id = kwargs.pop('workflow_job_id', None) event = cls(**kwargs) if workflow_job_id: setattr(event, 'workflow_job_id', workflow_job_id) + setattr(event, 'host_map', host_map) event._update_from_event_data() return event @@ -484,8 +487,10 @@ class JobEvent(BasePlaybookEvent): if not self.job or not self.job.inventory: logger.info('Event {} missing job or inventory, host summaries not updated'.format(self.pk)) return - qs = self.job.inventory.hosts.filter(name__in=hostnames) job = self.job + + from awx.main.models.jobs import JobHostSummary # circular import + summaries = dict() for host in hostnames: host_stats = {} for stat in ('changed', 'dark', 'failures', 'ignored', 'ok', 'processed', 'rescued', 'skipped'): @@ -493,20 +498,18 @@ class JobEvent(BasePlaybookEvent): host_stats[stat] = self.event_data.get(stat, {}).get(host, 0) except AttributeError: # in case event_data[stat] isn't a dict. pass - if qs.filter(name=host).exists(): - host_actual = qs.get(name=host) - host_summary, created = job.job_host_summaries.get_or_create(host=host_actual, host_name=host_actual.name, defaults=host_stats) - else: - host_summary, created = job.job_host_summaries.get_or_create(host_name=host, defaults=host_stats) + host_id = self.host_map.get(host, None) + summaries.setdefault( + (host_id, host), + JobHostSummary(created=now(), modified=now(), job_id=job.id, host_id=host_id, host_name=host) + ) + host_summary = summaries[(host_id, host)] - if not created: - update_fields = [] - for stat, value in host_stats.items(): - if getattr(host_summary, stat) != value: - setattr(host_summary, stat, value) - update_fields.append(stat) - if update_fields: - host_summary.save(update_fields=update_fields) + for stat, value in host_stats.items(): + if getattr(host_summary, stat) != value: + setattr(host_summary, stat, value) + + JobHostSummary.objects.bulk_create(summaries.values()) @property def job_verbosity(self): diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 2acb1f2566..8e2c0a9dd5 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1215,6 +1215,8 @@ class BaseTask(object): else: event_data['host_name'] = '' event_data['host_id'] = '' + if event_data.get('event') == 'playbook_on_stats': + event_data['host_map'] = self.host_map if isinstance(self, RunProjectUpdate): # it's common for Ansible's SCM modules to print diff --git a/awx/main/tests/functional/models/test_events.py b/awx/main/tests/functional/models/test_events.py index a61c3fda25..0d2530b968 100644 --- a/awx/main/tests/functional/models/test_events.py +++ b/awx/main/tests/functional/models/test_events.py @@ -1,7 +1,9 @@ from unittest import mock import pytest -from awx.main.models import Job, JobEvent +from django.utils.timezone import now + +from awx.main.models import Job, JobEvent, Inventory, Host @pytest.mark.django_db @@ -61,3 +63,47 @@ def test_parent_failed(emit, event): assert events.count() == 2 for e in events.all(): assert e.failed is True + + +@pytest.mark.django_db +def test_host_summary_generation(): + hostnames = [f'Host {i}' for i in range(5000)] + inv = Inventory() + inv.save() + Host.objects.bulk_create([ + Host(created=now(), modified=now(), name=h, inventory_id=inv.id) + for h in hostnames + ]) + j = Job(inventory=inv) + j.save() + host_map = dict((host.name, host.id) for host in inv.hosts.all()) + JobEvent.create_from_data( + job_id=j.pk, + parent_uuid='abc123', + event='playbook_on_stats', + event_data={ + 'ok': dict((hostname, len(hostname)) for hostname in hostnames), + 'changed': {}, + 'dark': {}, + 'failures': {}, + 'ignored': {}, + 'processed': {}, + 'rescued': {}, + 'skipped': {}, + }, + host_map=host_map + ).save() + + assert j.job_host_summaries.count() == len(hostnames) + assert sorted([s.host_name for s in j.job_host_summaries.all()]) == sorted(hostnames) + + for s in j.job_host_summaries.all(): + assert host_map[s.host_name] == s.host_id + assert s.ok == len(s.host_name) + assert s.changed == 0 + assert s.dark == 0 + assert s.failures == 0 + assert s.ignored == 0 + assert s.processed == 0 + assert s.rescued == 0 + assert s.skipped == 0 From 3cb24753070c01a80941c6bc4abc32d53e4fa1c8 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 12 May 2020 09:38:59 -0400 Subject: [PATCH 2/4] properly handle host summary bulk updates if hosts go missing --- awx/main/models/events.py | 18 ++++---- .../tests/functional/models/test_events.py | 44 ++++++++++++++++++- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/awx/main/models/events.py b/awx/main/models/events.py index 80d530a2c3..57de629a8f 100644 --- a/awx/main/models/events.py +++ b/awx/main/models/events.py @@ -489,25 +489,23 @@ class JobEvent(BasePlaybookEvent): return job = self.job - from awx.main.models.jobs import JobHostSummary # circular import + from awx.main.models import Host, JobHostSummary # circular import + existing = Host.objects.filter(id__in=self.host_map.values()).values_list('id', flat=True) + summaries = dict() for host in hostnames: + host_id = self.host_map.get(host, None) + if host_id not in existing: + host_id = None host_stats = {} for stat in ('changed', 'dark', 'failures', 'ignored', 'ok', 'processed', 'rescued', 'skipped'): try: host_stats[stat] = self.event_data.get(stat, {}).get(host, 0) except AttributeError: # in case event_data[stat] isn't a dict. pass - host_id = self.host_map.get(host, None) - summaries.setdefault( - (host_id, host), - JobHostSummary(created=now(), modified=now(), job_id=job.id, host_id=host_id, host_name=host) + summaries[(host_id, host)] = JobHostSummary( + created=now(), modified=now(), job_id=job.id, host_id=host_id, host_name=host, **host_stats ) - host_summary = summaries[(host_id, host)] - - for stat, value in host_stats.items(): - if getattr(host_summary, stat) != value: - setattr(host_summary, stat, value) JobHostSummary.objects.bulk_create(summaries.values()) diff --git a/awx/main/tests/functional/models/test_events.py b/awx/main/tests/functional/models/test_events.py index 0d2530b968..6dd0cac06a 100644 --- a/awx/main/tests/functional/models/test_events.py +++ b/awx/main/tests/functional/models/test_events.py @@ -67,7 +67,7 @@ def test_parent_failed(emit, event): @pytest.mark.django_db def test_host_summary_generation(): - hostnames = [f'Host {i}' for i in range(5000)] + hostnames = [f'Host {i}' for i in range(500)] inv = Inventory() inv.save() Host.objects.bulk_create([ @@ -107,3 +107,45 @@ def test_host_summary_generation(): assert s.processed == 0 assert s.rescued == 0 assert s.skipped == 0 + + +@pytest.mark.django_db +def test_host_summary_generation_with_deleted_hosts(): + hostnames = [f'Host {i}' for i in range(10)] + inv = Inventory() + inv.save() + Host.objects.bulk_create([ + Host(created=now(), modified=now(), name=h, inventory_id=inv.id) + for h in hostnames + ]) + j = Job(inventory=inv) + j.save() + host_map = dict((host.name, host.id) for host in inv.hosts.all()) + + # delete half of the hosts during the playbook run + for h in inv.hosts.all()[:5]: + h.delete() + + JobEvent.create_from_data( + job_id=j.pk, + parent_uuid='abc123', + event='playbook_on_stats', + event_data={ + 'ok': dict((hostname, len(hostname)) for hostname in hostnames), + 'changed': {}, + 'dark': {}, + 'failures': {}, + 'ignored': {}, + 'processed': {}, + 'rescued': {}, + 'skipped': {}, + }, + host_map=host_map + ).save() + + + ids = sorted([s.host_id or -1 for s in j.job_host_summaries.order_by('id').all()]) + names = sorted([s.host_name for s in j.job_host_summaries.all()]) + assert ids == [-1, -1, -1, -1, -1, 6, 7, 8, 9, 10] + assert names == ['Host 0', 'Host 1', 'Host 2', 'Host 3', 'Host 4', 'Host 5', + 'Host 6', 'Host 7', 'Host 8', 'Host 9'] From 917c6b405e97ed2371d32b6d96206ced224214c8 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 12 May 2020 14:46:57 -0400 Subject: [PATCH 3/4] properly update .failed, .last_job_id, and last_job_host_summary --- awx/main/models/events.py | 20 ++++++++++++++++++- awx/main/models/jobs.py | 14 ------------- .../tests/functional/models/test_events.py | 6 +++++- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/awx/main/models/events.py b/awx/main/models/events.py index 57de629a8f..e8d87253eb 100644 --- a/awx/main/models/events.py +++ b/awx/main/models/events.py @@ -503,12 +503,30 @@ class JobEvent(BasePlaybookEvent): host_stats[stat] = self.event_data.get(stat, {}).get(host, 0) except AttributeError: # in case event_data[stat] isn't a dict. pass - summaries[(host_id, host)] = JobHostSummary( + summary = JobHostSummary( created=now(), modified=now(), job_id=job.id, host_id=host_id, host_name=host, **host_stats ) + summary.failed = bool(summary.dark or summary.failures) + summaries[(host_id, host)] = summary JobHostSummary.objects.bulk_create(summaries.values()) + # update the last_job_id and last_job_host_summary_id + # in single queries + host_mapping = dict( + (summary['host'], summary['id']) + for summary in JobHostSummary.objects.filter(job_id=job.id).values('id', 'host') + ) + all_hosts = Host.objects.filter( + pk__in=host_mapping.keys() + ).only('id') + for h in all_hosts: + h.last_job_id = job.id + if h.id in host_mapping: + h.last_job_host_summary_id = host_mapping[h.id] + Host.objects.bulk_update(all_hosts, ['last_job_id', 'last_job_host_summary_id']) + + @property def job_verbosity(self): return self.job.verbosity diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 1ecb788900..8f42b9d577 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -1129,20 +1129,6 @@ class JobHostSummary(CreatedModifiedModel): self.failed = bool(self.dark or self.failures) update_fields.append('failed') super(JobHostSummary, self).save(*args, **kwargs) - self.update_host_last_job_summary() - - def update_host_last_job_summary(self): - update_fields = [] - if self.host is None: - return - if self.host.last_job_id != self.job_id: - self.host.last_job_id = self.job_id - update_fields.append('last_job_id') - if self.host.last_job_host_summary_id != self.id: - self.host.last_job_host_summary_id = self.id - update_fields.append('last_job_host_summary_id') - if update_fields: - self.host.save(update_fields=update_fields) class SystemJobOptions(BaseModel): diff --git a/awx/main/tests/functional/models/test_events.py b/awx/main/tests/functional/models/test_events.py index 6dd0cac06a..7f881a2fea 100644 --- a/awx/main/tests/functional/models/test_events.py +++ b/awx/main/tests/functional/models/test_events.py @@ -67,7 +67,7 @@ def test_parent_failed(emit, event): @pytest.mark.django_db def test_host_summary_generation(): - hostnames = [f'Host {i}' for i in range(500)] + hostnames = [f'Host {i}' for i in range(100)] inv = Inventory() inv.save() Host.objects.bulk_create([ @@ -108,6 +108,10 @@ def test_host_summary_generation(): assert s.rescued == 0 assert s.skipped == 0 + for host in Host.objects.all(): + assert host.last_job_id == j.id + assert host.last_job_host_summary.host == host + @pytest.mark.django_db def test_host_summary_generation_with_deleted_hosts(): From 74308f3dad749e9f1d353e54601d9382de838d87 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 12 May 2020 16:20:22 -0400 Subject: [PATCH 4/4] further optimize job host summary queries --- awx/main/models/events.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/awx/main/models/events.py b/awx/main/models/events.py index e8d87253eb..ac33a311f4 100644 --- a/awx/main/models/events.py +++ b/awx/main/models/events.py @@ -490,12 +490,15 @@ class JobEvent(BasePlaybookEvent): job = self.job from awx.main.models import Host, JobHostSummary # circular import - existing = Host.objects.filter(id__in=self.host_map.values()).values_list('id', flat=True) + all_hosts = Host.objects.filter( + pk__in=self.host_map.values() + ).only('id') + existing_host_ids = set(h.id for h in all_hosts) summaries = dict() for host in hostnames: host_id = self.host_map.get(host, None) - if host_id not in existing: + if host_id not in existing_host_ids: host_id = None host_stats = {} for stat in ('changed', 'dark', 'failures', 'ignored', 'ok', 'processed', 'rescued', 'skipped'): @@ -514,12 +517,9 @@ class JobEvent(BasePlaybookEvent): # update the last_job_id and last_job_host_summary_id # in single queries host_mapping = dict( - (summary['host'], summary['id']) - for summary in JobHostSummary.objects.filter(job_id=job.id).values('id', 'host') + (summary['host_id'], summary['id']) + for summary in JobHostSummary.objects.filter(job_id=job.id).values('id', 'host_id') ) - all_hosts = Host.objects.filter( - pk__in=host_mapping.keys() - ).only('id') for h in all_hosts: h.last_job_id = job.id if h.id in host_mapping: