Fix batch commitlog sync regression

patch by Zhao Yang; reviewed by jasobrown for CASSANDRA-14292


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/00c90c16
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/00c90c16
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/00c90c16

Branch: refs/heads/cassandra-3.11
Commit: 00c90c16e99fdfb153cb418f0e3486b62183986e
Parents: c673389
Author: Zhao Yang <zhaoyangsingap...@gmail.com>
Authored: Tue Mar 6 09:58:53 2018 +0800
Committer: Jason Brown <jasedbr...@gmail.com>
Committed: Wed Mar 7 04:00:03 2018 -0800

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../db/commitlog/AbstractCommitLogService.java  |  9 ++-
 .../db/commitlog/BatchCommitLogService.java     |  2 +-
 .../db/commitlog/BatchCommitLogTest.java        | 66 ++++++++++++++++++++
 4 files changed, 75 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/00c90c16/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index ad558de..6596602 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.17
+ * Fix batch commitlog sync regression (CASSANDRA-14292)
  * Write to pending endpoint when view replica is also base replica 
(CASSANDRA-14251)
  * Chain commit log marker potential performance regression in batch commit 
mode (CASSANDRA-14194)
  * Fully utilise specified compaction threads (CASSANDRA-14210)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/00c90c16/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogService.java
----------------------------------------------------------------------
diff --git 
a/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogService.java 
b/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogService.java
index f3cfd29..1cee55d 100644
--- a/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogService.java
+++ b/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogService.java
@@ -263,12 +263,17 @@ public abstract class AbstractCommitLogService
      */
     public WaitQueue.Signal requestExtraSync()
     {
-        syncRequested = true;
         WaitQueue.Signal signal = syncComplete.register();
-        haveWork.release(1);
+        requestSync();
         return signal;
     }
 
+    protected void requestSync()
+    {
+        syncRequested = true;
+        haveWork.release(1);
+    }
+
     public void shutdown()
     {
         shutdown = true;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/00c90c16/src/java/org/apache/cassandra/db/commitlog/BatchCommitLogService.java
----------------------------------------------------------------------
diff --git 
a/src/java/org/apache/cassandra/db/commitlog/BatchCommitLogService.java 
b/src/java/org/apache/cassandra/db/commitlog/BatchCommitLogService.java
index ceb5d64..c0e6afc 100644
--- a/src/java/org/apache/cassandra/db/commitlog/BatchCommitLogService.java
+++ b/src/java/org/apache/cassandra/db/commitlog/BatchCommitLogService.java
@@ -30,7 +30,7 @@ class BatchCommitLogService extends AbstractCommitLogService
     {
         // wait until record has been safely persisted to disk
         pending.incrementAndGet();
-        haveWork.release();
+        requestSync();
         alloc.awaitDiskSync(commitLog.metrics.waitingOnCommit);
         pending.decrementAndGet();
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/00c90c16/test/unit/org/apache/cassandra/db/commitlog/BatchCommitLogTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/db/commitlog/BatchCommitLogTest.java 
b/test/unit/org/apache/cassandra/db/commitlog/BatchCommitLogTest.java
new file mode 100644
index 0000000..c7f7e57
--- /dev/null
+++ b/test/unit/org/apache/cassandra/db/commitlog/BatchCommitLogTest.java
@@ -0,0 +1,66 @@
+package org.apache.cassandra.db.commitlog;
+
+import static org.junit.Assert.*;
+
+import java.nio.ByteBuffer;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.Config;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.db.Mutation;
+import org.apache.cassandra.db.RowUpdateBuilder;
+import org.apache.cassandra.db.compaction.CompactionManager;
+import org.apache.cassandra.db.marshal.AsciiType;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.schema.KeyspaceParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class BatchCommitLogTest
+{
+    private static final long CL_BATCH_SYNC_WINDOW = 1000; // 1 second
+    private static final String KEYSPACE1 = "CommitLogTest";
+    private static final String STANDARD1 = "Standard1";
+
+    @BeforeClass
+    public static void before()
+    {
+        DatabaseDescriptor.setCommitLogSync(Config.CommitLogSync.batch);
+        DatabaseDescriptor.setCommitLogSyncBatchWindow(CL_BATCH_SYNC_WINDOW);
+
+        KeyspaceParams.DEFAULT_LOCAL_DURABLE_WRITES = false;
+        SchemaLoader.prepareServer();
+        SchemaLoader.createKeyspace(KEYSPACE1,
+                                    KeyspaceParams.simple(1),
+                                    SchemaLoader.standardCFMD(KEYSPACE1, 
STANDARD1, 0, AsciiType.instance, BytesType.instance));
+        CompactionManager.instance.disableAutoCompaction();
+    }
+
+    @Test
+    public void testBatchCLSyncImmediately()
+    {
+        ColumnFamilyStore cfs1 = 
Keyspace.open(KEYSPACE1).getColumnFamilyStore(STANDARD1);
+        Mutation m = new RowUpdateBuilder(cfs1.metadata, 0, "key")
+                         .clustering("bytes")
+                         .add("val", ByteBuffer.allocate(10 * 1024))
+                         .build();
+
+        long startNano = System.nanoTime();
+        CommitLog.instance.add(m);
+        long delta = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - 
startNano);
+        assertTrue("Expect batch commitlog sync immediately, but took " + 
delta, delta < CL_BATCH_SYNC_WINDOW);
+    }
+
+    @Test
+    public void testBatchCLShutDownImmediately() throws InterruptedException
+    {
+        long startNano = System.nanoTime();
+        CommitLog.instance.shutdownBlocking();
+        long delta = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - 
startNano);
+        assertTrue("Expect batch commitlog shutdown immediately, but took " + 
delta, delta < CL_BATCH_SYNC_WINDOW);
+        CommitLog.instance.start();
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to