STAR-1902: Option to disable call to NativeLibrary.trySkipCache (#932)

STAR-1902: Option to disable call to NativeLibrary.trySkipCache

Introduce system property "cassandra.commitlog.skip_file_advice" that allows
to skip the native call to "fadvise" with "FADV_DONTNEED" argument.
The native call is not skipped by default.

This patch was originally implemented in DSE 5.1 under DSP-22315.

(cherry picked from commit 5c78e1d)

src/java/org/apache/cassandra/config/CassandraRelevantProperties.java

@@ -127,6 +127,8 @@ public enum CassandraRelevantProperties
     COMMITLOG_IGNORE_REPLAY_ERRORS("cassandra.commitlog.ignorereplayerrors"),
     COMMITLOG_MAX_OUTSTANDING_REPLAY_BYTES("cassandra.commitlog_max_outstanding_replay_bytes", convertToString(1024 * 1024 * 64)),
     COMMITLOG_MAX_OUTSTANDING_REPLAY_COUNT("cassandra.commitlog_max_outstanding_replay_count", "1024"),
+    // Allows skipping advising the OS to free cached pages associated with commitlog flushing
+    COMMITLOG_SKIP_FILE_ADVICE("cassandra.commitlog.skip_file_advice"),
     COMMITLOG_STOP_ON_ERRORS("cassandra.commitlog.stop_on_errors"),
     /**
      * Entities to replay mutations for upon commit log replay, property is meant to contain

src/java/org/apache/cassandra/db/commitlog/MemoryMappedSegment.java

@@ -22,23 +22,32 @@
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.io.FSWriteError;
+import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.utils.INativeLibrary;
 import org.apache.cassandra.utils.SyncUtil;
 
+import static org.apache.cassandra.config.CassandraRelevantProperties.COMMITLOG_SKIP_FILE_ADVICE;
+
 /*
  * Memory-mapped segment. Maps the destination channel into an appropriately-sized memory-mapped buffer in which the
  * mutation threads write. On sync forces the buffer to disk.
  * If possible, recycles used segment files to avoid reallocating large chunks of disk.
  */
-public class MemoryMappedSegment extends CommitLogSegment
+class MemoryMappedSegment extends CommitLogSegment
 {
+    @VisibleForTesting
+    static boolean skipFileAdviseToFreePageCache = COMMITLOG_SKIP_FILE_ADVICE.getBoolean();
+
     /**
      * Constructs a new segment file.
      *
      * @param commitLog the commit log it will be used with.
+     * @param manager the commit log segment manager that is linked with {@code commitLog}.
      */
     MemoryMappedSegment(CommitLog commitLog, AbstractCommitLogSegmentManager manager)
     {
@@ -90,6 +99,15 @@ protected void flush(int startMarker, int nextMarker)
         {
             throw new FSWriteError(e, getPath());
         }
+
+        if (!skipFileAdviseToFreePageCache)
+        {
+            adviceOnFileToFreePageCache(fd, startMarker, nextMarker, logFile);
+        }
+    }
+
+    void adviceOnFileToFreePageCache(int fd, int startMarker, int nextMarker, File logFile)
+    {
         INativeLibrary.instance.trySkipCache(fd, startMarker, nextMarker, logFile.absolutePath());
     }
 

test/distributed/org/apache/cassandra/db/commitlog/MemoryMappedSegmentStartupTest.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.db.commitlog;
+
+import java.io.IOException;
+
+import org.junit.After;
+import org.junit.Test;
+
+import org.apache.cassandra.distributed.Cluster;
+
+import static org.apache.cassandra.config.CassandraRelevantProperties.COMMITLOG_SKIP_FILE_ADVICE;
+import static org.junit.Assert.assertEquals;
+
+public class MemoryMappedSegmentStartupTest
+{
+    @After
+    public void tearDown() throws Exception
+    {
+        COMMITLOG_SKIP_FILE_ADVICE.reset();
+    }
+
+    @Test
+    public void shouldSetSkipFileAdviceTrueWithParameterTrue() throws IOException
+    {
+        COMMITLOG_SKIP_FILE_ADVICE.setBoolean(true);
+        try (Cluster cluster = Cluster.build(1).start())
+        {
+            assertEquals(true, cluster.get(1).callOnInstance(() -> MemoryMappedSegment.skipFileAdviseToFreePageCache));
+        }
+    }
+
+    @Test
+    public void shouldSetSkipFileAdviceFalseWithParameterFalse() throws IOException
+    {
+        COMMITLOG_SKIP_FILE_ADVICE.setBoolean(false);
+        try (Cluster cluster = Cluster.build(1).start())
+        {
+            assertEquals(false, cluster.get(1).callOnInstance(() -> MemoryMappedSegment.skipFileAdviseToFreePageCache));
+        }
+    }
+
+    @Test
+    public void shouldSetSkipFileAdviceFalseWithParameterMissing() throws IOException
+    {
+        try (Cluster cluster = Cluster.build(1).start())
+        {
+            assertEquals(false, cluster.get(1).callOnInstance(() -> MemoryMappedSegment.skipFileAdviseToFreePageCache));
+        }
+    }
+}

test/unit/org/apache/cassandra/db/commitlog/MemoryMappedSegmentTest.java

@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.db.commitlog;
+
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.io.util.File;
+import org.mockito.Mockito;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+public class MemoryMappedSegmentTest
+{
+    @BeforeClass
+    public static void beforeClass() throws Exception
+    {
+        SchemaLoader.prepareServer();
+    }
+
+    @Test
+    public void shouldNotSkipFileAdviseToFreeSystemCache()
+    {
+        //given
+        MemoryMappedSegment.skipFileAdviseToFreePageCache = false;
+        MemoryMappedSegment memoryMappedSegment = memoryMappedSegment();
+        int startMarker = 0;
+        int nextMarker = 1024;
+
+        //when
+        memoryMappedSegment.flush(startMarker, nextMarker);
+
+        //then
+        verify(memoryMappedSegment)
+                .adviceOnFileToFreePageCache(eq(memoryMappedSegment.fd), eq(startMarker), eq(nextMarker), eq(memoryMappedSegment.logFile));
+    }
+
+    @Test
+    public void shouldSkipFileAdviseToFreeSystemCache()
+    {
+        //given
+        MemoryMappedSegment.skipFileAdviseToFreePageCache = true;
+        MemoryMappedSegment memoryMappedSegment = memoryMappedSegment();
+
+        //when
+        memoryMappedSegment.flush(0, 1024);
+
+        //then
+        verify(memoryMappedSegment, never())
+                .adviceOnFileToFreePageCache(anyInt(), anyInt(), anyInt(), any(File.class));
+    }
+
+    private MemoryMappedSegment memoryMappedSegment()
+    {
+        return Mockito.spy(new MemoryMappedSegment(CommitLog.instance, CommitLog.instance.getSegmentManager()));
+    }
+}