Add write buffering to reduce IOPS from small sequential writes#65
Add write buffering to reduce IOPS from small sequential writes#65
Conversation
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
bbockelm
left a comment
There was a problem hiding this comment.
Good first try, small changes requested.
src/multiuser.cpp
Outdated
There was a problem hiding this comment.
When buffered writes are not enabled at constructor time (i.e., disabled in the config file), never take this mutex.
Merge the logic in https://github.com/opensciencegrid/xrootd-multiuser/pull/61/changes with the changes in this function; this work will conflict with that and it's straightforward to do both.
| // Sequential write - check if we should buffer it | ||
| size_t total_size = m_write_buffer.size() + size; | ||
|
|
||
| if (total_size <= m_write_buffer_size) { |
There was a problem hiding this comment.
There are edge cases here I'd like approached differently.
If the total is over the buffer size, then copy as much data as you can to fill the buffer and then write. This way, writes are always the buffer size (except maybe the last before close) at the cost of memcpy.
There was a problem hiding this comment.
Updated to fill the buffer completely before flushing when total exceeds buffer size. After flush, remaining data is either buffered (if it fits) or written directly (if too large). All writes are now buffer-sized except possibly the last. Fixed in b830dd6.
src/multiuser.cpp
Outdated
There was a problem hiding this comment.
Similarly as above - if the buffering is disabled in the config file, then skip the lock.
There was a problem hiding this comment.
Changed to use std::unique_lock with std::defer_lock - the lock is only acquired when m_write_buffer_size > 0, skipping the lock entirely when buffering is disabled. Fixed in b830dd6.
…#61 logic Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
Write Buffering Implementation Plan
multiuser.writebuffersizeto MultiuserFileSystemOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.