search

cybozu-go / aptutil

Go utilities for Debian APT repositories
MIT License
124 stars 29 forks source link

No cleanup for temporary directories if sync fails for them #61

Open lcsongor opened 2 years ago

lcsongor commented 2 years ago

Description

Only directories that have been linked to the repo-id are cleaned up.

Reproducible? yes, but hard

Time of the issue

Randomly

System

Own test systems

Software version

go-apt-mirror: v1.4.2

Prerequisites

Not really clear why a sync would fail to an apt repo, but the causes could be unreliable IP connectivity or incorrectly configured upstream apt repos.

Behavior

Steps to reproduce

Start synchronizing an apt repository to which there is bad IP connectivity.

Expected behavior

Should the synchronization fail, a message is displayed (it is) and the incorrectly downloaded apt repository is deleted from the disk.

Actual behavior

The incorrectly and incompletely downloaded apt directories are laying around and slowly taking up disk space, no mechanism takes care of cleaning them.

Corrective actions taken

Delete incomplete downloaded apt directories.

Fix suggestion:

Implement a mechanism into the code that would keep a number of [configurable] temporary downloaded directories, and start wiping the older ones.

Hsn723 commented 2 years ago

Could you provide more information (logs, etc.)? Normally directories should be GC'ed after a successful run, and the GC process deletes any directory that is not a symlink or is not symlinked to, so incorectly/incompletely downloaded apt directories should be deleted as part of this process.

One possible way the actual behavior you describe could happen is if a correct configuration is never set and the update loop continuously fails (and thus never calls GC). In such cases, fixing the configuration file to use a proper apt repo should clean up old data on the first successful run. Can you confirm?

lcsongor commented 2 years ago

Exactly! You need a successful run in order to get the temporary directory GCed. I guess the fix is as simple as to add another call of the GC before the return at https://github.com/lcsongor/aptutil/blob/fix%2361/mirror/control.go#L142 and the disk won't fill up.

Hsn723 commented 2 years ago

Thanks for confirming my suspicions. Though this can easily be addressed on the user's side by fixing the configuration, I can see how prolonged network connectivity issues could exacerbate the issue. It is indeed a simple fix, so although I plan on addressing it, I can't comment on any timeline as this project is in maintenance mode.

lcsongor commented 2 years ago

@Hsn723 thanks for your response. I will was about to open a pull request with the fix but you were faster.