From 3d1ca5a25dfe0530cff6f3444608a01b9ba9ccac Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 26 Aug 2026 14:28:49 -0400 Subject: [PATCH] AAP-68024 Remove dead Host.last_job and last_job_host_summary FK fields (#16529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove dead Host.last_job and Host.last_job_host_summary FK fields PR #16332 replaced these denormalized ForeignKey columns with dynamic derivation via Host.latest_summary, but left the columns in place. Nothing writes to or reads from them anymore — drop the dead weight. - Remove last_job and last_job_host_summary ForeignKey fields from Host - Convert HostSerializer to use SerializerMethodField instead of FK-backed fields - Remove redundant JobHostSummary.latest_for_host/latest_job_for_host classmethods - Add migration 0206 to drop the columns - Update tests to use Host.latest_summary instead of removed fields/methods * Remove dead last_job_host_summary_id UPDATE from cleanup_jobs The Host.last_job_host_summary FK was removed in the prior commit, so the raw SQL UPDATE that NULLed it before deleting JHS rows now references a non-existent column and crashes. Co-authored-by: Claude Opus 4.6 --- awx/api/serializers.py | 19 +++++----- awx/main/management/commands/cleanup_jobs.py | 17 +++------ .../0210_remove_host_last_job_fields.py | 18 ++++++++++ awx/main/models/events.py | 3 -- awx/main/models/inventory.py | 17 --------- awx/main/models/jobs.py | 16 --------- awx/main/signals.py | 5 --- .../tests/functional/models/test_events.py | 5 ++- .../models/test_host_summary_fields.py | 4 --- .../tests/unit/commands/test_cleanup_jobs.py | 36 +++++-------------- 10 files changed, 43 insertions(+), 97 deletions(-) create mode 100644 awx/main/migrations/0210_remove_host_last_job_fields.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 75041dc771..31b625e2b4 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -174,8 +174,6 @@ SUMMARIZABLE_FK_FIELDS = { 'workflow_approval': DEFAULT_SUMMARY_FIELDS + ('timeout',), 'schedule': DEFAULT_SUMMARY_FIELDS + ('next_run',), 'unified_job_template': DEFAULT_SUMMARY_FIELDS + ('unified_job_type',), - # last_job and last_job_host_summary are derived from JobHostSummary in HostSerializer, - # not from the stale FK fields on Host. 'last_update': DEFAULT_SUMMARY_FIELDS + ('status', 'failed', 'license_error'), 'current_update': DEFAULT_SUMMARY_FIELDS + ('status', 'failed', 'license_error'), 'current_job': DEFAULT_SUMMARY_FIELDS + ('status', 'failed', 'license_error'), @@ -1803,6 +1801,8 @@ class HostSerializer(BaseSerializerWithVariables): has_active_failures = serializers.SerializerMethodField() has_inventory_sources = serializers.SerializerMethodField() + last_job = serializers.SerializerMethodField() + last_job_host_summary = serializers.SerializerMethodField() class Meta: model = Host @@ -1818,7 +1818,7 @@ class HostSerializer(BaseSerializerWithVariables): 'last_job_host_summary', 'ansible_facts_modified', ) - read_only_fields = ('last_job', 'last_job_host_summary', 'ansible_facts_modified') + read_only_fields = ('ansible_facts_modified',) def build_relational_field(self, field_name, relation_info): field_class, field_kwargs = super(HostSerializer, self).build_relational_field(field_name, relation_info) @@ -1953,13 +1953,16 @@ class HostSerializer(BaseSerializerWithVariables): return ret if 'inventory' in ret and not obj.inventory: ret['inventory'] = None - last_summary = obj.latest_summary - if 'last_job' in ret: - ret['last_job'] = last_summary.job_id if last_summary else None - if 'last_job_host_summary' in ret: - ret['last_job_host_summary'] = last_summary.pk if last_summary else None return ret + def get_last_job(self, obj): + last_summary = obj.latest_summary + return last_summary.job_id if last_summary else None + + def get_last_job_host_summary(self, obj): + last_summary = obj.latest_summary + return last_summary.pk if last_summary else None + def get_has_active_failures(self, obj): last_summary = obj.latest_summary return bool(last_summary and last_summary.failed) diff --git a/awx/main/management/commands/cleanup_jobs.py b/awx/main/management/commands/cleanup_jobs.py index 391f2ce2b4..b454dff8e3 100644 --- a/awx/main/management/commands/cleanup_jobs.py +++ b/awx/main/management/commands/cleanup_jobs.py @@ -53,28 +53,19 @@ JHS_CHUNK_SIZE = 1000 def _pre_delete_job_host_summaries(job_pks, logger=None): - """Pre-delete JobHostSummary rows and clear Host FK references in batches. + """Pre-delete JobHostSummary rows in batches. - Django's cascade collector materializes all JHS IDs into a single - UPDATE ... IN (...) to SET_NULL on Host.last_job_host_summary. - With many jobs x hosts this exceeds PostgreSQL's 1GB alloc limit. - Doing it in chunks with raw SQL avoids that. + Deleting via raw SQL in chunks avoids Django's cascade collector + materializing all JHS IDs into memory at once, which can exceed + PostgreSQL's 1GB alloc limit when many jobs x hosts are involved. """ if not job_pks: return - # ANY(%s) is PostgreSQL-specific; AWX only supports PostgreSQL with connection.cursor() as cursor: for i in range(0, len(job_pks), JHS_CHUNK_SIZE): chunk = list(job_pks[i : i + JHS_CHUNK_SIZE]) - cursor.execute( - "UPDATE main_host SET last_job_host_summary_id = NULL" - " WHERE last_job_host_summary_id IN" - " (SELECT id FROM main_jobhostsummary WHERE job_id = ANY(%s))", - [chunk], - ) - cursor.execute( "DELETE FROM main_jobhostsummary WHERE job_id = ANY(%s)", [chunk], diff --git a/awx/main/migrations/0210_remove_host_last_job_fields.py b/awx/main/migrations/0210_remove_host_last_job_fields.py new file mode 100644 index 0000000000..45192e7012 --- /dev/null +++ b/awx/main/migrations/0210_remove_host_last_job_fields.py @@ -0,0 +1,18 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ('main', '0209_fix_system_auditor_migration'), + ] + + operations = [ + migrations.RemoveField( + model_name='host', + name='last_job', + ), + migrations.RemoveField( + model_name='host', + name='last_job_host_summary', + ), + ] diff --git a/awx/main/models/events.py b/awx/main/models/events.py index 17436f5ec2..c43957d621 100644 --- a/awx/main/models/events.py +++ b/awx/main/models/events.py @@ -589,9 +589,6 @@ class JobEvent(BasePlaybookEvent): JobHostSummary.objects.bulk_create(summaries.values()) - # last_job and last_job_host_summary are now derived via - # JobHostSummary.latest_for_host / latest_job_for_host - # Create/update Host Metrics self._update_host_metrics(updated_hosts_list) diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 53f1757769..4157d96921 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -550,23 +550,6 @@ class Host(CommonModelNameNotUnique, RelatedJobsMixin): help_text=_('Host variables in JSON or YAML format.'), ) ) - last_job = models.ForeignKey( - 'Job', - related_name='hosts_as_last_job+', - null=True, - default=None, - editable=False, - on_delete=models.SET_NULL, - ) - last_job_host_summary = models.ForeignKey( - 'JobHostSummary', - related_name='hosts_as_last_job_summary+', - blank=True, - null=True, - default=None, - editable=False, - on_delete=models.SET_NULL, - ) inventory_sources = models.ManyToManyField( 'InventorySource', related_name='hosts', diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 06cebf2ac6..81819571d9 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -1140,22 +1140,6 @@ class JobHostSummary(CreatedModifiedModel): self.skipped, ) - @classmethod - def latest_for_host(cls, host_id): - """Return the most recent JobHostSummary for a given host, or None.""" - return cls.objects.filter(host_id=host_id).order_by('-id').first() - - @classmethod - def latest_job_for_host(cls, host_id): - """Return the Job from the most recent JobHostSummary for a host, or None.""" - summary = cls.latest_for_host(host_id) - if summary: - try: - return summary.job - except cls.job.field.related_model.DoesNotExist: - return None - return None - def get_absolute_url(self, request=None): return reverse('api:job_host_summary_detail', kwargs={'pk': self.pk}, request=request) diff --git a/awx/main/signals.py b/awx/main/signals.py index bc9740d860..309727f729 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -302,11 +302,6 @@ def migrate_children_from_deleted_group_to_parent_groups(sender, **kwargs): pass -# Host.last_job and Host.last_job_host_summary are now derived from -# JobHostSummary.latest_for_host / latest_job_for_host. -# No signal handlers needed to maintain these denormalized FKs. - - # Set via ActivityStreamRegistrar to record activity stream events diff --git a/awx/main/tests/functional/models/test_events.py b/awx/main/tests/functional/models/test_events.py index 104b5601f4..bae0c73071 100644 --- a/awx/main/tests/functional/models/test_events.py +++ b/awx/main/tests/functional/models/test_events.py @@ -71,7 +71,7 @@ class TestEvents: assert s.skipped == 0 for host in Host.objects.all(): - latest_summary = JobHostSummary.latest_for_host(host.id) + latest_summary = host.latest_summary assert latest_summary is not None assert latest_summary.job_id == self.job.id assert latest_summary.host == host @@ -106,13 +106,12 @@ class TestEvents: # be related to the appropriate Host) assert JobHostSummary.objects.count() == 1 for h in Host.objects.all(): - latest_summary = JobHostSummary.latest_for_host(h.id) + latest_summary = h.latest_summary if h.name == 'Host 1': assert latest_summary is not None assert latest_summary.job_id == self.job.id assert latest_summary.id == JobHostSummary.objects.first().id else: - # all other hosts in the inventory should have no summary assert latest_summary is None def test_host_metrics_insert(self): diff --git a/awx/main/tests/functional/models/test_host_summary_fields.py b/awx/main/tests/functional/models/test_host_summary_fields.py index 35c4449085..d301e234e3 100644 --- a/awx/main/tests/functional/models/test_host_summary_fields.py +++ b/awx/main/tests/functional/models/test_host_summary_fields.py @@ -47,10 +47,6 @@ class TestHostSummaryFields: ).save() summary = JobHostSummary.objects.filter(host=host, job=job).first() - host.last_job = job - host.last_job_host_summary = summary - host.save(update_fields=['last_job', 'last_job_host_summary']) - host.refresh_from_db() return host, job, summary diff --git a/awx/main/tests/unit/commands/test_cleanup_jobs.py b/awx/main/tests/unit/commands/test_cleanup_jobs.py index 0e36807583..545a20fc94 100644 --- a/awx/main/tests/unit/commands/test_cleanup_jobs.py +++ b/awx/main/tests/unit/commands/test_cleanup_jobs.py @@ -18,13 +18,8 @@ class TestPreDeleteJobHostSummaries: _pre_delete_job_host_summaries(job_pks) - assert mock_cursor.execute.call_count == 2 - update_call = mock_cursor.execute.call_args_list[0] - assert 'UPDATE main_host SET last_job_host_summary_id = NULL' in update_call[0][0] - assert 'ANY(%s)' in update_call[0][0] - assert update_call[0][1] == [[1, 2, 3]] - - delete_call = mock_cursor.execute.call_args_list[1] + assert mock_cursor.execute.call_count == 1 + delete_call = mock_cursor.execute.call_args_list[0] assert 'DELETE FROM main_jobhostsummary' in delete_call[0][0] assert 'ANY(%s)' in delete_call[0][0] assert delete_call[0][1] == [[1, 2, 3]] @@ -38,16 +33,16 @@ class TestPreDeleteJobHostSummaries: _pre_delete_job_host_summaries(job_pks) - # 2 chunks x 2 SQL statements each = 4 execute calls - assert mock_cursor.execute.call_count == 4 + # 2 chunks x 1 DELETE each = 2 execute calls + assert mock_cursor.execute.call_count == 2 # First chunk should have JHS_CHUNK_SIZE items - first_update = mock_cursor.execute.call_args_list[0] - assert len(first_update[0][1][0]) == JHS_CHUNK_SIZE + first_delete = mock_cursor.execute.call_args_list[0] + assert len(first_delete[0][1][0]) == JHS_CHUNK_SIZE # Second chunk should have the remainder - second_update = mock_cursor.execute.call_args_list[2] - assert len(second_update[0][1][0]) == 499 + second_delete = mock_cursor.execute.call_args_list[1] + assert len(second_delete[0][1][0]) == 499 def test_sql_is_fully_static(self): """SQL strings contain no interpolated values — only ANY(%s) placeholders.""" @@ -77,21 +72,6 @@ class TestPreDeleteJobHostSummaries: logger.debug.assert_called_once() - def test_update_runs_before_delete(self): - """Host FK must be NULLed before JHS rows are deleted.""" - job_pks = [1] - with mock.patch('awx.main.management.commands.cleanup_jobs.connection') as mock_conn: - mock_cursor = mock.MagicMock() - mock_conn.cursor.return_value.__enter__ = mock.Mock(return_value=mock_cursor) - mock_conn.cursor.return_value.__exit__ = mock.Mock(return_value=False) - - _pre_delete_job_host_summaries(job_pks) - - first_sql = mock_cursor.execute.call_args_list[0][0][0] - second_sql = mock_cursor.execute.call_args_list[1][0][0] - assert 'UPDATE' in first_sql - assert 'DELETE' in second_sql - class TestDeleteMetaPreDelete: """Verify DeleteMeta.delete_jobs() calls _pre_delete_job_host_summaries correctly."""