facebook / facebook/rocksdb

Documentation is unclear about manual_wal_flush with WriteOptions::sync and without two_write_queues

Open
#12,287 1 comment 1 reaction 0 assignees View on GitHub
documentation up-for-grabs
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

When using `manual_wal_flush=true` *without* `two_write_queues` (i.e. `two_write_queues=false`), and without using transactions (just `atomic_flush=true`), I expect that writes with `WriteOptions::sync=true` should both flush the buffer and sync the WAL file, i.e. equivalent of `FlushWAL(true)`.

However nothing in the documentation explicitly promises this behaviour, and looking at the code I'm unsure if it is even actually true.

It seems to me that AddRecord which is called by Write will never call Flush if manual_flush is true: https://github.com/facebook/rocksdb/blob/59f4cbef8c3ada628c47ff7868a93c7d01e02fd7/db/log_writer.cc#L170

Also, the Write code itself will only call Sync directly on the file, but not directly flush the log writer into the file beforehand:
https://github.com/facebook/rocksdb/blob/59f4cbef8c3ada628c47ff7868a93c7d01e02fd7/db/db_impl/db_impl_write.cc#L1433

Only in case of `two_write_queues=true` does Write trigger a call to `FlushWAL(true)`.

### Expected behavior

`WriteOptions::sync=true` should ensure data persistence when combined with `manual_wal_flush` and this should be explicitly documented.

Contributor guide

Open the contributing guide

Research direction

Start by reading AddRecord in db/log_writer.cc and Write in db/db_impl/db_impl_write.cc, focusing on manual_wal_flush, WriteOptions::sync, two_write_queues, and FlushWAL(true). Trace whether the buffer is flushed before the WAL file is synced. Done means the behavior is resolved and the documentation explicitly states the guarantee for this configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.