szetszwo commented on a change in pull request #317:
URL: https://github.com/apache/incubator-ratis/pull/317#discussion_r535039898



##########
File path: 
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java
##########
@@ -231,6 +235,7 @@ private File getSegmentFile() {
   private volatile long endIndex;
   private RaftStorage storage;
   private RaftLogMetrics raftLogMetrics;
+  private final boolean stateMachineCachingEnabled;

Review comment:
       Let's pass the information in the method calls instead of add 
stateMachineCachingEnabled to LogSegment.

##########
File path: 
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLog.java
##########
@@ -431,7 +431,7 @@ private boolean isSegmentFull(LogSegment segment, 
LogEntryProto entry) {
     if (segment.getTotalSize() >= segmentMaxSize) {
       return true;
     } else {
-      final long entrySize = LogSegment.getEntrySize(entry);
+      final long entrySize = LogSegment.getEntrySize(entry, 
stateMachineCachingEnabled);

Review comment:
       Even if stateMachineCachingEnabled is false, the stateMachineData will 
be removed before writing the entry to the file.  So, it should remove 
stateMachineData inside LogSegment.getEntrySize.

##########
File path: 
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java
##########
@@ -60,8 +60,10 @@
 
   static final Logger LOG = LoggerFactory.getLogger(LogSegment.class);
 
-  static long getEntrySize(LogEntryProto entry) {
-    final int serialized = 
ServerProtoUtils.removeStateMachineData(entry).getSerializedSize();
+  static long getEntrySize(LogEntryProto entry, boolean 
stateMachineCachingEnabled) {

Review comment:
       There are different reasons to getEntrySize.  stateMachineCachingEnabled 
alone cannot determine if it should removeStateMachineData.




----------------------------------------------------------------
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]


Reply via email to