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;
- }
}
