[Maffooch]

Description

A reimport with group_by set and JIRA keep-in-sync on (finding_jira_sync) read one finding group and its JIRA issue at a time in DefaultReImporter.process_groups_for_all_findings:

• for every group name in the report, findings[0].finding_group (one query) and then the group's JIRA issue inside is_keep_in_sync (one more);
• for every group of a reactivated or unchanged finding, its JIRA issue (one query each).

A reimport touching 200 groups ran 784 statements where 241 do the same work.

This PR batches those reads and changes nothing about what gets pushed:

• The first finding of every group name is loaded once, with select_related("jira_issue") and prefetch_related("finding_group_set__jira_issue"). Finding_Group orders by id, so finding.finding_group (finding_group_set.all().first()) is served from the prefetch.
• The groups of the reactivated and unchanged findings are read with select_related("jira_issue").
• Their ids now come from two reads driven by id lists, chunked like the close-old pass: the open findings among the ids, then those findings' memberships. Before, a DISTINCT subquery joining dojo_finding to dojo_finding_group_findings sat under the Finding_Group read. On a 10M-finding database whose group tables' planner statistics read as empty while they held rows, Postgres planned that as a nested loop that re-ran the subquery for each group. Each run probed dojo_finding_pkey with id = <membership's finding_id> AND id = ANY(<1,440 ids>) for every membership: 431,572 probes and 94 s for 200 groups. The new reads have no subquery, so that plan cannot happen. We produced those statistics with rolled-back benchmark runs plus an autovacuum, and have not seen them in production.

Which objects are pushed to JIRA, and in which order, is the same as before. That includes a lone ungrouped finding pushed on its own JIRA issue, and every group being pushed when push_to_jira is set.

Test results

New TestDojoReimportGroupJiraPerformance in unittests/test_importers_performance.py: a reimport through DefaultReImporter.process_scan with group_by and a keep-in-sync JIRA instance whose groups have issues, into 2 and into 10 groups with the same findings. With dev's default_reimporter.py it fails: 124 statements with 2 groups and 148 with 10. With this change both are the same, and both versions push the same groups.

New unittests/test_reimport_finding_group_jira_queries.py, which calls the pass directly:

• test_group_lookups_do_not_grow_with_the_number_of_groups: on dev, 13 queries for 2 groups and 85 for 20. With this change both are the same. It also checks every pushed object, in order.
• test_unchanged_groups_are_read_by_id_not_through_a_subquery: fails on dev (the IN (SELECT shape), passes with this change.
• test_ungrouped_finding_is_pushed_itself_when_kept_in_sync (with and without a JIRA issue) and test_push_to_jira_pushes_every_group_without_reading_issues pass on dev and with this change: they pin current behavior.

Run in the unit-test compose stack, on this branch merged with the latest dev: the two new test modules plus test_importers_auto_finding_groups, test_reimport_finding_group_duplicates, test_jira_import_and_pushing_api, test_importers_performance, test_reimport_close_old_findings_batch, test_import_reimport, test_importers_importer, test_jira_finding_group_perf, test_reimport_batch_flush, test_reimport_final_drain, test_reimport_group_jira_sync, test_reimport_prefetch, test_reimport_suppressed_location and test_reimporter_persist_seam: 312 tests, OK (1 skipped).

ruff check is clean.

Measured on a database of 10M findings, reimporting a 2.9 MB Semgrep report (2,520 findings; 1,440 unchanged, 1,080 new, 1,080 closed; 200 groups by title, each with a JIRA issue). Old and new ran in one process, alternating, 5 runs each:

	Wall ms, median (range)	SQL ms, median (range)	SQL statements
before	10,742 (10,423 to 10,833)	3,647 (3,333 to 4,086)	784
after	10,457 (10,113 to 11,125)	3,594 (3,439 to 4,149)	241

• After/before wall time was 0.973 (1-minute load 4.5 to 8.2). Four other comparisons, one of them by an independent reviewer, gave 0.948, 0.964, 0.969 and 0.970. SQL time stayed within noise in all of them, so the statement count is the firm result, and the wall-time gain is about 3 to 5%. At 1,800 groups the reviewer measured 5,414 statements before and 333 after.
• With the empty-looking statistics described above, "before" took 112 s in 2 of 3 runs and "after" took 10.4 to 10.7 s in all of them. The reviewer held that state fixed and measured 113.8 s before against 11.3 s after (5 of 5 runs). They also saw autovacuum reach it on its own during a reimport on near-empty group tables, as on an instance that has only just started using group_by.
• The same 300 JIRA pushes were dispatched in the same order by the group pass in every comparison, and the groups (200) and memberships (3,600) were equal.

Documentation

No user-facing change.

🤖 Generated with Claude Code

[Maffooch]

Independent review of this change (I did not write it). Text only.

Regression tests. Run in a private unit-test container: with this PR's dojo/importers/default_reimporter.py, all 5 tests in unittests/test_reimport_finding_group_jira_queries.py pass. With dev's copy of that file mounted over it, 2 fail as intended (13 != 85 queries for 2 vs 20 groups, and the IN (SELECT group read), and the 3 behavior pins pass.

Same results as dev. On a 10M-finding database I ran dev's process_groups_for_all_findings and this PR's against the same import and reimport, each in a rolled-back transaction, across 22 shapes:

• every group_by option;
• JIRA keep-in-sync on and off, no JIRA at all, and push_to_jira;
• groups with, without and with deleted JIRA issues, and findings with their own issues;
• a group in another test holding this test's findings, findings in two groups, and a reimport that regroups by a different field;
• reactivated findings, lone ungrouped findings, archived groups;
• per-finding and bulk creation of new findings;
• 1,800 group names, so both id lists are chunked.

In every shape the groups pass pushed the same objects in the same order, and the groups, memberships, JIRA issue rows, finding states, alerts and history rows were identical. On entry to the pass, no first finding had finding_group or jira_issue cached on the in-memory instance, so reading it fresh from the database changes nothing.

Measurements (5 runs per side, alternating in one process):

	Statements	Wall time, new / old
200 groups, fresh planner statistics	784 to 241	0.970 (10,946 ms to 10,612 ms medians)
200 groups, group tables at reltuples 0 / relpages > 0	785 to 241	0.099 (113.8 s to 11.3 s; old hit the slow plan in 5 of 5)
1,800 groups	5,414 to 333	not timed

The stale state happened without any help, from an autovacuum running while a reimport's uncommitted rows held pages in near-empty group tables. So it could plausibly happen on an instance that has just started using group_by.

One suggestion. The new tests call process_groups_for_all_findings on hand-built importer state. A request-level case in unittests/test_importers_performance.py would pin the path users actually hit: a reimport with group_by and a keep-in-sync JIRA instance whose groups have issues, at two group counts, with the statement count not growing with the number of groups.

[Maffooch]

Resolution of the review:

• R1: fixed in 8a73ca5 (ruff follow-up b37403d). TestDojoReimportGroupJiraPerformance in unittests/test_importers_performance.py reimports through DefaultReImporter.process_scan with group_by and a keep-in-sync JIRA instance whose groups have issues, into 2 and into 10 groups with the same findings. With dev's default_reimporter.py it fails: 124 statements with 2 groups and 148 with 10. With this change it passes, and both versions push the same groups. The branch is merged with the latest dev (7059b12), and 312 neighbouring tests pass on it (1 skipped).
• R2 to R5 are about the paired benchmark change, not this code. The description here now also carries the reviewer's figures: 0.970 wall with fresh statistics, 0.099 with stale ones, and 5,414 to 333 statements at 1,800 groups.

Found while writing the test, and unchanged by this PR: a reimport names a new finding's group from the parsed title, while the import named the group from the saved, title-cased title. So a report whose titles are not already title-cased puts the reimport's new findings into a second group with the same name. The new test uses title-cased titles.