Skip to content

Add query-count regression tests to guard hot ORM paths against N+1 (pattern + candidate list) #92

@marcinpsk

Description

@marcinpsk

Summary

Adopt query-count regression tests as a lightweight, CI-enforced way to guard hot/loopy ORM paths against N+1 regressions. This is a pattern/tooling proposal plus a candidate list — not a single fix.

Motivated by the render_actions per-row query discussion (raised by @bonzo81 on bonzo81#298, memoization declined because the key is per-row and the dominant cost is instantiate(); deferred to "revisit if it proves heavy"), and re-surfaced by CodeRabbit on the 0.4.7 release PR (bonzo81#308). A query-count test is the cleanest way to empirically answer "is it heavy / does it scale" and to lock in any future fix.

Technique

Stdlib + pytest-django, nothing to install:

  • @pytest.mark.django_db — run against the real Postgres (CI already provides it via the postgres service in test.yaml).
  • CaptureQueriesContext(connection) — records every SQL statement in the block; len(ctx) is the count.
  • (or) django_assert_num_queries — pytest-django fixture for an exact-count assertion.

In CI this is invisible plumbing: pytest auto-discovers the test under netbox_librenms_plugin/tests/, runs it against Postgres, and a failed assertion turns the check red. The only hard requirement is a real DB (these tests can't be MagicMock-based).

Prefer scaling assertions over exact counts

  • Exact count (assert_num_queries(7)) — precise but brittle; any unrelated change shifts the number and invites "just bump it" fixes.
  • Scaling / no-growth — render with N=2 and N=6 items and assert the count does not grow with N. Expresses the real intent ("must not be O(N)"), survives refactors, only fails on a genuine per-row regression. Use this for N+1 guards.

Template:

@pytest.mark.django_db
def test_module_table_render_is_not_n_plus_one():
    small = make_vc_device_with_modules(n=2)
    large = make_vc_device_with_modules(n=6)

    def render_query_count(device):
        table = LibreNMSModuleTable(records_for(device), device=device)
        with CaptureQueriesContext(connection) as ctx:
            for row in table.rows:          # forces render_* per cell
                [str(cell) for cell in row]
        return len(ctx)

    # Per-row queries would make the delta ~ (6-2)*k; constant => no N+1.
    assert render_query_count(large) == render_query_count(small)

Cost / gotcha (be selective)

Almost all current tests are MagicMock-based and emit zero real queries (that's why tables/modules.py even catches RuntimeError "Database access not allowed"). Query-count tests must be DB-backed with real fixtures (Device, VirtualChassis, ModuleBay, Module, interfaces, IPs), so they're slower and need more setup. Apply surgically to genuine loopy paths, not everywhere.

Where it pays off

Signature to target: a for row/port in ...: loop that touches the ORM per iteration (.objects.get/filter(), .interfaces.all(), get_librenms_device_id() which reads obj.cf, instantiate()).

  • tables/modules.py render_actions / format_module_data — the known one; first application once develop syncs the module work from upstream. The scaling test also settles the Feat/module interface name predict hook bonzo81/netbox-librenms-plugin#298 "is it heavy" question directly.
  • views/base/interfaces_view.py _prefetch_netbox_data / get_context_data — iterates ports, resolves interfaces.
  • views/base/ip_addresses_view.py enrich_ip_data / _prefetch_netbox_data — already batched; a test locks that in against future un-batching.
  • views/base/cables_view.py _prepare_context / link enrichment.
  • VC member-resolution loops.

Lighter blanket alternative

nplusone in test mode can auto-flag N+1 across the whole suite — but only on paths exercised with a real DB, and it's noisier/less precise than targeted CaptureQueriesContext assertions. Prefer a handful of explicit scaling tests on the loopy paths.

Rollout

  1. When the fork's develop pulls in the module work (PR Feat/module interface name predict hook bonzo81/netbox-librenms-plugin#298 etc., currently only on upstream/develop), add the module-table scaling test first.
  2. If it proves its worth, add a shared make_vc_device_with_modules(n) fixture and apply the template to the interface/IP/cable enrichment paths.
  3. Use a naming convention (test_*_query_count) or a perf pytest marker so they're easy to find / run / skip as a group if CI time ever bites.

Context / trail

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions