facebook / facebook/rocksdb

WritableFileWriter::WriteDirect's buf_.RefitTail() causes redundant disk write when an immediate Flush() is invoked right after.

Open
#12,168 2 comments 0 reactions 0 assignees View on GitHub
enhancement up-for-grabs
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

In Log Writer's WritableFileWriter::WriteDirect(), an unaligned write using would result in rounding up the buffer to an aligned size. It also leaves a "leftover_tail" bytes with the intention to be merged with a subsequent write to overwrite the padded region.

**buf_.RefitTail()** is invoked inside WriteDirect()... which resets the buffer but still leaving the tail bytes as valid. The result of RefitTail() is setting the buf.CurrentSize() to the tail bytes. If WritableFileWriter::Flush() is invoked right after (before a subsequent write comes), then the logic would see a non-zero buf.CurrentSize() and again flush the tail bytes. Consequently, we're seeing 2 disk flushes for every 1 unaligned direct io write.

The above scenario is very common when we enable direct_io and sync (sync invokes Flush()).

### Expected behavior
The tail bytes should not be flushed again.
### Actual behavior
The tail bytes were flushed.
### Steps to reproduce the behavior
1. Setup a WritableFileWriter to use direct_io
2. Disable manualy sync (this would cause a sync to occur after every write)
3. Send an unaligned write (i.e. 1030 bytes) ... in my test my aligned size is 512B.
4. Verify that WriteDirect() would issue the write with 1536 bytes, then follows up with a 512 bytes write in the sync() to flush the tail bytes.

Contributor guide

Open the contributing guide

Research direction

Start with WritableFileWriter::WriteDirect() and follow the behavior of buf_.RefitTail() after an unaligned direct-I/O write. Then inspect WritableFileWriter::Flush() and reproduce the direct_io plus sync scenario using the 1030-byte write described. Done means the leftover tail is not written again by the immediate Flush().

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.