IGNITE-28075 Make writeBufferSize for RocksDB log storage configurable#7724
IGNITE-28075 Make writeBufferSize for RocksDB log storage configurable#7724rpuch wants to merge 1 commit intoapache:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a node-local configuration knob to tune RocksDB write buffer sizing for the partition raft log storage, wiring it through log storage manager creation and validating it via an integration test.
Changes:
- Introduce
RocksDbLogStorageOptionsand pass options intoDefaultLogStorageManagerfor RocksDB-specific tuning. - Wire partition raft log storage creation to read
ignite.system.properties.partitionsRaftLogStorageWriteBufferSize. - Add an integration test verifying the configured
writeBufferSizeis applied.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java | Passes partition-specific RocksDB log storage options into shared log storage manager creation. |
| modules/raft/src/main/java/org/apache/ignite/internal/raft/util/SharedLogStorageManagerUtils.java | Adds overload to supply RocksDbLogStorageOptions to DefaultLogStorageManager. |
| modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/RocksDbLogStorageOptions.java | New options holder that reads partition raft log write buffer size from system local properties. |
| modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageManager.java | Applies configurable writeBufferSize and derives related RocksDB compaction sizing from it. |
| modules/raft/src/test/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageManagerTest.java | Updates test construction to use the new DefaultLogStorageManager constructor signature. |
| modules/raft/src/integrationTest/java/org/apache/ignite/internal/raftsnapshot/ItLogStorageConfigurationTest.java | New IT asserting configured writeBufferSize is reflected in RocksDB column family options. |
| modules/raft/build.gradle | Adds RocksDB JNI dependency to the integration test classpath. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| return Long.parseLong(property.propertyValue()); | ||
| } catch (NumberFormatException e) { | ||
| LOG.warn( | ||
| "Failed to parse partitions writeBufferSize '{}', default value will be used ({})", | ||
| e, | ||
| property.propertyValue(), | ||
| DEFAULT_WRITE_BUFFER_SIZE | ||
| ); | ||
|
|
||
| return DEFAULT_WRITE_BUFFER_SIZE; | ||
| } |
There was a problem hiding this comment.
writeBufferSize is taken verbatim from config and can be <= 0 (or extremely large). RocksDB expects sensible positive sizes; with 0/negative here, the DB may fail to open or behave unexpectedly. Consider validating the parsed value (e.g., require > 0 and maybe enforce an upper bound) and falling back to the default (with a warning) when invalid.
| long writeBufferSize = specificOptions.writeBufferSize(); | ||
| int minWriteBufferNumberToMerge = 1; | ||
| int level0FileNumCompactionTrigger = 50; | ||
|
|
||
| opts.setWriteBufferSize(writeBufferSize); | ||
| opts.setMaxWriteBufferNumber(5); | ||
| opts.setMinWriteBufferNumberToMerge(1); | ||
| opts.setLevel0FileNumCompactionTrigger(50); | ||
|
|
||
| opts.setMinWriteBufferNumberToMerge(minWriteBufferNumberToMerge); | ||
| opts.setLevel0FileNumCompactionTrigger(level0FileNumCompactionTrigger); | ||
| opts.setLevel0SlowdownWritesTrigger(100); | ||
| opts.setLevel0StopWritesTrigger(200); | ||
| // Size of level 0 which is (in stable state) equal to | ||
| // WriteBufferSize * MinWriteBufferNumberToMerge * Level0FileNumCompactionTrigger | ||
| opts.setMaxBytesForLevelBase(3200 * SizeUnit.MB); | ||
| opts.setTargetFileSizeBase(320 * SizeUnit.MB); | ||
| opts.setMaxBytesForLevelBase(writeBufferSize * minWriteBufferNumberToMerge * level0FileNumCompactionTrigger); | ||
| opts.setTargetFileSizeBase(writeBufferSize * 5); | ||
|
|
There was a problem hiding this comment.
setMaxBytesForLevelBase / setTargetFileSizeBase are computed via multiplications on a user-provided writeBufferSize. For large values this can overflow long and produce negative/incorrect sizes. Consider using Math.multiplyExact (and falling back/capping on ArithmeticException) or otherwise clamping values to a safe maximum before passing them to RocksDB.
https://issues.apache.org/jira/browse/IGNITE-28075