Performance: speed up creating an override - #3068
Open
TheBeast85 wants to merge 2 commits into
Open
Conversation
gvmd re-planned nearly every statement it sent. With pg_stat_statements.track_planning turned on, get_targets with rows=-1 over 1000 targets showed where the time went: 10058 statements planning 403 ms execution 115 ms Planning cost three and a half times what executing did. track_planning is off by default, so pg_stat_statements reported only the 115 ms and the database looked harmless; the cgroup CPU told the real story, with postgres at 105 % of a core against gvmd's 13 %. * sql_pg.c gains a server side prepared statement cache. The existing _ps API only used PQexecParams, which sends the statement text every time and plans it every time. Now each statement is prepared once with PQprepare and run with PQexecPrepared, keyed by its text and cached per connection. Only statements that actually bind parameters are cached: the printf style calls put their values in the text, so every call would be a new key and a plan for "WHERE id = 4711" is of no use to "WHERE id = 4712". The cache is dropped wherever the connection goes away, including sql_close_fork, where the child abandons the parent's connection without closing it. A failed PQprepare is not an error, the caller falls back to PQexecParams. * manage_acl.c binds the resource value and the user UUID in the Super clause and in acl_user_owns_uuid. The Super clause was the single most expensive plan in gvmd, 241 ms of 387 ms. acl_user_owns_uuid runs once or twice for every row of every listing. * manage_sql_resources.c, manage_sql_tags.c and manage_sql_targets.c bind the UUID in find_resource_with_permission, the type, resource and parent type in resource_tag_count, and the target id in target_in_use. Verified: ACL isolation in both directions, a test user sees 0 of 1000 foreign targets but its own, and the owner does not see the test user's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Creating an override that is not tied to a task or a result affects every
report the overridden NVT appears in, and rebuilds the count cache for each
of them - "Auto Cache Rebuild" is the default. On a database with 1064 such
reports that took over nine seconds, and the cost is exactly linear in the
number of reports (measured from 1 to 1064 at a constant ~8.7 ms each).
Nothing here changes what ends up in report_counts. Verified by comparing
the cache for all affected reports character for character before and after:
the 691 rows for the overridden counts are identical.
* manage_pg.c only writes the session variables when the user actually
changes. manage_session_init costs three statements and was called about
eleven times per report, nearly always setting the session to the user it
was already on - 25 of the 60 statements per report. The cache lives in
sql_pg.c next to the prepared statement cache because it has the same
lifetime: it is dropped in sql_open, sql_close, sql_close_fork and in
sql_rollback. The rollback matters, SET SESSION is transactional. The
four places in manage_sql.c that write these variables directly drop it
themselves.
* manage_cert_loaded, manage_scap_loaded and manage_nvts_loaded ask
to_regclass instead of information_schema.tables. The latter is a view
over pg_class, pg_namespace and the privilege functions, and since gvmd
formats the schema and table name into the statement text it never reaches
the prepared statement cache, so PostgreSQL plans the whole view on every
call:
information_schema planning 1.503 ms execution 0.215 ms (21 plan rows)
to_regclass planning 0.035 ms execution 0.067 ms ( 3 plan rows)
init_result_get_iterator_severity asks for CERT every time it builds a
result iterator, so twice per report.
* report_cache_counts only rebuilds what is being invalidated. The iterator
yields a row for both values of override, but an override cannot change the
counts that ignore overrides, so when only the overridden counts are
cleared the other row is still correct and report_counts_id would do
nothing but read the cache back and throw it away. Callers that clear
neither still get every row.
One visible change: a missing row for the unoverridden counts used to be
created as a side effect of creating an override. It is not any more; it
appears on next access. The path without auto cache rebuild never did this
either, so the two are now consistent.
* init_get_iterator2_with takes an optional value to bind as $1, and
init_result_get_iterator_severity uses it for the report - in the extra
where clause and twice in the valid_overrides clause. That clause was the
most expensive statement in the whole call: 964 ms of the 2100 ms the
database spent, at only two calls per report, nearly all of it planning,
because the report rowid was formatted into the text and every report was
therefore a statement PostgreSQL had never seen. Bound, the text is the
same for every report, reaches the prepared statement cache and is planned
once - 964 ms becomes 75 ms.
The four branches now assemble the statement before running it, in one
place, so the bound and the formatted form cannot drift apart;
init_ps_iterator takes its argument as given and does not printf it. The
other callers are unchanged.
* create_tables adds indexes on overrides(nvt) and overrides(result_nvt).
The table only had id and uuid, while the two queries that run per report
select by nvt and result_nvt, so each one scanned the whole table. Over
133 reports, creating an override went from 0.854 s with no overrides
stored to 0.992 s with 255; it is flat now.
Measured on <create_override> over 1064 reports, median of 5:
before after
wall clock 9.334 s 5.076 s -46 %
statements per report 60.5 31.4
of those session swaps 25.0 4.0
database plan+execute 2.86 ms 1.06 ms -63 %
The cgroup CPU says where the rest is: postgres 4.82 s against gvmd's 0.51 s
per call, and only half of the postgres time is counted as planning and
executing. What is left is per statement overhead, so the way further is
fewer statements, not faster ones.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
auto-merge was automatically disabled
August 3, 2026 13:22
Head branch was pushed to by a user without write access
TheBeast85
force-pushed
the
perf/speed-up-create-override
branch
from
August 3, 2026 13:22
667a2d0 to
cde1078
Compare
Author
greenbonebot
enabled auto-merge (rebase)
August 3, 2026 13:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
to_regclassinstead ofinformation_schema.tablesinmanage_cert_loaded,manage_scap_loadedandmanage_nvts_loadedreport_cache_counts, only rebuild the counts that are being invalidatedoverridesonnvtand onresult_nvtWhy
Creating an override that is not tied to a task or a result affects every report the overridden NVT appears in and rebuilds the count cache for each one. With 1064 such reports that took over nine seconds, exactly linear in the number of reports.
Most of that was avoidable.
manage_session_initcosts three statements and ran about eleven times per report, nearly always setting the session to the user it was already on.information_schema.tablesis a view over the catalogue and the privilege functions, and since the table name is formatted into the text it was re-planned on every call, twice per report. The counts that ignore overrides cannot be changed by an override at all, so rebuilding them only read the cache back and discarded it. And thevalid_overridesclause carried the report id in its text, so every report was a statement PostgreSQL had never seen: 964 ms of the 2100 ms the database spent on the call, at only two calls per report.Now 5.076 s instead of 9.334 s, and 31 statements per report instead of 60.
One visible change: a missing row for the unoverridden counts is no longer created as a side effect of creating an override. It appears on next access, which is what the path without auto cache rebuild always did.
References
Depends on #3067
Checklist