You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2012/10/16 08:25:43 UTC
[2/2] git commit: Fix potential NPE during CFS reload
Fix potential NPE during CFS reload
patch by slebresne; reviewed by xedin for CASSANDRA-4786
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/c8a76187
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/c8a76187
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/c8a76187
Branch: refs/heads/trunk
Commit: c8a7618763f59372d291f928d996d2593b93872b
Parents: d525cf9
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Tue Oct 16 08:22:45 2012 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Tue Oct 16 08:23:31 2012 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/config/CFMetaData.java | 50 +++++++-------
.../org/apache/cassandra/db/ColumnFamilyStore.java | 17 +++++-
src/java/org/apache/cassandra/db/Memtable.java | 7 ++
4 files changed, 48 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/c8a76187/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 0edd211..c3df551 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -28,6 +28,7 @@
* Store more information into peers table (CASSANDRA-4351)
* Configurable bucket size for size tiered compaction (CASSANDRA-4704)
* Run leveled compaction in parallel (CASSANDRA-4310)
+ * Fix potential NPE during CFS reload (CASSANDRA-4786)
1.2-beta1
http://git-wip-us.apache.org/repos/asf/cassandra/blob/c8a76187/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index 176d63a..fe44b54 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -234,40 +234,40 @@ public final class CFMetaData
public final String ksName; // name of keyspace
public final String cfName; // name of this column family
public final ColumnFamilyType cfType; // standard, super
- public AbstractType<?> comparator; // bytes, long, timeuuid, utf8, etc.
- public AbstractType<?> subcolumnComparator; // like comparator, for supercolumns
+ public volatile AbstractType<?> comparator; // bytes, long, timeuuid, utf8, etc.
+ public volatile AbstractType<?> subcolumnComparator; // like comparator, for supercolumns
//OPTIONAL
- private String comment; // default none, for humans only
- private double readRepairChance; // default 1.0 (always), chance [0.0,1.0] of read repair
- private double dcLocalReadRepairChance; // default 0.0
- private boolean replicateOnWrite; // default false
- private int gcGraceSeconds; // default 864000 (ten days)
- private AbstractType<?> defaultValidator; // default BytesType (no-op), use comparator types
- private AbstractType<?> keyValidator; // default BytesType (no-op), use comparator types
- private int minCompactionThreshold; // default 4
- private int maxCompactionThreshold; // default 32
- private List<ByteBuffer> keyAliases = new ArrayList<ByteBuffer>();
- private List<ByteBuffer> columnAliases = new ArrayList<ByteBuffer>();
- private ByteBuffer valueAlias; // default NULL
- private Double bloomFilterFpChance; // default NULL
- private Caching caching; // default KEYS_ONLY (possible: all, key_only, row_only, none)
-
- Map<ByteBuffer, ColumnDefinition> column_metadata;
- public Class<? extends AbstractCompactionStrategy> compactionStrategyClass;
- public Map<String, String> compactionStrategyOptions;
-
- public CompressionParameters compressionParameters;
+ private volatile String comment; // default none, for humans only
+ private volatile double readRepairChance; // default 1.0 (always), chance [0.0,1.0] of read repair
+ private volatile double dcLocalReadRepairChance; // default 0.0
+ private volatile boolean replicateOnWrite; // default false
+ private volatile int gcGraceSeconds; // default 864000 (ten days)
+ private volatile AbstractType<?> defaultValidator; // default BytesType (no-op), use comparator types
+ private volatile AbstractType<?> keyValidator; // default BytesType (no-op), use comparator types
+ private volatile int minCompactionThreshold; // default 4
+ private volatile int maxCompactionThreshold; // default 32
+ private volatile List<ByteBuffer> keyAliases = new ArrayList<ByteBuffer>();
+ private volatile List<ByteBuffer> columnAliases = new ArrayList<ByteBuffer>();
+ private volatile ByteBuffer valueAlias; // default NULL
+ private volatile Double bloomFilterFpChance; // default NULL
+ private volatile Caching caching; // default KEYS_ONLY (possible: all, key_only, row_only, none)
+
+ volatile Map<ByteBuffer, ColumnDefinition> column_metadata;
+ public volatile Class<? extends AbstractCompactionStrategy> compactionStrategyClass;
+ public volatile Map<String, String> compactionStrategyOptions;
+
+ public volatile CompressionParameters compressionParameters;
// Default consistency levels for CQL3. The default for those values is ONE,
// but we keep the internal default to null as it help handling thrift compatibility
- private ConsistencyLevel readConsistencyLevel;
- private ConsistencyLevel writeConsistencyLevel;
+ private volatile ConsistencyLevel readConsistencyLevel;
+ private volatile ConsistencyLevel writeConsistencyLevel;
// Processed infos used by CQL. This can be fully reconstructed from the CFMedata,
// so it's not saved on disk. It is however costlyish to recreate for each query
// so we cache it here (and update on each relevant CFMetadata change)
- private CFDefinition cqlCfDef;
+ private volatile CFDefinition cqlCfDef;
public CFMetaData comment(String prop) { comment = enforceCommentNotNull(prop); return this;}
public CFMetaData readRepairChance(double prop) {readRepairChance = prop; return this;}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/c8a76187/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index 8f1b21c..8a1d54f 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -156,10 +156,23 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
// If the CF comparator has changed, we need to change the memtable,
// because the old one still aliases the previous comparator. We don't
// call forceFlush() because it can skip the switch if the memtable is
- // clean, which we don't want here.
+ // clean, which we don't want here. Also, because there can be a race
+ // between the time we acquire the current memtable and we flush it
+ // (another thread can have flushed it first), we attempt the switch
+ // until we know the memtable has the current comparator.
try
{
- maybeSwitchMemtable(getMemtableThreadSafe(), true).get();
+ while (true)
+ {
+ AbstractType comparator = metadata.comparator;
+ Memtable memtable = getMemtableThreadSafe();
+ if (memtable.initialComparator == comparator)
+ break;
+
+ Future future = maybeSwitchMemtable(getMemtableThreadSafe(), true);
+ if (future != null)
+ future.get();
+ }
}
catch (ExecutionException e)
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/c8a76187/src/java/org/apache/cassandra/db/Memtable.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Memtable.java b/src/java/org/apache/cassandra/db/Memtable.java
index 053d47f..82d22ca 100644
--- a/src/java/org/apache/cassandra/db/Memtable.java
+++ b/src/java/org/apache/cassandra/db/Memtable.java
@@ -41,6 +41,7 @@ import org.apache.cassandra.db.commitlog.ReplayPosition;
import org.apache.cassandra.db.filter.AbstractColumnIterator;
import org.apache.cassandra.db.filter.NamesQueryFilter;
import org.apache.cassandra.db.filter.SliceQueryFilter;
+import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.io.sstable.SSTableMetadata;
import org.apache.cassandra.io.sstable.SSTableReader;
import org.apache.cassandra.io.sstable.SSTableWriter;
@@ -101,10 +102,16 @@ public class Memtable
};
};
+ // Record the comparator of the CFS at the creation of the memtable. This
+ // is only used when a user update the CF comparator, to know if the
+ // memtable was created with the new or old comparator.
+ public final AbstractType initialComparator;
+
public Memtable(ColumnFamilyStore cfs)
{
this.cfs = cfs;
this.creationTime = System.currentTimeMillis();
+ this.initialComparator = cfs.metadata.comparator;
Callable<Set<Object>> provider = new Callable<Set<Object>>()
{