szetszwo commented on a change in pull request #317:
URL: https://github.com/apache/incubator-ratis/pull/317#discussion_r535791223
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLog.java
##########
@@ -391,7 +391,10 @@ public TermIndex getLastEntryTermIndex() {
if (currentOpenSegment == null) {
cache.addOpenSegment(entry.getIndex());
fileLogWorker.startLogSegment(entry.getIndex());
- } else if (isSegmentFull(currentOpenSegment, entry)) {
+ } else if (isSegmentFull(currentOpenSegment, entry,
+ stateMachineCachingEnabled ?
+ LogSegment.Op.CHECK_SEGMENT_FILE_FULL_WITH_STATE_MACHINE_CACHE:
+
LogSegment.Op.CHECK_SEGMENT_FILE_FULL_WITHOUT_STATE_MACHINE_CACHE)) {
Review comment:
@runzhiwang , we have two segment sizes, segment file size and segment
cache size. They can be different since
- for files, we always removeStateMachineData;
- for cache, it depends on stateMachineCachingEnabled.
These two sizes are different when stateMachineCachingEnabled == false.
The method isSegmentFull is actually asking isSegmentFileFull. Therefore,
it should not care about stateMachineCachingEnabled. As you mentioned, it is a
bug to use segment.getTotalSize() inside isSegmentFull. In order to fix the
bug, we probably have to spit it into two methods
- segment.getTotalFileSize()
- segment.getTotalCacheSize()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
