mirror of
https://github.com/ansible/awx.git
synced 2026-10-29 22:03:20 -02:30
AAP-68024 Remove dead Host.last_job and last_job_host_summary FK fields (#16529)
* 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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
3de8dd7918
commit
3d1ca5a25d
+11
-8
@@ -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)
|
||||
|
||||
@@ -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],
|
||||
|
||||
@@ -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',
|
||||
),
|
||||
]
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user