Refactor zip creation to reduce peak disk usage

[arobson-ods]

Addresses #173.

What happened

2026-09-16 21:43:49

FileNotFoundError: [Errno 2] No such file or directory:
  '/tmp/bulk-data-service-zip-2/iati-data-main/data/prosperglobaluk/mce-activity.xml'

  File ".../zippers.py", line 196, in create_empty_files_for_non_downloadable_datasets
    open(dataset_filename, "w").close()
  File ".../zippers.py", line 175, in prepare
  File ".../zipper.py", line 79, in zipper_run
  File ".../checker.py", line 35, in checker_service_loop

This was the only FileNotFoundError in the 30 day log. (Line numbers are from the deployed build, not this branch.)

create_empty_files_for_non_downloadable_datasets only creates the publisher directory for datasets with no download, but the line creating the placeholder file sits outside that check and runs for every dataset. So a dataset flagged as downloaded gets no folder of its own; if its XML is also missing, open() fires on a path whose directory does not exist. prosperglobaluk has one dataset, so nothing else had created that folder.

Raised from prepare(), after the copytree. There was no try/finally in zipper_run, so it escaped, skipped the clean_working_dir() at the end of the loop, and left the whole 12.7 Gb -2 directory on disk.

22:52:56 — 69 minutes later, the first OSError: [Errno 28] No space left on device. Then 135 out-of-disk failures over five and a half days, ending when the container was restarted.

What Changed

• Creates the publisher directory unconditionally, fixing the crash above.
• Deletes the files a ZIP was built from as soon as the ZIP exists, before extracting it to verify.
• Removes each format's working directory whether or not the run completes, so a failure cannot strand it.
• valid_zip_created() catches OSError as well as BadZipFile and removes the extraction in a finally, so no error leaves a part-extracted copy. It also opens the archive inside the try, so a truncated or missing ZIP is a failed verification rather than a raise.
• A forced full clean re-downloads the XML it deleted. clean_working_dir() wiped every XML file but left datasets_in_working_dir untouched, so every dataset still compared as present and unchanged and nothing was re-downloaded. Separate bug, and no sign it ever fired. But it cannot be deferred. The forced clean fires when valid_zip_created() returns False, and this PR widens that from "BadZipFile inside extractall" to "any BadZipFile or OSError". In 30 days of logs the old condition never fired; the new one would have fired 135 times, all during the September incident. So the path goes from never taken to taken on every disk-full failure and without this fix, each one publishes a near-empty ZIP over the good one.
• Both attempts failing is reported as an error. Previously nothing was logged and the run recorded its duration as normal.
• Disk usage is logged at each stage and exported as disk_free_bytes and disk_used_bytes.

Testing

• 268 tests pass, 13 new. tests/unit/test_zippers.py is new and needs no Docker; there were previously no unit tests for either zipper module.
• black, isort, flake8, mypy clean; pyright clean on changed files.
• End to end locally: for both formats the source directory goes before verification, and only the master directory survives.
• Again with extractall patched to raise ENOSPC: the failure is reported, the re-try re-downloads and rebuilds with real data, and nothing is left behind.

Notes for reviewer

• Kept extractall rather than zf.testzip(). Considered using testzip as it verifies with no disk writes, but with the staged copy freed first the peak is the same either way. It sits where the archive is written, not at verification. Making this switch might be worthwhile if process would benefit from a performance improvement.

Follow-up, not in this PR

Why the XML was missing at all. No blob fetch failed on the 16th, so the file was most likely deleted locally and never re-fetched. clean_working_dir deletes keyed on <reporting_org_short_name>/<short_name>, but new_or_updated_datasets only re-downloads on a changed short_name or hash. reporting_org_short_name is never compared. A dataset whose publisher path changes therefore loses its XML with nothing to bring it back. Unconfirmed (reporting_org_sync.py has no logging) but it fits: the crash path was prosperglobaluk/mce-activity.xml, and the dataset is now prosperglobaluk-activity.

[simon-20]

Thanks Andrew. Could you add an issue to cover that Follow Up point you mentioned?

[simon-20]

Could you swap usage.free to be usage.available - I think that is more accurate, because some of the free value is reserved for root.

[arobson-ods]

Thanks @simon-20. Agree 'available' more accurate than 'free'. Do you mean I should do total, used, available = shutil.disk_usage(context["ZIP_WORKINGDIR"])? Also, should I apply same logic to the prometheus metric i.e. disk_free_bytes becomes disk_available_bytes?