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 <[email protected]> Authored: Tue Oct 16 08:22:45 2012 +0200 Committer: Sylvain Lebresne <[email protected]> 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>>() {
