Skip to content

Conversation

@westonpace
Copy link
Member

This is still a draft while waiting on #5755 and #5773

This PR adds a new URI scheme file+uring. The scheme uses the same local file reader as file but has two custom Reader implementations that are based on the io_uring API. One of these creates a configurable number of process-wide ring threads and the reader communicates with this thread using a queue. The second assumes that the scheduler and decoder run on the same thread and uses a thread local uring instance.

Both are able to saturate up to 1.5M IOPS/s when combined with the scheduler rework. I've tested the thread local variant up to 2M IOPS/s. These numbers are assuming the data is not in the kernel page cache. I've seen results as high as 4M IOPS/s when the data is cached.

@github-actions github-actions bot added the enhancement New feature or request label Jan 21, 2026
@github-actions
Copy link
Contributor

PR Review: feat: io_uring based file reader

Summary

This PR adds a new file+uring URI scheme implementing io_uring-based I/O for high-performance local file access on Linux. The implementation includes two variants: a process-wide thread pool approach and a thread-local variant for current-thread runtimes.


P0 Issues (Must Fix)

1. Short Read Handling Missing (thread.rs:2263-2270)

When io_uring completes a read, the result value indicates the number of bytes actually read, which can be less than requested (short read). The current code only checks if result < 0 for errors but doesn't handle the case where result < request.length. This can lead to returning partially-filled buffers with uninitialized memory.

if result < 0 {
    state.err = Some(io::Error::from_raw_os_error(-result));
} else if request.length > 0 {
    // Should check: result as usize != request.length

The same issue exists in current_thread.rs:907-909.

2. Panic in submit_read on push_request Failure (current_thread.rs:1065)

push_request(request.clone()).unwrap();

This will panic if the submission queue is full (WouldBlock error). This should return an error future instead of panicking, similar to how UringReader::submit_read handles the channel send failure.


P1 Issues (Should Fix)

3. File Handle Cache May Serve Stale Data (reader.rs:1416-1422)

The file handle cache uses a 60-second TTL with 10K max capacity. If a file is modified or replaced after being cached, readers will continue using the old file descriptor. Consider:

  • Documenting this behavior clearly
  • Or checking file inode/mtime on cache hit

4. Busy-Spin in Current-Thread Future (current_thread_future.rs:1218-1219)

cx.waker().wake_by_ref();
Poll::Pending

When the request hasn't completed, this immediately wakes itself, causing a busy spin that will consume 100% CPU until completion. Consider yielding back to the executor properly.

5. Reader Trait Lifetime Change Breaking Change (traits.rs)

The Reader trait changed from async fn to returning BoxFuture. While the PR description mentions this depends on other PRs, this is a breaking change to a public trait that may affect downstream consumers.


Minor Observations

  • Good test coverage for basic functionality
  • Environment variable configuration is well-documented in the module docs
  • The thread pool approach with round-robin selection is reasonable for load balancing
  • The 100ms logging interval in the thread loop may be too frequent for production use; consider making it configurable or debug-level only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant