Repository: cassandra Updated Branches: refs/heads/trunk 737b3d0ce -> 721eaf6f0
Fix return of 'columns()' for AbstractBTreeColumns patch by benedict & slebresne; reviewed by benedict & slebresne for CASSANDRA-10220 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/520089b5 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/520089b5 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/520089b5 Branch: refs/heads/trunk Commit: 520089b52faef1abbd270c6de130bf6c576ab1c0 Parents: c7557bd Author: Benedict Elliott Smith <bened...@apache.org> Authored: Fri Aug 28 14:56:45 2015 +0100 Committer: Sylvain Lebresne <sylv...@datastax.com> Committed: Fri Sep 4 15:45:40 2015 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/db/PartitionColumns.java | 14 ++++++ .../db/partitions/AbstractBTreePartition.java | 33 ++++++++------ .../db/partitions/AtomicBTreePartition.java | 7 ++- .../db/partitions/CachedBTreePartition.java | 5 +-- .../db/partitions/FilteredPartition.java | 4 +- .../db/partitions/ImmutableBTreePartition.java | 10 ++--- .../db/partitions/PartitionUpdate.java | 45 +++++++++++--------- 8 files changed, 70 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 4d8a932..e4c5258 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.0-beta2 + * Fix columns returned by AbstractBtreePartitions (CASSANDRA-10220) * Fix backward compatibility issue due to AbstractBounds serialization bug (CASSANDRA-9857) * Fix startup error when upgrading nodes (CASSANDRA-10136) * Base table PRIMARY KEY can be assumed to be NOT NULL in MV creation (CASSANDRA-10147) http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/PartitionColumns.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/PartitionColumns.java b/src/java/org/apache/cassandra/db/PartitionColumns.java index e1008df..93204f4 100644 --- a/src/java/org/apache/cassandra/db/PartitionColumns.java +++ b/src/java/org/apache/cassandra/db/PartitionColumns.java @@ -40,6 +40,7 @@ public class PartitionColumns implements Iterable<ColumnDefinition> public PartitionColumns(Columns statics, Columns regulars) { + assert statics != null && regulars != null; this.statics = statics; this.regulars = regulars; } @@ -61,6 +62,19 @@ public class PartitionColumns implements Iterable<ColumnDefinition> return statics.isEmpty() ? this : new PartitionColumns(Columns.NONE, regulars); } + public PartitionColumns mergeTo(PartitionColumns that) + { + if (this == that) + return this; + Columns statics = this.statics.mergeTo(that.statics); + Columns regulars = this.regulars.mergeTo(that.regulars); + if (statics == this.statics && regulars == this.regulars) + return this; + if (statics == that.statics && regulars == that.regulars) + return that; + return new PartitionColumns(statics, regulars); + } + public boolean isEmpty() { return statics.isEmpty() && regulars.isEmpty(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java index 41015b0..da20c91 100644 --- a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java @@ -32,31 +32,32 @@ import static org.apache.cassandra.utils.btree.BTree.Dir.desc; public abstract class AbstractBTreePartition implements Partition, Iterable<Row> { - protected static final Holder EMPTY = new Holder(BTree.empty(), DeletionInfo.LIVE, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); + protected static final Holder EMPTY = new Holder(PartitionColumns.NONE, BTree.empty(), DeletionInfo.LIVE, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); protected final CFMetaData metadata; protected final DecoratedKey partitionKey; - protected final PartitionColumns columns; + protected abstract Holder holder(); protected abstract boolean canHaveShadowedData(); - protected AbstractBTreePartition(CFMetaData metadata, DecoratedKey partitionKey, PartitionColumns columns) + protected AbstractBTreePartition(CFMetaData metadata, DecoratedKey partitionKey) { this.metadata = metadata; this.partitionKey = partitionKey; - this.columns = columns; } protected static final class Holder { + final PartitionColumns columns; final DeletionInfo deletionInfo; // the btree of rows final Object[] tree; final Row staticRow; final EncodingStats stats; - Holder(Object[] tree, DeletionInfo deletionInfo, Row staticRow, EncodingStats stats) + Holder(PartitionColumns columns, Object[] tree, DeletionInfo deletionInfo, Row staticRow, EncodingStats stats) { + this.columns = columns; this.tree = tree; this.deletionInfo = deletionInfo; this.staticRow = staticRow; @@ -103,8 +104,7 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> public PartitionColumns columns() { - // We don't really know which columns will be part of the update, so assume it's all of them - return metadata.partitionColumns(); + return holder().columns; } public EncodingStats stats() @@ -232,7 +232,9 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> super(AbstractBTreePartition.this.metadata, AbstractBTreePartition.this.partitionKey, current.deletionInfo.getPartitionDeletion(), - AbstractBTreePartition.this.columns, + selection.fetchedColumns(), // non-selected columns will be filtered in subclasses by RowAndDeletionMergeIterator + // it would also be more precise to return the intersection of the selection and current.columns, + // but its probably not worth spending time on computing that. staticRow, isReversed, current.stats); @@ -317,6 +319,7 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> protected static Holder build(UnfilteredRowIterator iterator, int initialRowCapacity) { CFMetaData metadata = iterator.metadata(); + PartitionColumns columns = iterator.columns(); boolean reversed = iterator.isReverseOrder(); BTree.Builder<Row> builder = BTree.builder(metadata.comparator, initialRowCapacity); @@ -335,13 +338,15 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> if (reversed) builder.reverse(); - return new Holder(builder.build(), deletionBuilder.build(), iterator.staticRow(), iterator.stats()); + return new Holder(columns, builder.build(), deletionBuilder.build(), iterator.staticRow(), iterator.stats()); } - // live must (as the name suggests) not contain any deletion information - protected static Holder build(RowIterator rows, DeletionInfo live, boolean buildEncodingStats, int initialRowCapacity) + // Note that when building with a RowIterator, deletion will generally be LIVE, but we allow to pass it nonetheless because PartitionUpdate + // passes a MutableDeletionInfo that it mutates later. + protected static Holder build(RowIterator rows, DeletionInfo deletion, boolean buildEncodingStats, int initialRowCapacity) { CFMetaData metadata = rows.metadata(); + PartitionColumns columns = rows.columns(); boolean reversed = rows.isReverseOrder(); BTree.Builder<Row> builder = BTree.builder(metadata.comparator, initialRowCapacity); @@ -357,9 +362,9 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> Row staticRow = rows.staticRow(); Object[] tree = builder.build(); - EncodingStats stats = buildEncodingStats ? EncodingStats.Collector.collect(staticRow, BTree.iterator(tree), live) + EncodingStats stats = buildEncodingStats ? EncodingStats.Collector.collect(staticRow, BTree.iterator(tree), deletion) : EncodingStats.NO_STATS; - return new Holder(tree, live, staticRow, stats); + return new Holder(columns, tree, deletion, staticRow, stats); } @Override @@ -371,7 +376,7 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> metadata.ksName, metadata.cfName, metadata.getKeyValidator().getString(partitionKey().getKey()), - columns)); + columns())); if (staticRow() != Rows.EMPTY_STATIC_ROW) sb.append("\n ").append(staticRow().toString(metadata)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java index e00a75e..ae8a1c3 100644 --- a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java @@ -66,8 +66,6 @@ public class AtomicBTreePartition extends AbstractBTreePartition private static final int CLOCK_SHIFT = 17; // CLOCK_GRANULARITY = 1^9ns >> CLOCK_SHIFT == 132us == (1/7.63)ms - private static final Holder EMPTY = new Holder(BTree.empty(), DeletionInfo.LIVE, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); - private static final AtomicIntegerFieldUpdater<AtomicBTreePartition> wasteTrackerUpdater = AtomicIntegerFieldUpdater.newUpdater(AtomicBTreePartition.class, "wasteTracker"); private static final AtomicReferenceFieldUpdater<AtomicBTreePartition, Holder> refUpdater = AtomicReferenceFieldUpdater.newUpdater(AtomicBTreePartition.class, Holder.class, "ref"); @@ -87,7 +85,7 @@ public class AtomicBTreePartition extends AbstractBTreePartition public AtomicBTreePartition(CFMetaData metadata, DecoratedKey partitionKey, MemtableAllocator allocator) { // involved in potential bug? partition columns may be a subset if we alter columns while it's in memtable - super(metadata, partitionKey, metadata.partitionColumns()); + super(metadata, partitionKey); this.allocator = allocator; this.ref = EMPTY; } @@ -149,6 +147,7 @@ public class AtomicBTreePartition extends AbstractBTreePartition deletionInfo = current.deletionInfo; } + PartitionColumns columns = update.columns().mergeTo(current.columns); Row newStatic = update.staticRow(); Row staticRow = newStatic.isEmpty() ? current.staticRow @@ -156,7 +155,7 @@ public class AtomicBTreePartition extends AbstractBTreePartition Object[] tree = BTree.update(current.tree, update.metadata().comparator, update, update.rowCount(), updater); EncodingStats newStats = current.stats.mergeWith(update.stats()); - if (tree != null && refUpdater.compareAndSet(this, current, new Holder(tree, deletionInfo, staticRow, newStats))) + if (tree != null && refUpdater.compareAndSet(this, current, new Holder(columns, tree, deletionInfo, staticRow, newStats))) { updater.finish(); return new long[]{ updater.dataSize, updater.colUpdateTimeDelta }; http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/CachedBTreePartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/CachedBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/CachedBTreePartition.java index b122791..afe1cc3 100644 --- a/src/java/org/apache/cassandra/db/partitions/CachedBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/CachedBTreePartition.java @@ -41,7 +41,6 @@ public class CachedBTreePartition extends ImmutableBTreePartition implements Cac private CachedBTreePartition(CFMetaData metadata, DecoratedKey partitionKey, - PartitionColumns columns, Holder holder, int createdAtInSec, int cachedLiveRows, @@ -49,7 +48,7 @@ public class CachedBTreePartition extends ImmutableBTreePartition implements Cac int nonTombstoneCellCount, int nonExpiringLiveCells) { - super(metadata, partitionKey, columns, holder); + super(metadata, partitionKey, holder); this.createdAtInSec = createdAtInSec; this.cachedLiveRows = cachedLiveRows; this.rowsWithNonExpiringCells = rowsWithNonExpiringCells; @@ -118,7 +117,6 @@ public class CachedBTreePartition extends ImmutableBTreePartition implements Cac return new CachedBTreePartition(iterator.metadata(), iterator.partitionKey(), - iterator.columns(), holder, nowInSec, cachedLiveRows, @@ -216,7 +214,6 @@ public class CachedBTreePartition extends ImmutableBTreePartition implements Cac return new CachedBTreePartition(metadata, header.key, - header.sHeader.columns(), holder, createdAtInSec, cachedLiveRows, http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/FilteredPartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/FilteredPartition.java b/src/java/org/apache/cassandra/db/partitions/FilteredPartition.java index e0b568d..26a947b 100644 --- a/src/java/org/apache/cassandra/db/partitions/FilteredPartition.java +++ b/src/java/org/apache/cassandra/db/partitions/FilteredPartition.java @@ -29,7 +29,7 @@ public class FilteredPartition extends ImmutableBTreePartition { public FilteredPartition(RowIterator rows) { - super(rows.metadata(), rows.partitionKey(), rows.columns(), build(rows, DeletionInfo.LIVE, false, 16)); + super(rows.metadata(), rows.partitionKey(), build(rows, DeletionInfo.LIVE, false, 16)); } /** @@ -60,7 +60,7 @@ public class FilteredPartition extends ImmutableBTreePartition public PartitionColumns columns() { - return columns; + return FilteredPartition.this.columns(); } public DecoratedKey partitionKey() http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java index a13e070..9af4bad 100644 --- a/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java @@ -37,16 +37,15 @@ public class ImmutableBTreePartition extends AbstractBTreePartition DeletionInfo deletionInfo, EncodingStats stats) { - super(metadata, partitionKey, columns); - this.holder = new Holder(tree, deletionInfo, staticRow, stats); + super(metadata, partitionKey); + this.holder = new Holder(columns, tree, deletionInfo, staticRow, stats); } protected ImmutableBTreePartition(CFMetaData metadata, DecoratedKey partitionKey, - PartitionColumns columns, Holder holder) { - super(metadata, partitionKey, columns); + super(metadata, partitionKey); this.holder = holder; } @@ -77,8 +76,7 @@ public class ImmutableBTreePartition extends AbstractBTreePartition */ public static ImmutableBTreePartition create(UnfilteredRowIterator iterator, int initialRowCapacity) { - return new ImmutableBTreePartition(iterator.metadata(), iterator.partitionKey(), iterator.columns(), - build(iterator, initialRowCapacity)); + return new ImmutableBTreePartition(iterator.metadata(), iterator.partitionKey(), build(iterator, initialRowCapacity)); } protected Holder holder() http://git-wip-us.apache.org/repos/asf/cassandra/blob/520089b5/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java index 1a27b39..b1776ca 100644 --- a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java +++ b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java @@ -81,21 +81,20 @@ public class PartitionUpdate extends AbstractBTreePartition int initialRowCapacity, boolean canHaveShadowedData) { - super(metadata, key, columns); + super(metadata, key); this.deletionInfo = deletionInfo; - this.holder = new Holder(BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); + this.holder = new Holder(columns, BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); this.canHaveShadowedData = canHaveShadowedData; rowBuilder = builder(initialRowCapacity); } private PartitionUpdate(CFMetaData metadata, DecoratedKey key, - PartitionColumns columns, Holder holder, MutableDeletionInfo deletionInfo, boolean canHaveShadowedData) { - super(metadata, key, columns); + super(metadata, key); this.holder = holder; this.deletionInfo = deletionInfo; this.isBuilt = true; @@ -132,8 +131,8 @@ public class PartitionUpdate extends AbstractBTreePartition public static PartitionUpdate emptyUpdate(CFMetaData metadata, DecoratedKey key) { MutableDeletionInfo deletionInfo = MutableDeletionInfo.live(); - Holder holder = new Holder(BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); - return new PartitionUpdate(metadata, key, PartitionColumns.NONE, holder, deletionInfo, false); + Holder holder = new Holder(PartitionColumns.NONE, BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); + return new PartitionUpdate(metadata, key, holder, deletionInfo, false); } /** @@ -149,8 +148,8 @@ public class PartitionUpdate extends AbstractBTreePartition public static PartitionUpdate fullPartitionDelete(CFMetaData metadata, DecoratedKey key, long timestamp, int nowInSec) { MutableDeletionInfo deletionInfo = new MutableDeletionInfo(timestamp, nowInSec); - Holder holder = new Holder(BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); - return new PartitionUpdate(metadata, key, PartitionColumns.NONE, holder, deletionInfo, false); + Holder holder = new Holder(PartitionColumns.NONE, BTree.empty(), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); + return new PartitionUpdate(metadata, key, holder, deletionInfo, false); } /** @@ -167,13 +166,13 @@ public class PartitionUpdate extends AbstractBTreePartition MutableDeletionInfo deletionInfo = MutableDeletionInfo.live(); if (row.isStatic()) { - Holder holder = new Holder(BTree.empty(), deletionInfo, row, EncodingStats.NO_STATS); - return new PartitionUpdate(metadata, key, new PartitionColumns(Columns.from(row.columns()), Columns.NONE), holder, deletionInfo, false); + Holder holder = new Holder(new PartitionColumns(Columns.from(row.columns()), Columns.NONE), BTree.empty(), deletionInfo, row, EncodingStats.NO_STATS); + return new PartitionUpdate(metadata, key, holder, deletionInfo, false); } else { - Holder holder = new Holder(BTree.singleton(row), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); - return new PartitionUpdate(metadata, key, new PartitionColumns(Columns.NONE, Columns.from(row.columns())), holder, deletionInfo, false); + Holder holder = new Holder(new PartitionColumns(Columns.NONE, Columns.from(row.columns())), BTree.singleton(row), deletionInfo, Rows.EMPTY_STATIC_ROW, EncodingStats.NO_STATS); + return new PartitionUpdate(metadata, key, holder, deletionInfo, false); } } @@ -201,14 +200,14 @@ public class PartitionUpdate extends AbstractBTreePartition { Holder holder = build(iterator, 16); MutableDeletionInfo deletionInfo = (MutableDeletionInfo) holder.deletionInfo; - return new PartitionUpdate(iterator.metadata(), iterator.partitionKey(), iterator.columns(), holder, deletionInfo, false); + return new PartitionUpdate(iterator.metadata(), iterator.partitionKey(), holder, deletionInfo, false); } public static PartitionUpdate fromIterator(RowIterator iterator) { MutableDeletionInfo deletionInfo = MutableDeletionInfo.live(); Holder holder = build(iterator, deletionInfo, true, 16); - return new PartitionUpdate(iterator.metadata(), iterator.partitionKey(), iterator.columns(), holder, deletionInfo, false); + return new PartitionUpdate(iterator.metadata(), iterator.partitionKey(), holder, deletionInfo, false); } protected boolean canHaveShadowedData() @@ -321,7 +320,7 @@ public class PartitionUpdate extends AbstractBTreePartition Object[] tree = BTree.<Row>transformAndFilter(holder.tree, (x) -> x.updateAllTimestamp(newTimestamp)); Row staticRow = holder.staticRow.updateAllTimestamp(newTimestamp); EncodingStats newStats = EncodingStats.Collector.collect(staticRow, BTree.<Row>iterator(tree), deletionInfo); - this.holder = new Holder(tree, deletionInfo, staticRow, newStats); + this.holder = new Holder(holder.columns, tree, deletionInfo, staticRow, newStats); } /** @@ -356,6 +355,15 @@ public class PartitionUpdate extends AbstractBTreePartition return size; } + @Override + public PartitionColumns columns() + { + // The superclass implementation calls holder(), but that triggers a build of the PartitionUpdate. But since + // the columns are passed to the ctor, we know the holder always has the proper columns even if it doesn't have + // the built rows yet, so just bypass the holder() method. + return holder.columns; + } + protected Holder holder() { maybeBuild(); @@ -534,7 +542,7 @@ public class PartitionUpdate extends AbstractBTreePartition Row staticRow = holder.staticRow.isEmpty() ? row : Rows.merge(holder.staticRow, row, createdAtInSec); - holder = new Holder(holder.tree, holder.deletionInfo, staticRow, holder.stats); + holder = new Holder(holder.columns, holder.tree, holder.deletionInfo, staticRow, holder.stats); } else { @@ -567,7 +575,7 @@ public class PartitionUpdate extends AbstractBTreePartition assert deletionInfo == holder.deletionInfo; EncodingStats newStats = EncodingStats.Collector.collect(holder.staticRow, BTree.<Row>iterator(merged), deletionInfo); - this.holder = new Holder(merged, holder.deletionInfo, holder.staticRow, newStats); + this.holder = new Holder(holder.columns, merged, holder.deletionInfo, holder.staticRow, newStats); rowBuilder = null; isBuilt = true; } @@ -649,8 +657,7 @@ public class PartitionUpdate extends AbstractBTreePartition MutableDeletionInfo deletionInfo = deletionBuilder.build(); return new PartitionUpdate(metadata, header.key, - header.sHeader.columns(), - new Holder(rows.build(), deletionInfo, header.staticRow, header.sHeader.stats()), + new Holder(header.sHeader.columns(), rows.build(), deletionInfo, header.staticRow, header.sHeader.stats()), deletionInfo, false); }