Skip to content

Support conditional sendfile based on transport and platform#214

Merged
michaelklishin merged 1 commit into
mainfrom
no-sendfile-darwin
Apr 29, 2026
Merged

Support conditional sendfile based on transport and platform#214
michaelklishin merged 1 commit into
mainfrom
no-sendfile-darwin

Conversation

@kjnilsson
Copy link
Copy Markdown
Contributor

Add use_sendfile flag to read state to conditionally use file:sendfile() for TCP on non-macOS systems, while falling back to pread+send for SSL and macOS. Update tests to be platform-aware using assert_sendfile_pread.

Add use_sendfile flag to read state to conditionally use file:sendfile()
for TCP on non-macOS systems, while falling back to pread+send for SSL
and macOS. Update tests to be platform-aware using assert_sendfile_pread.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds transport/platform-aware selection of file:sendfile/5 vs file:pread/3 + socket send when streaming chunks, and updates tests to tolerate macOS behavior differences.

Changes:

  • Extend the reader state with a use_sendfile flag derived from transport and OS type.
  • Route send_file/3 through a new sendfile/6 implementation that can fall back to pread + send.
  • Make sendfile-related tests macOS-aware via assert_sendfile_pread/3.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/osiris_log.erl Introduces use_sendfile in read state and threads it into send_file/3 to disable sendfile on SSL/macOS.
test/osiris_log_SUITE.erl Updates sendfile expectations to account for macOS using pread instead of sendfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/osiris_log.erl
Comment thread src/osiris_log.erl
@kjnilsson kjnilsson marked this pull request as ready for review April 28, 2026 14:05
Copy link
Copy Markdown
Contributor

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that this change eliminates a macOS-specific osiris_replica:parse_chunk/3 function_clause when a replica stops (e.g. in RabbitMQ tests).

The root cause comes down to a write(2) followed by sendfile(2) not guaranteed data delivery order even in the case of TCP sockets, which trips up the above function. Or at least that's our best current understanding.

@michaelklishin michaelklishin merged commit 96e7f74 into main Apr 29, 2026
9 checks passed
@michaelklishin michaelklishin deleted the no-sendfile-darwin branch April 29, 2026 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants