Skip to content

Pull-through content app never deletes the temp file for non-content paths (fills WORKING_DIRECTORY) #7846

Description

@c0d33p

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

  1. Create a gem repository, a remote for https://rubygems.org/ (policy
    on_demand) and a distribution with that remote set (pull-through).
  2. Run bundle install against the distribution a few times.
  3. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions