fix: AggregateFITS leaked one file handle per HDU per result (crash at ~500 results)#1386
Merged
Merged
Conversation
_hdus opened the source fits once per requested HDU (a fresh fits.open via result.value) and the returned ImageHDUs kept the memmaps alive — 2 leaked fds/result measured; extract_fits crashes on "Too many open files" around ~500 results at the default ulimit. extract_csv leaked one handle per result the same way. Each source file is now opened once per result, data copied out of the memmap, and closed in a finally. 19% faster at 2 HDUs (interleaved A/B); output byte-identical. fd-count regression test. Found by the new aggregate_images/aggregate_fits profiling stages (#1385). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AggregateFITS(the fits_make catalogue workflow) leaked one file handle per requested HDU per result:_hduscalledresult.value(...)— a freshfits.open— for every HDU, and the returnedImageHDUs kept the memmaps (and so the handles) alive. Measured: 200 leaked fds for 100 results × 2 HDUs. At the defaultulimitof 1024,extract_fits/output_to_foldercrashes with "Too many open files" around ~500 results — the Euclid use case is 3000.extract_csvhad the same one-leak-per-result pattern.Fix: each source fits file is opened once per result (not once per HDU), extracted data is copied out of the memmap, and the source is closed deterministically. Verified: 0 leaked fds at 100 results, byte-identical output (201 HDUs), and 19% faster per result at 2 HDUs in an interleaved A/B (the win grows with HDU count — the workflow guide uses 4).
Found by the new
aggregate_images/aggregate_fitsprofiling stages (autofit_workspace_test PR) — the last uncovered leg of the #1375 catalogue workflow (csv was profiled and sped up in #1376; png/fits never were).AggregateImagesprofiled clean: ~1.5 ms/result, already caches one open per subplot type per result — no fruit taken there.API Changes
None — bug fix only: no behaviour change other than handles being closed (and
extract_csvtables being materialised rather than possibly memmap-backed).See full details below.
Test Plan
pytest test_autofit/— 1495 passed, 1 skippedtest_aggregate_fits.py::test_extract_fits_closes_files(fd counting, /proc-guarded)Full API Changes (for automation & release notes)
Changed Behaviour
AggregateFITS._hdus— onefits.openper unique source file per result; data copied out of the memmap; sources closed in afinally.AggregateFITS.extract_csv— source fits closed per result; returnedTables are materialised copies.Generated by the PyAutoLabs agent workflow.