Skip to content

perf(importers): look up only the endpoints a report can match - #16225

Merged
Maffooch merged 2 commits into
devfrom
perf/endpoint-manager-scoped-lookup
Oct 7, 2026
Merged

Maffooch merged 2 commits into
devfrom
perf/endpoint-manager-scoped-lookup

Conversation

@Maffooch

@Maffooch Maffooch commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Description

EndpointManager.get_or_create_endpoints runs on every import and reimport flush. To match the endpoints the report names, it read every endpoint of the product into Python. Only opening the cursor counts as SQL time, so on a product with millions of endpoints a flush spent minutes iterating rows while SQL time stayed small. Production diagnostics show this shape: reimports with wall time far above SQL time, at worst about 33 s of SQL in a request that ran for about 25 minutes.

The lookup now selects only the product's endpoints whose lower(host) matches the host of a queued key. A queued key with no host also matches null or empty hosts. The existing (product, lower(host)) index serves the lookup. Only rows whose full key was queued are kept. Matching does not change:

  • protocol and host compare case-insensitively;
  • the scheme's default port equals no port;
  • the first endpoint by id wins;
  • other products' endpoints never match.

Test results

New TestGetOrCreateEndpointsScopedLookup in unittests/test_endpoint_manager.py:

  • the rows loaded from dojo_endpoint do not grow when unrelated endpoints are added to the product. On dev it fails with 1 != 61 (60 unrelated endpoints added);
  • the matching rules above, plus null host and other-product endpoints, are pinned in one test.

On dev the module gives 13 tests and 2 failures; with this change all 13 pass, run with the CI tag filters (--exclude-tag=non-parallel --exclude-tag=performance).

Wider runs pass in both modes:

  • test_endpoint_manager, test_importers_performance, test_import_reimport, test_tag_inheritance_perf and test_endpoint_meta_import: 212 tests with DD_V3_FEATURE_LOCATIONS=False, 180 with it on.
  • The test_importers_performance query counts are unchanged.

Scale check through POST /api/v2/import-scan/ and /reimport-scan/ (Generic Findings JSON, one endpoint per finding) into a product holding 5M endpoints:

Request dev wall time This PR
Reimport of 200 findings, nothing changed 201 s (0.85 s SQL) 12.8 s
Reimport of 1,000 findings, nothing changed 574 s (2.2 s SQL) 22 s
Import of 1,000 findings 754 s 112 s
Reimport of 1,000 findings, 20% changed 431 s 50 s

Query counts are the same before and after; only the rows read change.

Ruff (ruff==0.16.9) check passes on the changed files.

Documentation

No user-facing change.

Checklist

  • Make sure to rebase your PR against the very latest dev.
  • Submit all PRs, features and bug fixes alike, against the dev branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is Ruff compliant (see ruff.toml).
  • Your code is python 3.13 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

🤖 Generated with Claude Code

EndpointManager.get_or_create_endpoints read every endpoint of the product
into Python on each import and reimport flush to match the handful the
report names. Only opening the cursor counts as SQL time, so on a product
with millions of endpoints a flush spent minutes iterating rows while the
SQL time stayed small. A large product's reimport took hundreds of seconds
even when nothing had to be created.

The lookup now selects only the product's endpoints whose lower(host)
matches a queued key's host (plus null/empty hosts when a queued key has
none), which the existing (product, lower(host)) index serves, and keeps
only rows whose full key was queued. Matching is unchanged: case-insensitive
protocol and host, the scheme's default port equal to no port, first by id
wins, and other products' endpoints never match.

Measured on a product with 5M endpoints: a 1000-finding reimport with
nothing changed went from 574 s to 22 s wall, an import of 1000 findings
from 754 s to 112 s, and a reimport with 20% changed from 431 s to 50 s.
Query counts are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch Maffooch added this to the 3.4.100 milestone Oct 6, 2026
@Maffooch
Maffooch requested a review from blakeaowens as a code owner October 6, 2026 17:34
@Maffooch

Maffooch commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Adversarial review (independent session, per review-pr)

Recommendation: request-changes. There is one blocker. On real data the scoped lookup matches the old full-product scan, and the speedup holds up. The blocker is that the SQL prefilter lowercases with Postgres while the key check lowercases with Python. For some hosts the two disagree, and the importer then creates a new duplicate endpoint on every flush.

Blocker

1. Python str.lower() and Postgres lower() disagree, so some existing endpoints are never found and a duplicate is created on every import/reimport. dojo/importers/endpoint_manager.py:187-193 (_existing_endpoints_for_queued_hosts) filters on Lower("host") IN {key.host}, where key.host was lowercased in Python (_make_endpoint_unique_tuple, line 62). A row is only a candidate when Postgres's lowercase of its host equals Python's lowercase of the queued host. Where the two differ, the row is excluded before the Python key comparison runs. The old full scan lowercased both sides in Python, so it never had this mismatch.

Checked on Postgres 16, en_US.utf8, libc provider:

  • lower('İSTANBUL.COM') gives istanbul.com. Python gives i̇stanbul.com (with a combining dot).
  • Greek capital sigma at the end of a word: Python 'ΑΣ1'.lower() gives ας1 (final sigma). Postgres gives ασ1.
  • With a C ctype or a column using COLLATE "C", Postgres lowercases ASCII only. lower('ÄBC.example.com' COLLATE "C") gives Äbc.example.com, so every uppercase non-ASCII host diverges on such a database.

These hosts reach the table with their case intact. Endpoint.from_uri keeps it (hyperlink.parse('https://İSTANBUL.example.com/x').host returns 'İSTANBUL.example.com'), and a failed Endpoint.clean() only logs "storing broken endpoint" and saves the row anyway.

Reproduced with the old loop and the new code side by side on the same rows, in a rolled-back transaction. One existing endpoint https://İSTANBUL.example.com, then three flushes that each queue that same endpoint:

  • old full scan: 1 row after three flushes;
  • this PR: 4 rows after three flushes, so one new duplicate per flush, with no upper bound.

The duplicates also reach Pro's endpoint-manager subclass, which reprioritizes the product whenever created is non-empty. So every reimport of such a report also triggers a product-wide prioritization pass.

Suggested fix: make the SQL candidate set a superset of what the Python key check accepts. The smallest change that stops the growth is to also match Postgres's own lowercase of the raw queued hosts, so a host spelled the same way always finds its row:

raw_hosts = {kwargs["host"] for kwargs in self._endpoints_to_create.values() if kwargs["host"]}
host_filter = Q(lower_host__in=lowered) | Q(lower_host__in=[Lower(Value(h)) for h in raw_hosts])

I tried that filter, together with the null-host change from item 2, as a patch to the method on the same rows (three flushes of İSTANBUL.example.com plus a hostless key). This PR ends with 5 rows, 3 of them duplicates. The patched version ends with 2, so nothing is created. A rarer leftover case remains: two different spellings that Python folds together and Postgres does not (a stored ΑΣ1 queued as ας1). That case creates one row and then stabilizes. Please add a regression test that flushes İSTANBUL.example.com twice against an existing row and asserts nothing is created.

Nice to have

2. A queued key with no host makes the lookup scan every endpoint in the database. endpoint_manager.py:191-192 adds Q(host__isnull=True) | Q(host=""). That predicate cannot use idx_ep_product_lower_host, so Postgres falls back to a parallel seq scan of dojo_endpoint. EXPLAIN ANALYZE on the 5M-endpoint database:

  • as written: Parallel Seq Scan, 47,757 ms;
  • written as lower(host) IS NULL OR lower(host) = '': three bitmap index scans on idx_ep_product_lower_host, 6.5 ms.

So Q(lower_host__isnull=True) | Q(lower_host="") gives the same rows from the index. A no-host endpoint is a common parser output (a scheme-less string parses as a path), so this matters in practice. I measured one flush of 452 keys, one of them hostless, at 31.1 s against the 5M-endpoint product. Without that key the lookup is index-only.

3. The branch no longer contains the current origin/dev. Rebase before merge.

Adversarial attempts

  1. Red then green, checked myself. I restored endpoint_manager.py from the base commit and kept the new tests. test_rows_loaded_do_not_grow_with_unrelated_product_endpoints failed with 1 != 61 and a subtest list mismatch; 13 tests, 2 failures. With the PR's code, all 13 pass. The command used the CI tag filters and DD_V3_FEATURE_LOCATIONS=False.
  2. Equivalence on real data. On the 5M-endpoint product I queued 452 keys: 400 random existing endpoints, with every fourth host uppercased, every fourth protocol uppercased and every fourth path changed, plus 50 new hosts, tmpl.example.com and one null-host key. I compared against a full-scan reference that keeps only queued keys. Result: 0 differences in the endpoint id returned per key, and 151 created on both sides. New code took 31.1 s (see item 2). The full scan took 345.8 s. Everything was rolled back.
  3. Edge hosts, old vs new on the same rows:
    • match identically: uppercase IPv6 (2001:DB8::1); bracketed IPv6 on the default port; default port 80/443 vs none; mixed-case duplicates (first by id wins); userinfo, path, query and fragment variants on the same host; empty vs null host in both directions; a host with leading and trailing spaces; another product's endpoint; straße/STRASSE; the Kelvin sign; the titlecase digraph Dž;
    • differ: ΑΣ1.example.com (old found the existing row, new created one) and İSTANBUL across repeated flushes (attempt 4).
  4. Repeated reimport flushes: blocker 1. Base stays at 1 row; this PR grows by 1 row per flush.
  5. Do any endpoints_by_key consumers need keys that were not queued? No. The only consumer is persist(). Every status key comes from record_endpoint in the same flush (record_for_finding, lines 338-343), and _endpoints_to_create is cleared only at the end of get_or_create_endpoints. Pro's subclass only wraps the call. The old code returned a key for every product endpoint, but nothing read the extra keys.
  6. Scale. At 100k, 1M and 10M endpoints per product, the lookup stays an index probe per distinct host, except for the null-host case (item 2), which degrades to a full-table scan in SQL. That is still far cheaper than the old Python iteration.

Suites

unittests.test_importers_performance, unittests.test_import_reimport and unittests.test_endpoint_meta_import with DD_V3_FEATURE_LOCATIONS=False and the CI tag filters: 164 tests, OK (16 skipped). unittests.test_endpoint_manager: 13/13. I did not run these with Locations on, because the endpoint manager is not used in that mode.

Verified hands-on vs static

  • Hands-on: red then green; the edge-host comparison on a scratch test database; old vs new on the 5M-endpoint dataset; EXPLAIN ANALYZE of both predicate shapes; Postgres vs Python lower() checks; hyperlink host parsing.
  • Static only: what Pro's subclass does with extra created rows (read, not run).
  • Still to verify: the fix for item 1, with a repeated-flush test; whether any supported deployment runs a C-ctype database, which makes item 1 hit every uppercase non-ASCII host.

🤖 Generated with Claude Code

An adversarial review found that the scoped endpoint lookup filtered on
LOWER(host) with Python-lowered key hosts. The two disagree for some
characters (a dotted capital I, a final sigma, any non-ASCII letter under a
C collation), so the existing endpoint was missed and every flush created
another duplicate.

The filter now also accepts the database's LOWER() of the raw queued hosts,
computed inside the same query, so no round trip is added and the importer
query counts are unchanged. The null/empty-host branch is written against
lower_host so the (product, lower(host)) index serves it instead of a scan
of the whole endpoint table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch

Maffooch commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Both points are fixed in e89121f.

Blocker (Python vs database lowercase). The filter now accepts both the Python-lowered key hosts and LOWER() of the raw queued hosts, computed inside the same SQL statement as Lower(Value(host)) literals. So no round trip is added, and the importer query counts stay where they were. The key comparison still uses Python's lowercase on both sides, as the old full-product scan did, so the rows matched are the same as before.

New test test_host_whose_python_and_database_lowercase_differ_is_reused. It flushes İSTANBUL.example.com (and an empty host) three times against existing rows.

  • On the previous head (b52c8c9) it fails, creating a duplicate on every flush: 2 != 5 rows.
  • With the fix it passes, with no new rows.

Should-fix (null/empty-host branch). This branch is now Q(lower_host__isnull=True) | Q(lower_host=""), so the (product, lower(host)) index serves it.

Re-run with DD_V3_FEATURE_LOCATIONS off and on, using the CI tag filters: test_importers_performance, test_endpoint_manager and test_import_reimport give 146 tests OK in each mode, with the perf counts unchanged. A first version that lowercased the hosts in a separate query added +1 to every perf count, which is why it is inline now. Ruff 0.16.9 is clean.

🤖 Generated with Claude Code

@Maffooch
Maffooch added this pull request to the merge queue Oct 7, 2026
Merged via the queue into dev with commit 9c6f9a7 Oct 7, 2026
49 checks passed
@Maffooch
Maffooch deleted the perf/endpoint-manager-scoped-lookup branch October 7, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants