Skip to content

Populate HostMetric.used_in_inventories with actual inventory counts #16436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tll3r wants to merge 1 commit into
base: devel
Choose a base branch
from
+195 −3
Open

Populate HostMetric.used_in_inventories with actual inventory counts #16436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions awx/main/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,16 @@
hidden=True,
)

register(
'HOST_METRIC_INVENTORY_COUNTS_LAST_TS',
field_class=fields.DateTimeField,
label=_('Last run date for update_host_metric_inventory_counts'),
allow_null=True,
category=_('System'),
category_slug='system',
hidden=True,
)

register(
'AWX_CLEANUP_PATHS',
field_class=fields.BooleanField,
Expand Down
10 changes: 10 additions & 0 deletions awx/main/management/commands/update_host_metric_inventory_counts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from django.core.management.base import BaseCommand
from awx.main.tasks.host_metrics import HostMetricInventoryCountTask


class Command(BaseCommand):
help = 'Populate HostMetric.used_in_inventories with the count of inventories each host belongs to'

def handle(self, *args, **options):
rows = HostMetricInventoryCountTask.update_counts()
self.stdout.write(f'Updated {rows} HostMetric records')
64 changes: 61 additions & 3 deletions awx/main/tasks/host_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import logging

from django.conf import settings
from django.db.models import Count, F
from django.db.models.functions import TruncMonth
from django.db.models import Count, F, OuterRef, Subquery, Value
from django.db.models.functions import Coalesce, TruncMonth
from django.utils.timezone import now
from dispatcherd.publish import task
from awx.main.dispatch import get_task_queuename
from awx.main.models.inventory import HostMetric, HostMetricSummaryMonthly
from awx.main.models.inventory import Host, HostMetric, HostMetricSummaryMonthly
from awx.main.tasks.helpers import is_run_threshold_reached
from awx.conf.license import get_license
from ansible_base.lib.utils.db import advisory_lock
Expand Down Expand Up @@ -37,6 +37,25 @@ def host_metric_summary_monthly():
logger.info("Finished host_metric_summary_monthly")


@task(queue=get_task_queuename)
def update_host_metric_inventory_counts():
"""Populate HostMetric.used_in_inventories with the count of inventories each host belongs to."""
with advisory_lock('host_metric_inventory_counts', lock_session_timeout_milliseconds=300000, wait=False) as acquired:
if not acquired:
logger.info("Another instance of update_host_metric_inventory_counts is already running. Exiting.")
return
if is_run_threshold_reached(
getattr(settings, 'HOST_METRIC_INVENTORY_COUNTS_LAST_TS', None),
getattr(settings, 'HOST_METRIC_INVENTORY_COUNTS_INTERVAL', 1) * 86400,
):
logger.info(
f"Executing update_host_metric_inventory_counts, last ran at "
f"{getattr(settings, 'HOST_METRIC_INVENTORY_COUNTS_LAST_TS', '---')}"
)
rows = HostMetricInventoryCountTask.update_counts()
logger.info(f"Finished update_host_metric_inventory_counts, updated {rows} records")


class HostMetricTask:
"""
This class provides cleanup task for HostMetric model.
Expand Down Expand Up @@ -276,3 +295,42 @@ def _get_first_month():
"""Returns first month after host metrics hard delete threshold"""
threshold = getattr(settings, 'CLEANUP_HOST_METRICS_HARD_THRESHOLD', 36)
return datetime.date.today().replace(day=1) - relativedelta(months=int(threshold) - 1)


class HostMetricInventoryCountTask:
"""
Populates HostMetric.used_in_inventories with the number of distinct
inventories each hostname appears in (matching Host.name to HostMetric.hostname).
Only updates rows whose count actually changed to avoid unnecessary WAL churn.
"""

@staticmethod
def update_counts():
counts = dict(
Host.objects.values('name')
.annotate(cnt=Count('inventory_id', distinct=True))
.values_list('name', 'cnt')
)

rows = 0

# Set hosts with inventory membership to their actual count (only where changed)
if counts:
inventory_count_subquery = (
Host.objects.filter(name=OuterRef('hostname'))
.values('name')
.annotate(cnt=Count('inventory_id', distinct=True))
.values('cnt')
)
rows += HostMetric.objects.filter(hostname__in=counts.keys()).exclude(
used_in_inventories=Subquery(inventory_count_subquery)
).update(used_in_inventories=Subquery(inventory_count_subquery))

# Set hosts without any inventory membership to 0 (only where not already 0)
rows += HostMetric.objects.exclude(hostname__in=counts.keys()).exclude(
used_in_inventories=0
).update(used_in_inventories=0)

Comment on lines +309 to +333

@coderabbitai coderabbitai Bot Apr 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid materializing full host-name counts into Python for updates.

counts = dict(...) plus hostname__in=counts.keys() does not scale for large datasets and can produce very large IN (...) clauses; it also introduces a stale key-set window across two UPDATEs. Use one correlated subquery with Coalesce(..., 0) for a single set-based update.

Proposed refactor 
 class HostMetricInventoryCountTask:
@@
     `@staticmethod`
     def update_counts():
-        counts = dict(
-            Host.objects.values('name')
-            .annotate(cnt=Count('inventory_id', distinct=True))
-            .values_list('name', 'cnt')
-        )
-
-        rows = 0
-
-        # Set hosts with inventory membership to their actual count (only where changed)
-        if counts:
-            inventory_count_subquery = (
-                Host.objects.filter(name=OuterRef('hostname'))
-                .values('name')
-                .annotate(cnt=Count('inventory_id', distinct=True))
-                .values('cnt')
-            )
-            rows += HostMetric.objects.filter(hostname__in=counts.keys()).exclude(
-                used_in_inventories=Subquery(inventory_count_subquery)
-            ).update(used_in_inventories=Subquery(inventory_count_subquery))
-
-        # Set hosts without any inventory membership to 0 (only where not already 0)
-        rows += HostMetric.objects.exclude(hostname__in=counts.keys()).exclude(
-            used_in_inventories=0
-        ).update(used_in_inventories=0)
+        inventory_count_subquery = (
+            Host.objects.filter(name=OuterRef('hostname'))
+            .values('name')
+            .annotate(cnt=Count('inventory_id', distinct=True))
+            .values('cnt')[:1]
+        )
+        computed_count = Coalesce(Subquery(inventory_count_subquery), Value(0))
+
+        rows = HostMetric.objects.exclude(
+            used_in_inventories=computed_count
+        ).update(used_in_inventories=computed_count)

         settings.HOST_METRIC_INVENTORY_COUNTS_LAST_TS = now()
         logger.info(f"update_host_metric_inventory_counts: updated {rows} HostMetric records")
         return rows
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
counts = dict(
Host.objects.values('name')
.annotate(cnt=Count('inventory_id', distinct=True))
.values_list('name', 'cnt')
)
rows = 0
# Set hosts with inventory membership to their actual count (only where changed)
if counts:
inventory_count_subquery = (
Host.objects.filter(name=OuterRef('hostname'))
.values('name')
.annotate(cnt=Count('inventory_id', distinct=True))
.values('cnt')
)
rows += HostMetric.objects.filter(hostname__in=counts.keys()).exclude(
used_in_inventories=Subquery(inventory_count_subquery)
).update(used_in_inventories=Subquery(inventory_count_subquery))
# Set hosts without any inventory membership to 0 (only where not already 0)
rows += HostMetric.objects.exclude(hostname__in=counts.keys()).exclude(
used_in_inventories=0
).update(used_in_inventories=0)
inventory_count_subquery = (
Host.objects.filter(name=OuterRef('hostname'))
.values('name')
.annotate(cnt=Count('inventory_id', distinct=True))
.values('cnt')[:1]
)
computed_count = Coalesce(Subquery(inventory_count_subquery), Value(0))
rows = HostMetric.objects.exclude(
used_in_inventories=computed_count
).update(used_in_inventories=computed_count)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@awx/main/tasks/host_metrics.py` around lines 309 - 333, The current code
materializes a huge counts dict and issues two separate UPDATEs using
hostname__in=counts.keys(), which is unscalable and racy; replace both steps
with a single set-based update that uses a correlated subquery and Coalesce to
compute inventory membership per Host.hostname and write 0 when no match. Build
inventory_count_subquery like the existing inventory_count_subquery
(Host.objects.filter(name=OuterRef('hostname')).annotate(cnt=Count('inventory_id',
distinct=True)).values('cnt')), then perform one update:
HostMetric.objects.exclude(used_in_inventories=Coalesce(Subquery(inventory_count_subquery),
Value(0))).update(used_in_inventories=Coalesce(Subquery(inventory_count_subquery),
Value(0))) so you never materialize counts in Python and only update rows whose
value will actually change.

settings.HOST_METRIC_INVENTORY_COUNTS_LAST_TS = now()
logger.info(f"update_host_metric_inventory_counts: updated {rows} HostMetric records")
return rows
109 changes: 109 additions & 0 deletions awx/main/tests/functional/commands/test_update_host_metric_inventory_counts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import pytest
from django.utils.timezone import now

from awx.main.management.commands.update_host_metric_inventory_counts import Command
from awx.main.models import Inventory, Host, Organization
from awx.main.models.inventory import HostMetric


@pytest.fixture
def org(db):
return Organization.objects.create(name='test-org')


@pytest.fixture
def inventories(org):
return [
Inventory.objects.create(name='inv-1', organization=org),
Inventory.objects.create(name='inv-2', organization=org),
Inventory.objects.create(name='inv-3', organization=org),
]


@pytest.mark.django_db
def test_no_hosts_sets_zero(inventories):
"""HostMetric records with no matching Host rows get used_in_inventories=0."""
current_time = now()
HostMetric.objects.create(hostname='orphan-host', last_automation=current_time)

Command().handle()

hm = HostMetric.objects.get(hostname='orphan-host')
assert hm.used_in_inventories == 0


@pytest.mark.django_db
def test_single_inventory(inventories):
"""Host in one inventory produces used_in_inventories=1."""
current_time = now()
Host.objects.create(name='host-a', inventory=inventories[0])
HostMetric.objects.create(hostname='host-a', last_automation=current_time)

Command().handle()

hm = HostMetric.objects.get(hostname='host-a')
assert hm.used_in_inventories == 1


@pytest.mark.django_db
def test_multiple_inventories(inventories):
"""Same hostname across 3 inventories produces used_in_inventories=3."""
current_time = now()
for inv in inventories:
Host.objects.create(name='shared-host', inventory=inv)
HostMetric.objects.create(hostname='shared-host', last_automation=current_time)

Command().handle()

hm = HostMetric.objects.get(hostname='shared-host')
assert hm.used_in_inventories == 3


@pytest.mark.django_db
def test_mixed_hosts(inventories):
"""Different hosts with different inventory membership counts."""
current_time = now()

Host.objects.create(name='host-x', inventory=inventories[0])
Host.objects.create(name='host-x', inventory=inventories[1])
Host.objects.create(name='host-y', inventory=inventories[0])

HostMetric.objects.create(hostname='host-x', last_automation=current_time)
HostMetric.objects.create(hostname='host-y', last_automation=current_time)
HostMetric.objects.create(hostname='host-z', last_automation=current_time)

Command().handle()

assert HostMetric.objects.get(hostname='host-x').used_in_inventories == 2
assert HostMetric.objects.get(hostname='host-y').used_in_inventories == 1
assert HostMetric.objects.get(hostname='host-z').used_in_inventories == 0


@pytest.mark.django_db
def test_idempotent(inventories):
"""Running the command twice produces the same result."""
current_time = now()
Host.objects.create(name='host-idem', inventory=inventories[0])
Host.objects.create(name='host-idem', inventory=inventories[1])
HostMetric.objects.create(hostname='host-idem', last_automation=current_time)

Command().handle()
assert HostMetric.objects.get(hostname='host-idem').used_in_inventories == 2

Command().handle()
assert HostMetric.objects.get(hostname='host-idem').used_in_inventories == 2


@pytest.mark.django_db
def test_updates_after_inventory_change(inventories):
"""Counts update correctly when a host is added to a new inventory."""
current_time = now()
Host.objects.create(name='host-grow', inventory=inventories[0])
HostMetric.objects.create(hostname='host-grow', last_automation=current_time)

Command().handle()
assert HostMetric.objects.get(hostname='host-grow').used_in_inventories == 1

Host.objects.create(name='host-grow', inventory=inventories[2])
Command().handle()
assert HostMetric.objects.get(hostname='host-grow').used_in_inventories == 2
5 changes: 5 additions & 0 deletions awx/settings/defaults.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@
'awx.main.tasks.system.cleanup_images_and_files': {'task': 'awx.main.tasks.system.cleanup_images_and_files', 'schedule': 10800},
'awx.main.tasks.host_metrics.cleanup_host_metrics': {'task': 'awx.main.tasks.host_metrics.cleanup_host_metrics', 'schedule': 12600},
'awx.main.tasks.host_metrics.host_metric_summary_monthly': {'task': 'awx.main.tasks.host_metrics.host_metric_summary_monthly', 'schedule': 14400},
'awx.main.tasks.host_metrics.update_host_metric_inventory_counts': {'task': 'awx.main.tasks.host_metrics.update_host_metric_inventory_counts', 'schedule': 14400},
'awx.main.tasks.system.periodic_resource_sync': {'task': 'awx.main.tasks.system.periodic_resource_sync', 'schedule': 900},
'awx.main.tasks.host_indirect.cleanup_and_save_indirect_host_entries_fallback': {
'task': 'awx.main.tasks.host_indirect.cleanup_and_save_indirect_host_entries_fallback',
Expand Down Expand Up @@ -982,6 +983,10 @@
HOST_METRIC_SUMMARY_TASK_LAST_TS = None
HOST_METRIC_SUMMARY_TASK_INTERVAL = 7 # days

# Host metric inventory counts task - last time of run
HOST_METRIC_INVENTORY_COUNTS_LAST_TS = None
HOST_METRIC_INVENTORY_COUNTS_INTERVAL = 1 # days


# TODO: cmeyers, replace with with register pattern
# The register pattern is particularly nice for this because we need
Expand Down
Loading