Version
pulpcore 3.113.0, pulp_gem 0.7.5, filesystem storage, docker-compose deployment.
Describe the bug
When a distribution with a remote serves a path that doesn't map to content
(remote.get_remote_artifact_content_type() returns None — for pulp_gem that is
everything except gems/*.gem, notably the compact index files versions and
info/<gem>), Handler._stream_remote_artifact() downloads the file from the
remote into WORKING_DIRECTORY and never deletes it.
The finally: block only unlinks the temp file when the download failed:
https://github.com/pulp/pulpcore/blob/3.113.0/pulpcore/content/handler.py#L1331-L1336
and _save_artifact(), which otherwise takes ownership of the file, only runs
when save_artifact=True:
https://github.com/pulp/pulpcore/blob/3.113.0/pulpcore/content/handler.py#L1345
So a successful download with save_artifact=False leaves the file behind
forever. From reading the code the same should happen for a remote with
streamed policy even for content paths.
This gets bad quickly with bundler as the client: every bundle install sends a
ranged GET for /versions (rubygems.org's versions file is ~22 MB), pulp fetches
the whole file each time (#5144) and leaks it. With ~500 VMs running cinc-client
converges against our proxy we leak roughly 9 GB/hour and the volume fills up
within a day:
$ ls -lt /var/lib/pulp/tmp | head -5
-rw------- 1 700 700 22463870 Jul 3 21:15 tmpbmxulrn3-versions
-rw------- 1 700 700 22463870 Jul 3 21:15 tmpcjgy30uz-versions
-rw------- 1 700 700 22463870 Jul 3 21:15 tmp6imwb_7m-versions
-rw------- 1 700 700 22463870 Jul 3 21:14 tmphwdkhayy-versions
To Reproduce
- Create a gem repository, a remote for https://rubygems.org/ (policy
on_demand) and a distribution with that remote set (pull-through).
- Run
bundle install against the distribution a few times.
ls /var/lib/pulp/tmp — one ~22 MB tmp*-versions file per request.
Expected behavior
The temp file is removed once the response has been streamed to the client.
Additional context
Related to #5144 (metadata re-downloaded on every request) — this one is about
the disk leak, which turns that slowness into an outage. Happy to test a patch;
extending the finally block to also unlink when the file wasn't handed over to
_save_artifact looks sufficient.
Version
pulpcore 3.113.0, pulp_gem 0.7.5, filesystem storage, docker-compose deployment.
Describe the bug
When a distribution with a remote serves a path that doesn't map to content
(
remote.get_remote_artifact_content_type()returns None — for pulp_gem that iseverything except
gems/*.gem, notably the compact index filesversionsandinfo/<gem>),Handler._stream_remote_artifact()downloads the file from theremote into WORKING_DIRECTORY and never deletes it.
The
finally:block only unlinks the temp file when the download failed:https://github.com/pulp/pulpcore/blob/3.113.0/pulpcore/content/handler.py#L1331-L1336
and
_save_artifact(), which otherwise takes ownership of the file, only runswhen
save_artifact=True:https://github.com/pulp/pulpcore/blob/3.113.0/pulpcore/content/handler.py#L1345
So a successful download with
save_artifact=Falseleaves the file behindforever. From reading the code the same should happen for a remote with
streamedpolicy even for content paths.This gets bad quickly with bundler as the client: every
bundle installsends aranged GET for
/versions(rubygems.org's versions file is ~22 MB), pulp fetchesthe whole file each time (#5144) and leaks it. With ~500 VMs running cinc-client
converges against our proxy we leak roughly 9 GB/hour and the volume fills up
within a day:
To Reproduce
on_demand) and a distribution with that remote set (pull-through).
bundle installagainst the distribution a few times.ls /var/lib/pulp/tmp— one ~22 MBtmp*-versionsfile per request.Expected behavior
The temp file is removed once the response has been streamed to the client.
Additional context
Related to #5144 (metadata re-downloaded on every request) — this one is about
the disk leak, which turns that slowness into an outage. Happy to test a patch;
extending the finally block to also unlink when the file wasn't handed over to
_save_artifactlooks sufficient.