rpuch commented on code in PR #7299:
URL: https://github.com/apache/ignite-3/pull/7299#discussion_r2644910437


##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentFile.java:
##########
@@ -45,18 +45,36 @@ class SegmentFile implements ManuallyCloseable {
      */
     private static final int CLOSED_POS_MARKER = -1;
 
+    private static final MappedByteBufferSyncer SYNCER = 
MappedByteBufferSyncer.createSyncer();
+
     private final MappedByteBuffer buffer;
 
+    /** Flag indicating if an fsync call should follow every write to the 
buffer. */
+    private final boolean isSync;
+
+    /** Position of the first non-reserved byte in the buffer. */
     private final AtomicInteger bufferPosition = new AtomicInteger();
 
+    /** Number of concurrent writers to the buffer. */
     private final AtomicInteger numWriters = new AtomicInteger();
 
-    private SegmentFile(RandomAccessFile file) throws IOException {
+    /** Position in the buffer <b>up to which</b> some data has been written. 
*/

Review Comment:
   ```suggestion
       /** Position in the buffer <b>up to which</b> some data has been written 
(but not necessarily synced to disk). */
   ```



##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentFile.java:
##########
@@ -163,8 +199,35 @@ private void close(byte @Nullable [] bytesToWrite) {
         }
     }
 
+    int lastWritePosition() {
+        return lastWritePosition;
+    }
+
+    int syncPosition() {
+        return syncPosition;
+    }
+
     void sync() {
-        buffer.force();
+        sync(lastWritePosition);
+    }
+
+    private void sync(int upToPosition) {
+        if (syncPosition >= upToPosition) {
+            return;
+        }
+
+        synchronized (syncLock) {
+            int syncPosition = this.syncPosition;
+
+            if (syncPosition >= upToPosition) {

Review Comment:
   Same about flipping



##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentFile.java:
##########
@@ -163,8 +199,35 @@ private void close(byte @Nullable [] bytesToWrite) {
         }
     }
 
+    int lastWritePosition() {
+        return lastWritePosition;
+    }
+
+    int syncPosition() {
+        return syncPosition;
+    }
+
     void sync() {
-        buffer.force();
+        sync(lastWritePosition);
+    }
+
+    private void sync(int upToPosition) {
+        if (syncPosition >= upToPosition) {

Review Comment:
   It seems that flipping the inequality will make the code more readable



##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentFile.java:
##########
@@ -133,6 +165,10 @@ private void close(byte @Nullable [] bytesToWrite) {
 
         if (bytesToWrite != null && pos + bytesToWrite.length <= 
buffer.limit()) {
             slice(pos, bytesToWrite.length).put(bytesToWrite);
+

Review Comment:
   Please mention in the javadoc of `close*()` methods that closure does not 
imply syncing by itself



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

Reply via email to