Skip to content
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

Jump to bottom

Export artifact-dedup optimizations break pulp_rpm export. #4210

Closed
ggainey opened this issue Jul 31, 2023 · 3 comments · Fixed by #4211
Closed

Export artifact-dedup optimizations break pulp_rpm export. #4210

ggainey opened this issue Jul 31, 2023 · 3 comments · Fixed by #4211
Assignees
@ggainey
Labels
Issue prio-list Triage-Needed

Comments

@ggainey
Copy link
Contributor

ggainey commented Jul 31, 2023

Version
main, 18, 21, 22, 23, 28, 29

Describe the bug
After the changes for #4159 were released, pulp_rpm nightly tests started failing. This turns out to be the result of a subtle bug in the optimizations introduced for that fix.

This has to be fixed and released ASAP to all versions #4159 was releasxed into.

To Reproduce
@ggainey ggainey self-assigned this Jul 31, 2023
ggainey added a commit to ggainey/pulpcore that referenced this issue Jul 31, 2023
@ggainey
Reset export to using a set() for Artifacts.
5b83d59 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes pulp#4210.
@ggainey ggainey mentioned this issue Jul 31, 2023
Merged
ggainey added a commit to ggainey/pulpcore that referenced this issue Jul 31, 2023
@ggainey
Reset export to using a set() for Artifacts.
5e2a8d7 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes pulp#4210.
@ggainey
Copy link
Contributor Author

ggainey commented Jul 31, 2023

The specific test-failure that brought this to light was :

pulp [1aa88cb7-e5d6-4cb1-8890-f64b74fe80bc]: pulpcore.app.tasks.importer:ERROR: FATAL import-failure importing ./tmp5atvoa1n/repository-89e18c83-bc2b-4632-8bfb-45517d1e0504_1/pulpcore.app.modelresource.ContentArtifactResource.json
pulp [1aa88cb7-e5d6-4cb1-8890-f64b74fe80bc]: pulpcore.tasking.tasks:INFO: Task 0189a2e6-b8c5-72d5-b5e9-485bc9970581 failed (Artifact matching query does not exist.)

Artifacts needed by the ks-tree mentioned in the test were not exported correctly, and were therefore not being found at import-time.

@dralley
Copy link
Contributor

dralley commented Jul 31, 2023

Do you know why the queryset approach was interfering?

@ggainey
Copy link
Contributor Author

ggainey commented Jul 31, 2023

Do you know why the queryset approach was interfering?

I do not. It may be some interaction with the sub-repos of distribution-trees and multiple-repos, that's the only sequence that fails.

The current released behavior breaks the main-path workflow, and digging further into "why" was a lower priority than "make it work first". Replacing the approach with a set() means the current state of the attached PR is still significantly better than it has been for the past three years.

Maybe we want to do some more investigation "eventually" - but "stopping the bleeding Right Now" is today's priority.

dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
3194be5 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
abd22f1 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
2fcbb5a 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
6b66cbe 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
72aeb47 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
9f315b5 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
patchback bot pushed a commit that referenced this issue Jul 31, 2023
@ggainey @patchback
Reset export to using a set() for Artifacts.
acb5be6 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@patchback patchback bot mentioned this issue Jul 31, 2023
Merged
dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
b338c32 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
b9495cb 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
466198b 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
8e97a7e 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
ggainey added a commit that referenced this issue Jul 31, 2023
@ggainey
Reset export to using a set() for Artifacts.
78cd576 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
dralley pushed a commit that referenced this issue Jul 31, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
c2fbd25 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
@ggainey ggainey mentioned this issue Aug 1, 2023
Closed
dralley pushed a commit to dralley/pulpcore that referenced this issue Sep 12, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
9b03e1d 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes pulp#4210.

(cherry picked from commit 3194be5)
dralley pushed a commit that referenced this issue Sep 12, 2023
@ggainey @dralley
Reset export to using a set() for Artifacts.
138083f 
The QuerySet approach introduced a subtle bug the breaks pulp_rpm
export. If we want to retain the 'good parts' of that approach,
it will need to be its own investigation/PR.

fixes #4210.

(cherry picked from commit 3194be5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ggainey ggainey

Labels
Issue prio-list Triage-Needed
Projects
RH Pulp Kanban board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reset export to using a set() for Artifacts. ggainey/pulpcore
2 participants
@dralley @ggainey