Skip to content

Avoid checking a stale relationship #2949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
centosinfra-prod-github-app merged 1 commit into from
Jan 16, 2026
+7 −3
Merged

Avoid checking a stale relationship #2949

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packit_service/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3429,6 +3429,8 @@ def create(cls, run_model: "PipelineModel", ranch: str) -> "TFTTestRunGroupModel
test_run_group = cls()
test_run_group.ranch = ranch
session.add(test_run_group)
# Flush to get test_run_group.id before using it
session.flush()

# lock the run_model in the DB by using SELECT ... FOR UPDATE
# all other workers accessing this run_model will block here
Expand All @@ -3441,18 +3443,20 @@ def create(cls, run_model: "PipelineModel", ranch: str) -> "TFTTestRunGroupModel
logger.warning(f"PipelineModel with id={run_model.id} not found in DB.")
return test_run_group

if locked_run_model.test_run_group:
# Check test_run_group_id directly to avoid stale relationship cache.
# The relationship might not be loaded or might be stale from a different session.
if locked_run_model.test_run_group_id:
# Clone run model
new_run_model = PipelineModel()
new_run_model.project_event = locked_run_model.project_event
new_run_model.package_name = locked_run_model.package_name
new_run_model.srpm_build = locked_run_model.srpm_build
new_run_model.copr_build_group = locked_run_model.copr_build_group
new_run_model.koji_build_group = locked_run_model.koji_build_group
new_run_model.test_run_group = test_run_group
new_run_model.test_run_group_id = test_run_group.id
Copy link
Member

@nforro nforro Jan 16, 2026

Choose a reason for hiding this comment

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

Isn't this enough? Why keep the line above?

Copy link
Member Author

@majamassarini majamassarini Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think the real problem is the if test. We end up on the wrong branch of the if. And the same PipelineModel line is overriden by the "second" task. But I may be wrong.

Copy link
Member

@nforro nforro Jan 16, 2026

Choose a reason for hiding this comment

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

Sure, I get that, but my comment is about recording the relationship (in either branch). Both lines do effectively the same thing, don't they?

Copy link
Member Author

@majamassarini majamassarini Jan 16, 2026

Choose a reason for hiding this comment

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

Oh ok, now I get your comment. Ok I think you are right, I think that removing the line above is safe.

session.add(new_run_model)
else:
locked_run_model.test_run_group = test_run_group
locked_run_model.test_run_group_id = test_run_group.id
session.add(locked_run_model)

return test_run_group
Expand Down
Loading