perf(l1): enable unordered writes for trie node column families#6531
perf(l1): enable unordered writes for trie node column families#6531dicethedev wants to merge 1 commit intolambdaclass:mainfrom
Conversation
Greptile SummaryThis PR adds
Confidence Score: 3/5Safe to merge in terms of stability (the option is ignored), but the claimed performance optimization is not achieved — the PR does not do what it intends. There is one P1 finding: crates/storage/backend/rocksdb.rs — specifically line 133 and the relationship between the CF-level
|
| Filename | Overview |
|---|---|
| crates/storage/backend/rocksdb.rs | Adds set_unordered_write(true) to trie node CF options, but this is a no-op because unordered_write is a DB-level (DBOptions) setting in RocksDB that is silently ignored when applied to CF-level options in a ColumnFamilyDescriptor. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[RocksDB open] --> B[DB-level opts\nenable_pipelined_write=true\nwal_recovery_mode=PointInTime\nunordered_write=NOT SET]
A --> C[CF loop]
C --> D{CF name?}
D --> |ACCOUNT_TRIE_NODES\nSTORAGE_TRIE_NODES| E[cf_opts\nset_unordered_write=true\n⚠️ DB-level option, silently ignored]
D --> |HEADERS / BODIES / etc.| F[cf_opts\nstandard settings]
E --> G[ColumnFamilyDescriptor]
F --> G
G --> H[open_cf_descriptors\nunordered_write flag dropped,\nnever applied to DB write path]
B --> H
H --> I[RocksDB running\nno unordered_write effect]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 133
Comment:
**`unordered_write` is a DB-level option, not a CF-level option — this is a no-op**
`unordered_write` belongs to `DBOptions` in RocksDB, not `ColumnFamilyOptions`. When it is set on the `Options` object passed to a `ColumnFamilyDescriptor`, the flag is silently ignored during CF-open because RocksDB only reads the DB-level portion of the options from the primary DB-open call, not from individual CF descriptors (confirmed by the [RocksDB blog](http://rocksdb.org/blog/2019/08/15/unordered-write.html), which shows it set exclusively on `DBOptions`).
To actually take effect, `set_unordered_write(true)` must be called on the top-level `opts` object (line 32). However, doing so makes it a DB-wide setting — RocksDB has no mechanism to enable unordered writes selectively per column family. Additionally, enabling it at DB level would need verification against `enable_pipelined_write(true)` (line 60) and the `PointInTime` WAL recovery mode (line 54), which may require configuration adjustments.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "perf(l1): enable unordered writes for tr..." | Re-trigger Greptile
| cf_opts.set_min_write_buffer_number_to_merge(2); | ||
| cf_opts.set_target_file_size_base(256 * 1024 * 1024); // 256MB | ||
| cf_opts.set_memtable_prefix_bloom_ratio(0.2); // Bloom filter | ||
| cf_opts.set_unordered_write(true); |
There was a problem hiding this comment.
unordered_write is a DB-level option, not a CF-level option — this is a no-op
unordered_write belongs to DBOptions in RocksDB, not ColumnFamilyOptions. When it is set on the Options object passed to a ColumnFamilyDescriptor, the flag is silently ignored during CF-open because RocksDB only reads the DB-level portion of the options from the primary DB-open call, not from individual CF descriptors (confirmed by the RocksDB blog, which shows it set exclusively on DBOptions).
To actually take effect, set_unordered_write(true) must be called on the top-level opts object (line 32). However, doing so makes it a DB-wide setting — RocksDB has no mechanism to enable unordered writes selectively per column family. Additionally, enabling it at DB level would need verification against enable_pipelined_write(true) (line 60) and the PointInTime WAL recovery mode (line 54), which may require configuration adjustments.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 133
Comment:
**`unordered_write` is a DB-level option, not a CF-level option — this is a no-op**
`unordered_write` belongs to `DBOptions` in RocksDB, not `ColumnFamilyOptions`. When it is set on the `Options` object passed to a `ColumnFamilyDescriptor`, the flag is silently ignored during CF-open because RocksDB only reads the DB-level portion of the options from the primary DB-open call, not from individual CF descriptors (confirmed by the [RocksDB blog](http://rocksdb.org/blog/2019/08/15/unordered-write.html), which shows it set exclusively on `DBOptions`).
To actually take effect, `set_unordered_write(true)` must be called on the top-level `opts` object (line 32). However, doing so makes it a DB-wide setting — RocksDB has no mechanism to enable unordered writes selectively per column family. Additionally, enabling it at DB level would need verification against `enable_pipelined_write(true)` (line 60) and the `PointInTime` WAL recovery mode (line 54), which may require configuration adjustments.
How can I resolve this? If you propose a fix, please make it concise.
Motivation
Trie node writes during block execution do not require strict ordering guarantees. RocksDB's default ordered-write path adds unnecessary synchronization overhead for these CFs.
Description
Adds
cf_opts.set_unordered_write(true)toACCOUNT_TRIE_NODESandSTORAGE_TRIE_NODES. Unordered writes allow RocksDB to skip WAL sequencing constraints, potentially increasing write throughput for state-heavy workloads.Not applied to other CFs where ordering or consistency guarantees may matter (e.g.
HEADERS,BODIES,RECEIPTS).Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PRincludes breaking changes to the
Storerequiring a re-sync.Closes #5937