Guard component/version/license upserts against concurrent insert races - #416
Merged
Conversation
persist_sbom_components runs the whole SBOM walk in one transaction, so two scans hitting the same purl or SPDX id at once make the loser's flush trip a unique constraint and abort the transaction, discarding every row staged ahead of it. The three get-or-create helpers now run their insert in a SAVEPOINT and re-fetch the winner on a unique violation, the same shape #290 gave the vulnerability catalog. Refs #398-A
session.get() returns Component | None; reusing the loop-scoped component name for it left mypy inferring the earlier non-optional type on reassignment.
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
1 participant
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.
Summary
_get_or_create_component,_get_or_create_component_version, and_get_or_create_licenseintasks/scan_source.pyeach did a plain"SELECT, then INSERT if missing".
persist_sbom_componentsruns theentire SBOM walk for a scan inside one transaction, committed once at
the end. Two scans hitting the same purl or SPDX id at the same time
both miss the lookup and both insert; the loser's flush trips the
unique constraint (
components.purl,component_versions.purl_with_version,or
licenses.spdx_id), Postgres aborts the transaction, and nothingcaught it, so the whole transaction was lost, including every
component/version/license the loop had already staged for that scan,
not just the one that collided.
This is the same shape PR #290 fixed for the vulnerability catalog
(ER8): the insert now runs inside a SAVEPOINT (
session.begin_nested()),and on a unique violation only that one statement is rolled back before
re-fetching the winner's row, so the rest of the caller's transaction
survives intact.
This is scoped to the race-safety fix only, not the batch-insert
performance work tracked separately in #398.
Test plan
tests/integration/scan/test_component_license_insert_race.py(3 tests) reproduces the race against a real Postgres for all three
helpers: a second session holds an uncommitted insert open on the
target purl/spdx_id while the first session, having already staged
an unrelated row in the same transaction, tries to upsert the same
identity. Verified each test fails against the pre-fix code with an
unhandled
IntegrityErrorand passes after the fix, with exactlyone row surviving and the row staged before the race still committed.
tests/unit/tasks/test_license_self_heal.py's fake sessionto support
begin_nested()(the "new license" branch now opens one).tests/unit/tasks(945 tests via the broader run) andtests/integration/scan(all scan pipeline tests, including F-2dedup, F-3 sanitisation, EOL/malicious stamping) pass unchanged.
tasks/scan_source.py: 92% (target 80%).Refs #398