imbajin commented on code in PR #3420:
URL: https://github.com/apache/hugegraph/pull/3420#discussion_r4393463279
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/PartitionEngine.java:
##########
@@ -267,6 +263,16 @@ public LogStorage createLogStorage(final String uri, final
RaftOptions raftOptio
return this.started;
}
+ static LogStorage createLogStorage(String uri, RaftOptions raftOptions,
+ HgStoreEngineOptions.RaftOptions
storeRaftOptions) {
+ if (storeRaftOptions.isUseRocksDBSegmentLogStorage()) {
+ return RocksDBSegmentLogStorage.builder(uri, raftOptions)
+
.setMaxSegmentFileSize(storeRaftOptions.getMaxSegmentFileSize())
Review Comment:
**[P2] Handle undersized preallocated blank segments when increasing the
configured size**
At this change, passing a larger size to the builder does not make existing
blank segments safe for entries within that new limit. With 16 MiB blank
segments left by a previous run, reopening at 64 MiB and appending a 20 MiB
entry still selects an old 16 MiB segment. JRaft 1.3.14 [reloads blank files
into the allocation
queue](https://github.com/sofastack/sofa-jraft/blob/v1.3.14/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/log/RocksDBSegmentLogStorage.java#L486-L500),
[takes the first queued segment without a capacity
check](https://github.com/sofastack/sofa-jraft/blob/v1.3.14/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/log/RocksDBSegmentLogStorage.java#L399-L412),
and then [rejects the
append](https://github.com/sofastack/sofa-jraft/blob/v1.3.14/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/log/RocksDBSegmentLogStorage.java#L1077-L1083).
In the normal Raft append path, a short append causes [LogManager to report
EIO](https://github.com/sofastack/sofa
-jraft/blob/v1.3.14/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/LogManagerImpl.java#L405-L423),
so this can stop log writes rather than merely defer the new size.
Please safely replace/skip incompatible **blank** preallocated segments
while preserving segments containing logs, and add a small-to-large reopen
regression that leaves an old blank pool and appends an entry larger than the
old size but smaller than the new size. The current large-to-small recovery
test does not cover this direction.
Reviewed source: [current
version](https://github.com/apache/hugegraph/blob/8150ddffa7046f40ca949ba41b55d13862c5e62f/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/PartitionEngine.java#L267-L271).
Static source-trace finding independently cross-checked on dots; no runtime
reproduction or performance test is claimed. This confirms the [still-open
source-PR finding, 🔗 hugegraph/hugegraph
#341](https://github.com/hugegraph/hugegraph/pull/341#discussion_r4386737758),
which was not yet recorded on this Apache PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
