cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jbel...@apache.org
Subject svn commit: r991210 - in /cassandra/trunk: ./ src/java/org/apache/cassandra/db/commitlog/ src/java/org/apache/cassandra/db/migration/ src/java/org/apache/cassandra/service/ test/unit/org/apache/cassandra/db/commitlog/
Date Tue, 31 Aug 2010 14:54:11 GMT
Author: jbellis
Date: Tue Aug 31 14:54:10 2010
New Revision: 991210

URL: http://svn.apache.org/viewvc?rev=991210&view=rev
Log:
avoid attempting to keep CL header constant size (schema change can defeat this); it's not
necessary now that header is a separate file now.  forceNewSegment was attempting to start
a new CL header when the schema changed, which was race-prone.
patch by jbellis; reviewed by gdusbabek for CASSANDRA-1435

Modified:
    cassandra/trunk/CHANGES.txt
    cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
    cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogHeader.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddColumnFamily.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddKeyspace.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropColumnFamily.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropKeyspace.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameColumnFamily.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameKeyspace.java
    cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java
    cassandra/trunk/test/unit/org/apache/cassandra/db/commitlog/CommitLogHeaderTest.java

Modified: cassandra/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/cassandra/trunk/CHANGES.txt?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/CHANGES.txt (original)
+++ cassandra/trunk/CHANGES.txt Tue Aug 31 14:54:10 2010
@@ -39,6 +39,7 @@ dev
  * fix EstimatedHistogram.max (CASSANDRA-1413)
  * handle zero-length (or missing) rows during HH paging (CASSANDRA-1432)
  * include secondary indexes during schema migrations (CASSANDRA-1406)
+ * fix commitlog header race during schema change (CASSANDRA-1435)
 
 
 0.7-beta1

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLog.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLog.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLog.java Tue Aug 31 14:54:10
2010
@@ -471,31 +471,6 @@ public class CommitLog
         }
     }
     
-    public void forceNewSegment()
-    {
-        Callable task = new Callable()
-        {
-            public Object call() throws Exception
-            {
-                sync();
-                segments.add(new CommitLogSegment());
-                return null;
-            }
-        };
-        try
-        {
-            executor.submit(task).get();
-        }
-        catch (InterruptedException e)
-        {
-            throw new RuntimeException(e);
-        }
-        catch (ExecutionException e)
-        {
-            throw new RuntimeException(e);
-        }
-    }
-
     void sync() throws IOException
     {
         currentSegment().sync();

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogHeader.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogHeader.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogHeader.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogHeader.java Tue Aug
31 14:54:10 2010
@@ -43,11 +43,10 @@ public class CommitLogHeader
     public static CommitLogHeaderSerializer serializer = new CommitLogHeaderSerializer();
 
     private Map<Integer, Integer> cfDirtiedAt; // position at which each CF was last
flushed
-    private final int cfCount; // we keep this in case cfcount changes in the interim (size
of lastFlushedAt is not a good indication).
-    
+
     CommitLogHeader()
     {
-        this(new HashMap<Integer, Integer>(), CFMetaData.getCfToIdMap().size());
+        this(new HashMap<Integer, Integer>());
     }
     
     /*
@@ -55,11 +54,9 @@ public class CommitLogHeader
      * also builds an index of position to column family
      * Id.
     */
-    private CommitLogHeader(Map<Integer, Integer> cfDirtiedAt, int cfCount)
+    private CommitLogHeader(Map<Integer, Integer> cfDirtiedAt)
     {
-        this.cfCount = cfCount;
         this.cfDirtiedAt = cfDirtiedAt;
-        assert cfDirtiedAt.size() <= cfCount;
     }
         
     boolean isDirty(Integer cfId)
@@ -154,13 +151,10 @@ public class CommitLogHeader
     {
         public void serialize(CommitLogHeader clHeader, DataOutput dos) throws IOException
         {
-            assert clHeader.cfDirtiedAt.size() <= clHeader.cfCount;
             Checksum checksum = new CRC32();
 
             // write the first checksum after the fixed-size part, so we won't read garbage
lastFlushedAt data.
-            dos.writeInt(clHeader.cfCount); // 4
             dos.writeInt(clHeader.cfDirtiedAt.size()); // 4
-            checksum.update(clHeader.cfCount);
             checksum.update(clHeader.cfDirtiedAt.size());
             dos.writeLong(checksum.getValue());
 
@@ -173,21 +167,12 @@ public class CommitLogHeader
                 checksum.update(entry.getValue());
             }
             dos.writeLong(checksum.getValue());
-
-            // keep the size constant by padding for missing flushed-at entries.  these do
not affect checksum.
-            for (int i = clHeader.cfDirtiedAt.entrySet().size(); i < clHeader.cfCount;
i++)
-            {
-                dos.writeInt(0);
-                dos.writeInt(0);
-            }
         }
 
         public CommitLogHeader deserialize(DataInput dis) throws IOException
         {
             Checksum checksum = new CRC32();
 
-            int cfCount = dis.readInt();
-            checksum.update(cfCount);
             int lastFlushedAtSize = dis.readInt();
             checksum.update(lastFlushedAtSize);
             if (checksum.getValue() != dis.readLong())
@@ -208,7 +193,7 @@ public class CommitLogHeader
                 throw new IOException("Invalid or corrupt commitlog header");
             }
 
-            return new CommitLogHeader(lastFlushedAt, cfCount);
+            return new CommitLogHeader(lastFlushedAt);
         }
     }
 }

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddColumnFamily.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddColumnFamily.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddColumnFamily.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddColumnFamily.java Tue Aug
31 14:54:10 2010
@@ -87,10 +87,6 @@ public class AddColumnFamily extends Mig
         CFMetaData.fixMaxId();
         if (!clientMode)
             Table.open(ksm.name).initCf(cfm.cfId, cfm.cfName);
-
-        if (!clientMode)
-            // force creation of a new commit log segment.
-            CommitLog.instance().forceNewSegment();
     }
 
     public void subdeflate(org.apache.cassandra.db.migration.avro.Migration mi)

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddKeyspace.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddKeyspace.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddKeyspace.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/AddKeyspace.java Tue Aug 31
14:54:10 2010
@@ -77,7 +77,6 @@ public class AddKeyspace extends Migrati
         if (!clientMode)
         {
             Table.open(ksm.name);
-            CommitLog.instance().forceNewSegment();
         }
     }
     

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropColumnFamily.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropColumnFamily.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropColumnFamily.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropColumnFamily.java Tue Aug
31 14:54:10 2010
@@ -92,8 +92,6 @@ public class DropColumnFamily extends Mi
         if (!clientMode)
         {
             Table.open(ksm.name).dropCf(cfm.cfId);
-            // we don't really need a new segment, but let's force it to be consistent with
other operations.
-            CommitLog.instance().forceNewSegment();
         }
     }
     

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropKeyspace.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropKeyspace.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropKeyspace.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/DropKeyspace.java Tue Aug 31
14:54:10 2010
@@ -81,7 +81,6 @@ public class DropKeyspace extends Migrat
         
         if (!clientMode)
         {
-            CommitLog.instance().forceNewSegment();
             // clear up any local hinted data for this keyspace.
             HintedHandOffManager.renameHints(name, null);
         }

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameColumnFamily.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameColumnFamily.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameColumnFamily.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameColumnFamily.java Tue
Aug 31 14:54:10 2010
@@ -107,7 +107,6 @@ public class RenameColumnFamily extends 
         if (!clientMode)
         {
             Table.open(ksm.name).renameCf(cfId, newName);
-            CommitLog.instance().forceNewSegment();
         }
     }
     

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameKeyspace.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameKeyspace.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameKeyspace.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/RenameKeyspace.java Tue Aug
31 14:54:10 2010
@@ -112,9 +112,6 @@ public class RenameKeyspace extends Migr
         {
             Table.clear(oldKsm.name);
             Table.open(newName);
-            // this isn't strictly necessary since the set of all cfs was not modified.
-            CommitLog.instance().forceNewSegment();
-    
             HintedHandOffManager.renameHints(oldName, newName);
         }
     }

Modified: cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java Tue Aug 31 14:54:10
2010
@@ -1647,7 +1647,6 @@ public class StorageService implements I
        
 
         setMode("Draining: replaying commit log", false);
-        CommitLog.instance().forceNewSegment();
         // want to make sure that any segments deleted as a result of flushing are gone.
         DeletionService.waitFor();
         CommitLog.recover();

Modified: cassandra/trunk/test/unit/org/apache/cassandra/db/commitlog/CommitLogHeaderTest.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/test/unit/org/apache/cassandra/db/commitlog/CommitLogHeaderTest.java?rev=991210&r1=991209&r2=991210&view=diff
==============================================================================
--- cassandra/trunk/test/unit/org/apache/cassandra/db/commitlog/CommitLogHeaderTest.java (original)
+++ cassandra/trunk/test/unit/org/apache/cassandra/db/commitlog/CommitLogHeaderTest.java Tue
Aug 31 14:54:10 2010
@@ -47,21 +47,4 @@ public class CommitLogHeaderTest extends
         clh.turnOn(65, 2);
         assert clh.getReplayPosition() == 0;
     }
-        
-    @Test
-    public void constantSize() throws IOException
-    {
-        CommitLogHeader clh0 = new CommitLogHeader();
-        clh0.turnOn(2, 34);
-        ByteArrayOutputStream out0 = new ByteArrayOutputStream();
-        CommitLogHeader.serializer.serialize(clh0, new DataOutputStream(out0));
-
-        CommitLogHeader clh1 = new CommitLogHeader();
-        for (int i = 0; i < 5; i++)
-            clh1.turnOn(i, 1000 * i);
-        ByteArrayOutputStream out1 = new ByteArrayOutputStream();
-        CommitLogHeader.serializer.serialize(clh1, new DataOutputStream(out1));
-
-        assert out0.toByteArray().length == out1.toByteArray().length;
-    }
 }



Mime
View raw message