BP5Reader: close metadata FDs when writer is inactive #4846
+318
−15
Conversation
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
eisenhauer
force-pushed
the
close-metadata-fds
branch
from
b9b1b63 to
120382c
Compare
eisenhauer
force-pushed
the
close-metadata-fds
branch
from
120382c to
0716ab7
Compare
When the writer's active flag is false, all metadata is in its final state. Close each metadata file descriptor individually once all of its data has been read into memory: - mmd.0: always read in full, closed immediately - md.idx: closed when ParseMetadataIndex has consumed all entries (respects the 16 MB streaming-mode limit across multiple calls) - md.0: closed once all index entries are parsed, since every metadata section they reference has already been read This reduces file descriptor pressure on remote servers (e.g. XRootD) when many datasets are opened concurrently via campaign files. Also adds an fstat retry loop for md.idx to handle filesystem metadata lag: after a writer closes a file, fstat() may briefly report a stale size. The loop polls until the reported size stabilizes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
eisenhauer
force-pushed
the
close-metadata-fds
branch
from
0716ab7 to
162fb48
Compare
anagainaru
approved these changes
Feb 8, 2026
Comment on lines
1447
to
+1448
| m_MetadataIndex.Reset(true, false); | ||
| if (m_Comm.Rank() == 0) | ||
| m_MetadataIndex.m_Buffer.resize(0); |
Member
There was a problem hiding this comment.
Can you do a reset of the metadata index without resizing the buffer to 0? In no, I would put the resize inside the reset function
Member
Author
There was a problem hiding this comment.
Most of the time we call Reset and want the buffer to survive. Here, the next bit of code needs to see the buffer as size zero, so we can't really move it inside (without adding something like a parameter to say we wanted the buffer to go away or something).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When the writer's active flag is false, all metadata is in its final state. Close each metadata file descriptor individually once all of its data has been read into memory:
This reduces file descriptor pressure on remote servers (e.g. XRootD) when many datasets are opened concurrently via campaign files.
Also adds an fstat retry loop for md.idx to handle filesystem metadata lag: after a writer closes a file, fstat() may briefly report a stale size. The loop polls until the reported size stabilizes.
Finally, it adds a test, counting open FDs to make sure that the metadata FDs are actually closed. This test only functions on non-windows platforms.