Skip to content

New FlushWAL() API to take extra fields such as rate limiter priority #14037

Closed
hx235 wants to merge 1 commit intofacebook:mainfrom
hx235:manual_wal_flush_pri
Closed

New FlushWAL() API to take extra fields such as rate limiter priority #14037
hx235 wants to merge 1 commit intofacebook:mainfrom
hx235:manual_wal_flush_pri

Conversation

@hx235
Copy link
Contributor

@hx235 hx235 commented Oct 8, 2025

Context/Summary:
There is no way to tag or rate-limit write IO occurs during FlushWAL() with priority. Under Options::manual_wal_flush=true, it is the major source of write IO during user writes so we decide to add that support. A new option struct FlushWALOptions is introduced to avoid making the API ugly for future new fields.

Also, we can't use the WriteOptions (https://github.com/facebook/rocksdb/blob/main/include/rocksdb/options.h#L2293-L2302 i) since is associated with that particular Put/Merge/.. associated with that option but FlushWAL() can happen after that write. There is no way to carry that write option over in RocksDB. I also avoided using the WriteOptions since it's mostly for live write.

Test:
New UTs TEST_P(DBRateLimiterOnManualWALFlushTest, ManualWALFlush)

@meta-cla meta-cla bot added the CLA Signed label Oct 8, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 8, 2025

@hx235 has imported this pull request. If you are a Meta employee, you can view this in D84193522.

@hx235 hx235 force-pushed the manual_wal_flush_pri branch from 27fa90c to 12a956d Compare October 8, 2025 23:45
@hx235 hx235 requested a review from archang19 October 9, 2025 02:47
Copy link
Contributor

@archang19 archang19 left a comment

Choose a reason for hiding this comment

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

To confirm I understand why we need this change and cannot use WriteOptions

https://github.com/facebook/rocksdb/blob/main/include/rocksdb/options.h#L2293-L2302 is the issue right? ("Currently the support covers automatic WAL flushes")

@hx235
Copy link
Contributor Author

hx235 commented Oct 9, 2025

and cannot use WriteOptions

https://github.com/facebook/rocksdb/blob/main/include/rocksdb/options.h#L2293-L2302 is the issue right? ("Currently the support covers automatic WAL flushes")

Right - the WriteOptions is associated with that particular write while FlushWAL() happens after that write. There is no way to carry that write option over in RocksDB. I also avoid using the WriteOptions since it's mostly for live write. Added more in summary

@meta-codesync
Copy link

meta-codesync bot commented Oct 9, 2025

@hx235 merged this pull request in f722e68.

xingbowang pushed a commit to xingbowang/rocksdb that referenced this pull request Oct 13, 2025
…facebook#14037)

Summary:
**Context/Summary:**
There is no way to tag or rate-limit write IO occurs during FlushWAL() with priority. Under `Options::manual_wal_flush=true`, it is the major source of write IO during user writes so we decide to add that support. A new option struct `FlushWALOptions` is introduced to avoid making the API ugly for future new fields.

Also, we can't use the WriteOptions (https://github.com/facebook/rocksdb/blob/main/include/rocksdb/options.h#L2293-L2302 i) since is associated with that particular Put/Merge/.. associated with that option but FlushWAL() can happen after that write. There is no way to carry that write option over in RocksDB. I also avoided using the WriteOptions since it's mostly for live write.

Pull Request resolved: facebook#14037

Test Plan: New UTs `TEST_P(DBRateLimiterOnManualWALFlushTest, ManualWALFlush)`

Reviewed By: archang19

Differential Revision: D84193522

Pulled By: hx235

fbshipit-source-id: 18feb5235672010d19a101ce52c8abdcc4a789f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants