Use a more appropriate compression level for exports #3884

dralley · 2023-05-25T15:28:46Z

Exports will be larger, but should be much faster. There were reports of large exports taking multiple days to complete due to the overhead incurred by compression.

closes #3869

dralley · 2023-05-25T15:29:09Z

Pending discussion on https://bugzilla.redhat.com/show_bug.cgi?id=2188504

ggainey

Holy cow - I love this idea. Addresses the problem, without requiring a whole new config/API/workflow change - brilliant!

ipanova

I like the solution 🦭

ipanova · 2023-05-25T16:19:35Z

bit it seems like the tests is failing E pulp_smash.pulp3.bindings.PulpTaskError: (PulpTaskError(...), "Pulp task failed (__init__() got an unexpected keyword argument 'compresslevel')")

ipanova · 2023-05-25T16:27:18Z

@dralley did you ran any tests to come to the conclusion that compression level 1 is the most balanced? there is the whole range of 1-9 levels

ggainey · 2023-05-25T16:32:19Z

bit it seems like the tests is failing E pulp_smash.pulp3.bindings.PulpTaskError: (PulpTaskError(...), "Pulp task failed (__init__() got an unexpected keyword argument 'compresslevel')")

Oh bah - looks like compresslevel isn't supported on the streaming options. From the tarfile doc:

"For modes 'w:gz', 'r:gz', 'w:bz2', 'r:bz2', 'x:gz', 'x:bz2', tarfile.open() accepts the keyword argument compresslevel (default 9) to specify the compression level of the file."

dralley · 2023-05-25T16:55:01Z

I checked https://github.com/python/cpython/blob/main/Lib/tarfile.py#L1826 first, it looks like it should be supported...

It looks like this was only merged a year ago and therefore might not be in the Python versions we're using python/cpython@50cd4b6

dralley · 2023-05-25T18:00:52Z

@dralley did you ran any tests to come to the conclusion that compression level 1 is the most balanced? there is the whole range of 1-9 levels

I didn't run tests but I did look over some general compression benchmarks that others have done. Level 3 was 30-40% slower but only about 10% smaller, and anything beyond that was an even worse tradeoff. My feeling is that a lot of the content that people want to export like RPMs, metadata, etc. are already compressed so we don't need to try hard to compress it again.

ipanova · 2023-05-26T07:47:30Z

@dralley did you ran any tests to come to the conclusion that compression level 1 is the most balanced? there is the whole range of 1-9 levels

I didn't run tests but I did look over some general compression benchmarks that others have done. Level 3 was 30-40% slower but only about 10% smaller, and anything beyond that was an even worse tradeoff. My feeling is that a lot of the content that people want to export like RPMs, metadata, etc. are already compressed so we don't need to try hard to compress it again.

Gotcha, thanks for explanation. It makes sense.

ggainey

Daniel, might it be possible to subclass "just enough" of Tarfile to support this, and carry just that? As opposed to needing all 3K lines of tarfile.py?

dralley · 2023-05-30T05:29:21Z

@ggainey I agree and that is the intention - this is still in draft remember - but until then I just wanted a sense of whether it would work or not.

stale

dralley · 2023-05-31T05:36:39Z

Well, it works. However, is this something we're actually open to doing? We have explicitly tried to avoid doing monkeypatches or this kind of vendoring in the past. I think this solution would yield the best outcome (relative to no compression at all, or something like that) but I still definitely feel discomfort about it at a technical / maintenance level.

Am I missing an easier way to accomplish this?

@bmbouter @mdellweg @ipanova @ggainey

ggainey

I think this approach is the only reasonable way to address the problem for the actually-affected users. It's definitely "black magic", and needs some Very Explicit Comments to make it clear what's going on and why it works - but I can't think of a better approach.

dralley · 2023-06-08T03:42:25Z

I believe tar | gz | split would work - but I haven't tried it to know Absolutely For Sure. It's definitely worth the experiment. The important thing, tho, is "whatever we do in export, needs to be consumable in the current import code", so people don't have to recreate their exports.

Experimenting with this has been successful, although expressing it in code is a bit more challenging, but I want to double check -- is gzip-the-command-line-tool available on all systems we care about? Most distros have it installed by default, but a lot of container images do not.

dralley · 2023-06-09T01:40:27Z

I take it back, something about the data pipeline gets messed up and it's a real pain to debug. Combined with uncertainty about relying on the gzip cli tool, I think we should just continue with the original implementation.

mdellweg · 2023-06-09T10:15:53Z

pulpcore/app/monkeypatch.py

+        def __init__(self, *args, **kwargs):
+            super().__init__(*args, **kwargs)


Exactly if we do not add any init code, then just don't add these two lines.

mdellweg · 2023-06-09T10:19:32Z

CHANGES/3869.bugfix

@@ -0,0 +1 @@
+Exports now use gzip compression level 1 rather than compression level 9. Exported archives will now be slightly larger, but exports should be much faster. This is considered to be a more optimal balance of space/time for the export operation.


To make some of our teammates happy, this ought to be line broken at 98 characters.

(100 is the rule, but towncrier indents by another two.)

mdellweg · 2023-06-09T10:20:56Z

pulpcore/app/monkeypatch.py

@@ -0,0 +1,69 @@
+import sys


That is not how i learned python, but maybe django does some auto lookup?

Exports will be larger, but should be much faster. There were reports of large exports taking multiple days to complete due to the overhead incurred by compression. closes pulp#3869

patchback · 2023-06-13T11:10:35Z

Backport to 3.18: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.18/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #3917

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

patchback · 2023-06-13T11:10:49Z

Backport to 3.21: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.21/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #3918

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

patchback · 2023-06-13T11:11:03Z

Backport to 3.22: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.22/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #3919

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

patchback · 2023-06-13T11:11:17Z

Backport to 3.23: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.23/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #3920

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

B-Woody · 2023-06-14T01:45:54Z

I submitted the original Red Hat BZ relating to this issue and also for #3236 . Passing on my thanks to everyone who worked on this. It's going to be a huge help for some of the work we're doing.

patchback · 2023-08-01T00:47:10Z

Backport to 3.28: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.28/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #4220

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

patchback · 2023-09-13T14:52:15Z

Backport to 3.16: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.16/34c5babec5b9503dbd627bebb75cbc0403fe2ae7/pr-3884

Backported as #4425

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

dralley force-pushed the compression branch from f6beb18 to 5cc7f8b Compare May 25, 2023 15:29

dralley added backport-3.18 backport-3.22 labels May 25, 2023

ggainey approved these changes May 25, 2023

View reviewed changes

dralley force-pushed the compression branch from 5cc7f8b to ccf3d82 Compare May 25, 2023 15:35

dralley added the backport-3.21 label May 25, 2023

ipanova previously approved these changes May 25, 2023

View reviewed changes

dralley force-pushed the compression branch from ccf3d82 to 78ed456 Compare May 25, 2023 17:54

dralley force-pushed the compression branch from aba3ade to bb67da5 Compare May 25, 2023 18:42

ggainey reviewed May 26, 2023

View reviewed changes

dralley force-pushed the compression branch 2 times, most recently from 53bebbf to efbbb80 Compare May 30, 2023 05:50

dralley force-pushed the compression branch 2 times, most recently from 5efa600 to e372173 Compare May 31, 2023 05:34

dralley force-pushed the compression branch 4 times, most recently from ddc442f to 7b58832 Compare May 31, 2023 14:08

ggainey reviewed May 31, 2023

View reviewed changes

dralley force-pushed the compression branch from 6c052b9 to a1aab3f Compare June 9, 2023 01:39

dralley marked this pull request as ready for review June 9, 2023 01:40

dralley requested a review from mdellweg June 9, 2023 01:40

mdellweg reviewed Jun 9, 2023

View reviewed changes

Use a more appropriate compression level for exports

6276213

Exports will be larger, but should be much faster. There were reports of large exports taking multiple days to complete due to the overhead incurred by compression. closes pulp#3869

dralley requested a review from mdellweg June 9, 2023 14:55

dralley force-pushed the compression branch from a1aab3f to 6276213 Compare June 9, 2023 14:55

mdellweg approved these changes Jun 9, 2023

View reviewed changes

ggainey approved these changes Jun 12, 2023

View reviewed changes

ipanova approved these changes Jun 13, 2023

View reviewed changes

ipanova merged commit 34c5bab into pulp:main Jun 13, 2023

patchback bot mentioned this pull request Jun 13, 2023

[PR #3884/34c5babe backport][3.18] Use a more appropriate compression level for exports #3917

Merged

patchback bot mentioned this pull request Jun 13, 2023

[PR #3884/34c5babe backport][3.21] Use a more appropriate compression level for exports #3918

Merged

patchback bot mentioned this pull request Jun 13, 2023

[PR #3884/34c5babe backport][3.22] Use a more appropriate compression level for exports #3919

Merged

patchback bot mentioned this pull request Jun 13, 2023

[PR #3884/34c5babe backport][3.23] Use a more appropriate compression level for exports #3920

Merged

dralley deleted the compression branch June 14, 2023 02:23

dralley added the backport-3.28 label Aug 1, 2023

patchback bot mentioned this pull request Aug 1, 2023

[PR #3884/34c5babe backport][3.28] Use a more appropriate compression level for exports #4220

Closed

dralley mentioned this pull request Sep 4, 2023

Compression monkeypatch is not always being loaded #4351

Closed

ipanova added the backport-3.16 label Sep 13, 2023

patchback bot mentioned this pull request Sep 13, 2023

[PR #3884/34c5babe backport][3.16] Use a more appropriate compression level for exports #4425

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a more appropriate compression level for exports #3884

Use a more appropriate compression level for exports #3884

dralley commented May 25, 2023

dralley commented May 25, 2023

ggainey left a comment

ipanova left a comment

ipanova commented May 25, 2023

ipanova commented May 25, 2023

ggainey commented May 25, 2023

dralley commented May 25, 2023

dralley commented May 25, 2023 •

edited

Loading

ipanova commented May 26, 2023

ggainey left a comment

dralley commented May 30, 2023

dralley commented May 31, 2023 •

edited

Loading

ggainey left a comment

dralley commented Jun 8, 2023

dralley commented Jun 9, 2023 •

edited

Loading

mdellweg Jun 9, 2023

mdellweg Jun 9, 2023

mdellweg Jun 9, 2023

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

B-Woody commented Jun 14, 2023

patchback bot commented Aug 1, 2023 •

edited

Loading

patchback bot commented Sep 13, 2023 •

edited

Loading

		def __init__(self, args, *kwargs):
		super().__init__(args, *kwargs)

		@@ -0,0 +1 @@
		Exports now use gzip compression level 1 rather than compression level 9. Exported archives will now be slightly larger, but exports should be much faster. This is considered to be a more optimal balance of space/time for the export operation.

Use a more appropriate compression level for exports #3884

Use a more appropriate compression level for exports #3884

Conversation

dralley commented May 25, 2023

dralley commented May 25, 2023

ggainey left a comment

Choose a reason for hiding this comment

ipanova left a comment

Choose a reason for hiding this comment

ipanova commented May 25, 2023

ipanova commented May 25, 2023

ggainey commented May 25, 2023

dralley commented May 25, 2023

dralley commented May 25, 2023 • edited Loading

ipanova commented May 26, 2023

ggainey left a comment

Choose a reason for hiding this comment

dralley commented May 30, 2023

dralley commented May 31, 2023 • edited Loading

ggainey left a comment

Choose a reason for hiding this comment

dralley commented Jun 8, 2023

dralley commented Jun 9, 2023 • edited Loading

mdellweg Jun 9, 2023

Choose a reason for hiding this comment

mdellweg Jun 9, 2023

Choose a reason for hiding this comment

mdellweg Jun 9, 2023

Choose a reason for hiding this comment

patchback bot commented Jun 13, 2023 • edited Loading

Backport to 3.18: 💚 backport PR created

patchback bot commented Jun 13, 2023 • edited Loading

Backport to 3.21: 💚 backport PR created

patchback bot commented Jun 13, 2023 • edited Loading

Backport to 3.22: 💚 backport PR created

patchback bot commented Jun 13, 2023 • edited Loading

Backport to 3.23: 💚 backport PR created

B-Woody commented Jun 14, 2023

patchback bot commented Aug 1, 2023 • edited Loading

Backport to 3.28: 💚 backport PR created

patchback bot commented Sep 13, 2023 • edited Loading

Backport to 3.16: 💚 backport PR created

dralley commented May 25, 2023 •

edited

Loading

dralley commented May 31, 2023 •

edited

Loading

dralley commented Jun 9, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Jun 13, 2023 •

edited

Loading

patchback bot commented Aug 1, 2023 •

edited

Loading

patchback bot commented Sep 13, 2023 •

edited

Loading