Taught export to insure de-duplicated Artifact.json. #4161

ggainey · 2023-07-26T20:42:43Z

CHANGES/4159.bugfix

pulpcore/app/tasks/export.py

jpasqualetto · 2023-07-27T21:02:36Z

What's the impact of this change on fs-exports?

ggainey · 2023-07-28T00:24:22Z

What's the impact of this change on fs-exports?

That is a good question. fs-export is a different code-path aimed at a different end-output, but might benefit from what we've learned here - if not as part of this PR/issue. I'll review and open a new issue if it looks like we can follow a similar pattern there.

dralley · 2023-07-28T00:51:55Z

+1 to new issue / separate PR

ggainey · 2023-07-28T01:32:09Z

What's the impact of this change on fs-exports?

Taking a quick look at _export_to_filesystem, it looks like it is relying on hydrated Artifacts where it could get away with just knowing the Artifact.file. This is "mildly" suboptimal, but not a dealbreaker - going to the ddatabase at all takes most of your time, Artifacts aren't so large that "all fields" is horribly worse than just "a few fields".

I don't see a path that suffers the same duplication-effort - it doesn't do multiple-repos-at-once, the source of our Pain in this PR.

My first thought is that I wouldn't spend time trying to squeeze more performance out of that codepath, unless/until we actually see a problem.

dralley

LGTM, but please leave a TODO about maybe using .iterator() here

https://github.com/ggainey/pulpcore/blob/134610ff1eb48ce47fb06c0e5bae2f74dd815701/pulpcore/app/importexport.py#L104

pulpcore/app/tasks/export.py

mdellweg

I'd even be fine with merging this as is and promise to start on further refactoring right away with both ideas:

Using a db iterator
using qs.none()

Another thing (to think about later) that came to my mind: In the case of incremental, we now still include artifacts that are new to a repository version, but guaranteed to be in another (exported) repositories previous version. Could we safely skip them too?
Say the logic switching from:

$\displaystyle \bigcup_{\textrm{version}} \left( \textrm{version}.\textrm{artifacts} \setminus \textrm{prev}(\textrm{version}).\textrm{artifacts} \right)$

to

$\left( \displaystyle \bigcup_{\textrm{version}} \textrm{version}.\textrm{artifacts} \right) \setminus \left( \displaystyle \bigcup_{\textrm{version}} \textrm{prev}(\textrm{version}).\textrm{artifacts} \right)$

pulpcore/app/tasks/export.py

ggainey · 2023-07-28T14:34:22Z

I'd even be fine with merging this as is and promise to start on further refactoring right away with both ideas:

I am absolutely open to ways to make this better in the face of "many repos at once", as a future improvement. Open an issue, if you would, so we have at least a placeholder pointing back to this discussion.

ggainey · 2023-07-28T14:36:55Z

LGTM, but please leave a TODO about maybe using .iterator() here

https://github.com/ggainey/pulpcore/blob/134610ff1eb48ce47fb06c0e5bae2f74dd815701/pulpcore/app/importexport.py#L104

Clarify please? Do you mean just change to ...pb.iter(artifacts.iterator())? I feel liike I'm missing something .

ggainey · 2023-07-28T15:34:51Z

hold off on merging while I investigate some test weirdness, please

dralley · 2023-07-28T15:37:58Z

Clarify please? Do you mean just change to ...pb.iter(artifacts.iterator())? I feel liike I'm missing something .

iterate over artifacts.iterator(), increment the progressbar manually (and probably not one-at-a-time)

Along the way taught export to operate on a QuerySet of Artifacts instead of (prematurely) hydrating all affected Artifacts into a list. fixes pulp#4159.

ggainey · 2023-07-29T14:29:34Z

Clarify please? Do you mean just change to ...pb.iter(artifacts.iterator())? I feel liike I'm missing something .

iterate over artifacts.iterator(), increment the progressbar manually (and probably not one-at-a-time)

Gotcha - good call, results of doing so improve the time even more:

"started_at": "2023-07-29T14:06:46.511803Z",
"finished_at": "2023-07-29T14:16:42.599406Z", : 9m56s

patchback · 2023-07-29T15:06:18Z

Backport to 3.18: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.18/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4197

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-07-29T15:06:30Z

Backport to 3.21: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.21/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4198

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-07-29T15:06:43Z

Backport to 3.22: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.22/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4199

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-07-29T15:06:56Z

Backport to 3.23: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.23/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4200

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-07-29T15:07:09Z

Backport to 3.28: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.28/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4201

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-07-29T15:07:21Z

Backport to 3.29: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.29/617888748a85fa994be9258742d83ae3a7d4b034/pr-4161

Backported as #4202

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

mdellweg reviewed Jul 27, 2023

View reviewed changes

CHANGES/4159.bugfix Outdated Show resolved Hide resolved

mdellweg reviewed Jul 27, 2023

View reviewed changes

pulpcore/app/tasks/export.py Outdated Show resolved Hide resolved

dralley added backport-3.18 backport-3.21 backport-3.22 labels Jul 27, 2023

ggainey force-pushed the 4159_export_artifact_dups branch from 79140b8 to 134610f Compare July 28, 2023 00:13

ggainey marked this pull request as ready for review July 28, 2023 01:22

dralley approved these changes Jul 28, 2023

View reviewed changes

lubosmj reviewed Jul 28, 2023

View reviewed changes

pulpcore/app/tasks/export.py Show resolved Hide resolved

mdellweg approved these changes Jul 28, 2023

View reviewed changes

pulpcore/app/tasks/export.py Show resolved Hide resolved

mdellweg mentioned this pull request Jul 28, 2023

Investigate possible export performance and memory usage improvements. #4181

Open

ggainey marked this pull request as draft July 28, 2023 15:34

Taught export to insure de-duplicated Artifact.json.

fbb118c

Along the way taught export to operate on a QuerySet of Artifacts instead of (prematurely) hydrating all affected Artifacts into a list. fixes pulp#4159.

ggainey force-pushed the 4159_export_artifact_dups branch from 134610f to fbb118c Compare July 29, 2023 14:28

ggainey marked this pull request as ready for review July 29, 2023 14:29

ggainey merged commit 6178887 into pulp:main Jul 29, 2023
14 checks passed

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.18] Taught export to insure de-duplicated Artifact.json. #4197

Merged

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.21] Taught export to insure de-duplicated Artifact.json. #4198

Merged

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.22] Taught export to insure de-duplicated Artifact.json. #4199

Merged

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.23] Taught export to insure de-duplicated Artifact.json. #4200

Merged

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.28] Taught export to insure de-duplicated Artifact.json. #4201

Merged

patchback bot mentioned this pull request Jul 29, 2023

[PR #4161/61788874 backport][3.29] Taught export to insure de-duplicated Artifact.json. #4202

Merged

ggainey mentioned this pull request Jul 29, 2023

Taught perf-test to use rawhide instead of F36 repo. pulp/pulp_rpm#3218

Merged

ggainey deleted the 4159_export_artifact_dups branch February 2, 2024 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Taught export to insure de-duplicated Artifact.json. #4161

Taught export to insure de-duplicated Artifact.json. #4161

ggainey commented Jul 26, 2023

jpasqualetto commented Jul 27, 2023

ggainey commented Jul 28, 2023

dralley commented Jul 28, 2023

ggainey commented Jul 28, 2023

dralley left a comment •

edited

mdellweg left a comment

ggainey commented Jul 28, 2023

ggainey commented Jul 28, 2023

ggainey commented Jul 28, 2023

dralley commented Jul 28, 2023

ggainey commented Jul 29, 2023

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

Taught export to insure de-duplicated Artifact.json. #4161

Taught export to insure de-duplicated Artifact.json. #4161

Conversation

ggainey commented Jul 26, 2023

jpasqualetto commented Jul 27, 2023

ggainey commented Jul 28, 2023

dralley commented Jul 28, 2023

ggainey commented Jul 28, 2023

dralley left a comment • edited

Choose a reason for hiding this comment

mdellweg left a comment

Choose a reason for hiding this comment

ggainey commented Jul 28, 2023

ggainey commented Jul 28, 2023

ggainey commented Jul 28, 2023

dralley commented Jul 28, 2023

ggainey commented Jul 29, 2023

patchback bot commented Jul 29, 2023 • edited

Backport to 3.18: 💚 backport PR created

patchback bot commented Jul 29, 2023 • edited

Backport to 3.21: 💚 backport PR created

patchback bot commented Jul 29, 2023 • edited

Backport to 3.22: 💚 backport PR created

patchback bot commented Jul 29, 2023 • edited

Backport to 3.23: 💚 backport PR created

patchback bot commented Jul 29, 2023 • edited

Backport to 3.28: 💚 backport PR created

patchback bot commented Jul 29, 2023 • edited

Backport to 3.29: 💚 backport PR created

dralley left a comment •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited

patchback bot commented Jul 29, 2023 •

edited