Skip to content

perf(core): avoid rewriting all NULL-ecosystem rows during database maintenance - #8736

Open
Turbots wants to merge 1 commit into
dependency-check:mainfrom
Turbots:optimize-database-maintenance
Open

perf(core): avoid rewriting all NULL-ecosystem rows during database maintenance#8736
Turbots wants to merge 1 commit into
dependency-check:mainfrom
Turbots:optimize-database-maintenance

Conversation

@Turbots

@Turbots Turbots commented Aug 12, 2026

Copy link
Copy Markdown

Summary

Database maintenance after an NVD update rewrites every cpeEntry row with a NULL ecosystem on every run. The 'MULTIPLE' filter of UPDATE_ECOSYSTEM sits inside the correlated subquery, while the outer WHERE clause only checks e.ecosystem IS NULL. Rows whose vendor/product pair is unknown or mapped to MULTIPLE can never receive a value, yet H2 rewrites them with NULL-to-NULL updates each time — ~145k wasted row writes per maintenance run on a typical database.

Since the insert path (H2Functions.insertSoftware / CveDB.updateVulnerabilityInsertSoftware) already sets the ecosystem from the CpeEcosystemCache at insert time, the repair pass has very little real work to do.

Changes

  • dbStatements.properties: add an EXISTS guard to UPDATE_ECOSYSTEM so only rows that will actually receive an ecosystem value are updated (mirrors the approach already used by the MS SQL Server variant).
  • dbStatements_h2.properties: rewrite CLEANUP_ORPHANS as NOT EXISTS so H2 probes the idxSoftwareCpe index per row instead of materializing a full LEFT JOIN of cpeEntry against software inside an IN subquery.
  • CveDB.cleanupDatabase(): log the elapsed time of each maintenance statement to make future regressions visible.

Measurements

Full NVD cache rebuild (data feed mirror, H2 database, all years + modified):

before after
Updated the CPE ecosystem 144,969 records 14,080 records (883 ms)
Database maintenance total 189,482 ms 1,732 ms
Check for updates total 229,147 ms 47,638 ms

Verification

  • Verified both rewritten statements against H2 2.3.232 with fixture data: the update touches only fixable rows, leaves unknown/MULTIPLE rows NULL, and the orphan delete removes exactly the orphaned rows.
  • mvn -pl maven -am install builds successfully; ran a full rebuild plus scan through the built plugin against a real project.

🤖 Generated with Claude Code

…aintenance

The UPDATE_ECOSYSTEM statement updated every cpeEntry row with a NULL
ecosystem on every maintenance run. Rows whose vendor/product has no
usable cpeEcosystemCache entry (unknown or 'MULTIPLE') can never receive
a value, yet the statement rewrote them with NULL on each run - roughly
145k wasted row writes per update on H2.

- Add an EXISTS guard to UPDATE_ECOSYSTEM so only rows that will
  actually receive an ecosystem value are touched.
- Rewrite the H2 CLEANUP_ORPHANS as NOT EXISTS so it probes the
  idxSoftwareCpe index instead of materializing a full LEFT JOIN.
- Log the elapsed time of each maintenance statement.

Measured on a full NVD rebuild (H2): database maintenance dropped from
189,482 ms to 1,732 ms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@boring-cyborg boring-cyborg Bot added the core changes to core label Aug 12, 2026
@Turbots

Turbots commented Aug 12, 2026

Copy link
Copy Markdown
Author

Hello, first time contributor here!

Our team at KOR Financial (https://www.korfinancial.com) are actively using this plugin on a daily basis for every build on main branches, and our GitHub CI runners oftentimes have to re-download the NVD entries (we have a proxy for that so that's fine), but the database maintenance + defrag seemed to take very long on each run. Specifically on H2 there seemed to be many unnecessary updates, so I let my buddy Claude analyse the specifics and came up with these (quite astonishing) performance improvements.
We have over 400 production deployments a week and many more builds on our main branch across our repos, so this would be a huge timesaver for us!

Let me know if you want me to adjust something in the commit.

Also, not sure whether you prefer (or dislike) the Claude co-author in the commit message. Let me know and I can adjust.

@andrebrowne-kor

Copy link
Copy Markdown

@Turbots I had Claude take a look and...

Gaps and improvements

  1. The new timing log is invisible exactly when you'd want it (worth fixing). All three elapsed-time logs sit inside if (count > 0). Post-fix, a healthy steady-state run updates 0 rows — so the telemetry this PR adds to "make future regressions visible" will usually never print, and a regression that burns minutes doing zero-row work (precisely the failure class being fixed) logs nothing. Move the timing outside the count check, or log a debug line when count is 0.

  2. No automated regression test. Verification was manual against H2 fixtures. A test seeding three cpeEntry rows (cache match, no match, MULTIPLE match) and asserting executeUpdate() returns 1 and leaves the other two NULL would pin the semantics cheaply — these property-file statements are easy to break silently in future edits.

  3. Two siblings keep the slow/fragile pattern. The base CLEANUP_ORPHANS (inherited by MSSQL) and Oracle's variant still use NOT IN (SELECT cpeEntryId FROM software) — same materialization cost, plus the classic NOT IN NULL hazard (one NULL cpeEntryId and the delete silently matches nothing). Postgres already uses NOT EXISTS; standardizing the rest is a natural follow-up, even if out of scope here.

  4. Minor nits. The EXISTS guard duplicates the SET subquery verbatim, so matched rows do the cache lookup twice — cheap on a PK probe and the portable idiom, but an H2 MERGE INTO ... USING would do one pass if anyone cares later. The second timing variable is named ecosystemStart though it times the ecosystem removal statement. And System.nanoTime() is technically the right API for durations, though currentTimeMillis matches the surrounding code. The PR body's claim that it "mirrors the MS SQL Server variant" is true in effect (only touch matched rows) but not in form — MSSQL uses UPDATE ... FROM INNER JOIN, not an EXISTS guard.

None of these should block the merge — item 1 is the only one I'd ask the author to change in-PR; the rest are follow-ups.

@jmuntean09

jmuntean09 commented Aug 13, 2026

Copy link
Copy Markdown

Also, not sure whether you prefer (or dislike) the Claude co-author in the commit message. Let me know and I can adjust.

@Turbots I do prefer Claude to be there

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Optimizes H2 database maintenance and adds statement-level timing.

Changes:

  • Avoids no-op ecosystem updates.
  • Uses indexed orphan checks.
  • Adds maintenance timing logs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
dbStatements.properties Guards ecosystem updates with EXISTS.
dbStatements_h2.properties Replaces orphan cleanup with NOT EXISTS.
CveDB.java Records maintenance statement durations.
Suppressed comments (2)

core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java:1797

  • Orphan cleanup commonly has nothing to delete, but the query can still regress and become slow. Because this log remains behind count > 0, those executions have no per-statement duration; log the zero count and timing as well.
                    LOGGER.info("Cleaned up {} orphaned NVD records ({} ms)", count,
                            System.currentTimeMillis() - orphanStart);

core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java:1789

  • This timing also disappears when the statement changes zero rows, so a costly no-op UPDATE_ECOSYSTEM2 cannot be identified from the new per-statement logs. Log its count and elapsed time unconditionally.
                    LOGGER.info("Removed the CPE ecosystem on {} NVD records ({} ms)", count,
                            System.currentTimeMillis() - ecosystemStart);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1780 to +1781
LOGGER.info("Updated the CPE ecosystem on {} NVD records ({} ms)", count,
System.currentTimeMillis() - ecosystemStart);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core changes to core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants