Skip to content

Stream uid/hash dedupe candidates to bound reimport memory - #16042

Open
Maffooch wants to merge 1 commit into
bugfixfrom
fix/dedupe-candidates-stream-memory
Open

Maffooch wants to merge 1 commit into
bugfixfrom
fix/dedupe-candidates-stream-memory

Conversation

@Maffooch

Copy link
Copy Markdown
Contributor

Description

Symptom. Reimporting a large report into a product where many existing findings share the batch's hash codes or unique ids makes the post-processing task grow its resident memory steeply. A worker memory limit then fails the task in dedupe_batch_of_findings, under find_candidates_for_deduplication_uid_or_hash:

File "dojo/finding/helper.py", in post_process_findings_batch
File "dojo/finding/deduplication.py", in dedupe_batch_of_findings
...
File "dojo/finding/deduplication.py", in find_candidates_for_deduplication_uid_or_hash
    for ef in existing_qs:

Root cause. for ef in existing_qs: evaluates the candidate queryset in one go, which multiplies the memory:

  • The whole result set is fetched into the database client's buffer and cached on the queryset.
  • Each prefetch_related lookup (locations, vulnerability ids, CWEs, found_by, endpoint statuses) runs one query over every candidate id at once.
  • select_related("test", "test__engagement", "test__test_type") builds a separate Test, Engagement and Test_Type instance for every candidate row. Thousands of candidates from a handful of tests therefore carry thousands of identical copies of the same few rows.

Fix. The function's contract is unchanged: it still returns the same two maps, holding the same Finding instances in the same id order with the same prefetched relations.

  • Candidates are streamed with existing_qs.iterator(chunk_size=DEDUPE_CANDIDATE_CHUNK_SIZE) (1000). That means a server-side cursor, no result cache on the queryset, and prefetches issued per chunk instead of across every candidate.
  • As candidates stream in, they are pointed at one shared Test, Engagement and Test_Type instance per id. prefetch_related already shares instances per id this way. Every attribute value stays the same; only duplicate copies of identical rows are dropped.

The maps still have to hold every candidate instance, because the matcher walks them. So this removes the redundant copies around those instances, not the instances themselves.

Measured. 20,000 candidates share 50 hash codes. Each figure is peak RSS growth of one call, measured in a fresh process each time. Both runs gave identical numbers.

peak RSS growth
before (bugfix HEAD) 345 MB
streaming only 323 MB
streaming + shared select_related rows (this PR) 256 MB (about 26% less)

Test results

New unittests/test_dedupe_candidate_streaming.py. Its fixture has deliberate uid/hash collisions: several findings share one hash, a uid is shared by several findings, one finding carries both a colliding uid and a colliding hash, one is already a duplicate, one has the right uid from a different tool, and one is unrelated. The tests cover:

  • The returned maps are identical to the previous fully materialised evaluation, for chunk sizes 1, 2, 3 and 1000, in both deduplication and reimport mode.
  • The deduplication-mode result is spelled out explicitly (the duplicate is excluded, the other tool's uid never matches), and the candidates are still model instances with their prefetches.
  • The Finding candidate queryset is walked with iterator(chunk_size=...) and never materialised whole. QuerySet._fetch_all is never called for it.
  • Prefetch queries scale with the number of chunks, so no single prefetch spans every candidate.
  • Candidates from the same test share one Test / Engagement / Test_Type instance, and every relation still points at the right row.

Before the fix, the two streaming tests fail ([] != [2], 5 not greater than 5) and the two equivalence tests pass. With the fix, all of these pass locally:

  • test_dedupe_candidate_streaming
  • test_dedupe_injectable_scope
  • test_dedupe_location_prefetch
  • test_reimport_prefetch
  • test_deduplication_logic (85 tests, 3 skipped)
  • test_importers_performance (query counts unchanged)

ruff check is clean with the pinned ruff==0.16.5.

Documentation

No user-facing change.

Checklist

  • Rebased against the latest bugfix.
  • Bugfixes should be submitted against the bugfix branch.
  • Meaningful PR name.
  • Ruff compliant.
  • Python 3.13 compliant.
  • No model changes.
  • Unit tests added.

find_candidates_for_deduplication_uid_or_hash evaluated its candidate
queryset in one go. The whole result set was buffered by the database
client and cached on the queryset, each prefetch_related lookup spanned
every candidate id, and select_related built a separate Test, Engagement
and Test_Type instance for every candidate row. On a product where many
existing findings share the batch's hash codes or unique ids, that pushed
the post-processing task past a worker memory limit.

- Walk the candidates with iterator(chunk_size=DEDUPE_CANDIDATE_CHUNK_SIZE):
  a server-side cursor, no queryset result cache, and prefetches issued
  per chunk.
- Point each candidate at one shared Test / Engagement / Test_Type
  instance per id, as prefetch_related already does for its relations.
  Values are unchanged; only duplicate copies of identical rows are
  dropped.

The returned maps are unchanged: the same Finding instances, in the same
id order, with the same prefetches. For 20,000 colliding candidates, peak
RSS growth of the call drops from 345 MB to 256 MB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch Maffooch added this to the 3.3.300 milestone Sep 23, 2026
@Maffooch
Maffooch enabled auto-merge September 23, 2026 00:15

This branch has not been deployed

No deployments
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.

1 participant