fix: prevent NFS handle eviction during reads and parallelize per-inode reads#80
Draft
fix: prevent NFS handle eviction during reads and parallelize per-inode reads#80
Conversation
…de reads Bug #6: Add pin_count to HandlePool entries. get() pins the handle, preventing LRU eviction while an async read is in flight. The caller unpins after the operation completes. Eviction skips pinned entries, and the pool grows temporarily if all entries are pinned. This eliminates the TOCTOU window between get_or_open_handle and read that could cause EBADF under heavy load (>64 concurrent files). Bug #7: Use try_lock() on the per-handle prefetch mutex. When contended (another reader holds the lock on the same NFS inode), fall back to a direct range download that bypasses prefetch entirely. This prevents two NFS clients reading different parts of the same file from serializing behind each other's network I/O, and avoids one reader's seek pattern disrupting the other's prefetch window.
- try_read_direct returns None for non-xet files (plain git/LFS) so the caller falls back to waiting for the prefetch lock instead of silently returning empty data - Use get_file_entry() instead of manual inode table lookup - Add HandlePool::pin() method instead of reaching into handles directly - Remove duplicate comment
POSIX Compliance (pjdfstest) |
Benchmark Results |
1. Reinstate EBADF retry in NFS read: remove()/rename can delete a handle while pinned (pins only prevent LRU eviction). On EBADF, purge the stale entry and retry once with a fresh handle. 2. Use open-time xet_hash/file_size snapshot in try_read_direct instead of re-reading from the inode table. Prevents the contended read path from seeing a different file revision than the prefetch path. 3. Return EIO on premature stream EOF in try_read_direct instead of a short read, matching the invariant enforced by read_with_prefetch (FUSE fuse_read_update_size shrinks i_size on short reads). 4. Evict multiple entries in HandlePool::insert() to bring the pool back to capacity after pinned overflow bursts. Previously the pool grew permanently after a burst of pinned reads.
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
Fixes the last two bugs from the REVIEW.md audit (PR #69):
Bug 6 (NFS handle pool TOCTOU): Adds
pin_counttoHandlePoolentries.get()pins the handle so it cannot be LRU-evicted while an async read is in flight. The caller unpins after the operation completes. Eviction skips pinned entries and the pool grows temporarily if all entries are pinned. Eliminates the TOCTOU window betweenget_or_open_handleandreadthat caused EBADF under heavy load (>64 concurrent files).Bug 7 (NFS read serialization): Uses
try_lock()on the per-handle prefetch mutex. When contended (another reader holds the lock on the same NFS inode), falls back to a direct range download that bypasses prefetch entirely. This prevents two NFS clients reading different parts of the same file from serializing behind each other's network I/O, and avoids one reader's seek pattern disrupting the other's prefetch window.Includes 5 new unit tests for pin/unpin behavior and updated existing tests.