You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sa...@apache.org on 2018/11/28 16:15:31 UTC
[3/6] cassandra git commit: Reading legacy tables handles tombstones
for dropped collections
Reading legacy tables handles tombstones for dropped collections
Patch by Sam Tunnicliffe; reviewed by Aleksey Yeschenko and
Benedict Elliott Smith for CASSANDRA-14912
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/0a7fbee4
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/0a7fbee4
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/0a7fbee4
Branch: refs/heads/trunk
Commit: 0a7fbee43f25b6ad3172825cd29bae455223ab33
Parents: c9dc69d
Author: Sam Tunnicliffe <sa...@beobal.com>
Authored: Mon Nov 26 17:41:06 2018 +0000
Committer: Sam Tunnicliffe <sa...@beobal.com>
Committed: Wed Nov 28 16:03:16 2018 +0000
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/db/LegacyLayout.java | 33 +++++-
...bles-legacy_ka_14912-ka-1-CompressionInfo.db | Bin 0 -> 43 bytes
.../legacy_tables-legacy_ka_14912-ka-1-Data.db | Bin 0 -> 102 bytes
...gacy_tables-legacy_ka_14912-ka-1-Digest.sha1 | 1 +
...legacy_tables-legacy_ka_14912-ka-1-Filter.db | Bin 0 -> 16 bytes
.../legacy_tables-legacy_ka_14912-ka-1-Index.db | Bin 0 -> 36 bytes
...cy_tables-legacy_ka_14912-ka-1-Statistics.db | Bin 0 -> 4474 bytes
...egacy_tables-legacy_ka_14912-ka-1-Summary.db | Bin 0 -> 80 bytes
.../legacy_tables-legacy_ka_14912-ka-1-TOC.txt | 8 ++
.../cassandra/io/sstable/LegacySSTableTest.java | 103 +++++++++++++++++--
11 files changed, 137 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 3284250..0d33e3c 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.18
+ * Fix handling of collection tombstones for dropped columns from legacy sstables (CASSANDRA-14912)
* Fix missing rows when reading 2.1 SSTables with static columns in 3.0 (CASSANDRA-14873)
* Move TWCS message 'No compaction necessary for bucket size' to Trace level (CASSANDRA-14884)
* Sstable min/max metadata can cause data loss (CASSANDRA-14861)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/src/java/org/apache/cassandra/db/LegacyLayout.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java
index ecc507e..c80594c 100644
--- a/src/java/org/apache/cassandra/db/LegacyLayout.java
+++ b/src/java/org/apache/cassandra/db/LegacyLayout.java
@@ -24,6 +24,7 @@ import java.nio.ByteBuffer;
import java.security.MessageDigest;
import java.util.*;
+import org.apache.cassandra.cql3.ColumnIdentifier;
import org.apache.cassandra.cql3.SuperColumnCompatibility;
import org.apache.cassandra.utils.AbstractIterator;
import com.google.common.collect.Iterators;
@@ -65,6 +66,23 @@ public abstract class LegacyLayout
public final static int COUNTER_UPDATE_MASK = 0x08;
private final static int RANGE_TOMBSTONE_MASK = 0x10;
+ // Used in decodeBound if the number of components in the legacy bound is greater than the clustering size,
+ // indicating a complex column deletion (i.e. a collection tombstone), but the referenced column is either
+ // not present in the current table metadata, or is not currently a complex column. In that case, we'll
+ // check the dropped columns for the table which should contain the previous column definition. If that
+ // previous definition is also not complex (indicating that the column may have been dropped and re-added
+ // with different types multiple times), we use this fake definition to ensure that the complex deletion
+ // can be safely processed. This resulting deletion should be filtered out of any row created by a
+ // CellGrouper by the dropped column check, but this gives us an extra level of confidence as that check
+ // is timestamp based and so is fallible in the face of clock drift.
+ private static final ColumnDefinition INVALID_DROPPED_COMPLEX_SUBSTITUTE_COLUMN =
+ new ColumnDefinition("",
+ "",
+ ColumnIdentifier.getInterned(ByteBufferUtil.EMPTY_BYTE_BUFFER, UTF8Type.instance),
+ SetType.getInstance(UTF8Type.instance, true),
+ ColumnDefinition.NO_POSITION,
+ ColumnDefinition.Kind.REGULAR);
+
private LegacyLayout() {}
public static AbstractType<?> makeLegacyComparator(CFMetaData metadata)
@@ -215,10 +233,15 @@ public abstract class LegacyLayout
// pop the collection name from the back of the list of clusterings
ByteBuffer collectionNameBytes = components.remove(clusteringSize);
collectionName = metadata.getColumnDefinition(collectionNameBytes);
- if (collectionName == null) {
+ if (collectionName == null || !collectionName.isComplex()) {
collectionName = metadata.getDroppedColumnDefinition(collectionNameBytes, isStatic);
+ // if no record of the column having ever existed is found, something is badly wrong
if (collectionName == null)
throw new RuntimeException("Unknown collection column " + UTF8Type.instance.getString(collectionNameBytes) + " during deserialization");
+ // if we do have a record of dropping this column but it wasn't previously complex, use a fake
+ // column definition for safety (see the comment on the constant declaration for details)
+ if (!collectionName.isComplex())
+ collectionName = INVALID_DROPPED_COMPLEX_SUBSTITUTE_COLUMN;
}
}
@@ -1339,6 +1362,14 @@ public abstract class LegacyLayout
if (!helper.includes(tombstone.start.collectionName))
return false; // see CASSANDRA-13109
+ // The helper needs to be informed about the current complex column identifier before
+ // it can perform the comparison between the recorded drop time and the RT deletion time.
+ // If the RT has been superceded by a drop, we still return true as we don't want the
+ // grouper to terminate yet.
+ helper.startOfComplexColumn(tombstone.start.collectionName);
+ if (helper.isDroppedComplexDeletion(tombstone.deletionTime))
+ return true;
+
if (clustering == null)
{
clustering = tombstone.start.getAsClustering(metadata);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-CompressionInfo.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-CompressionInfo.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-CompressionInfo.db
new file mode 100644
index 0000000..cf8c97a
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-CompressionInfo.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Data.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Data.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Data.db
new file mode 100644
index 0000000..19c7d79
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Data.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Digest.sha1
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Digest.sha1 b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Digest.sha1
new file mode 100644
index 0000000..66d3a1c
--- /dev/null
+++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Digest.sha1
@@ -0,0 +1 @@
+2565739962
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Filter.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Filter.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Filter.db
new file mode 100644
index 0000000..1b7fa17
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Filter.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Index.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Index.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Index.db
new file mode 100644
index 0000000..a34ee93
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Index.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Statistics.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Statistics.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Statistics.db
new file mode 100644
index 0000000..405c3e3
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Statistics.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Summary.db
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Summary.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Summary.db
new file mode 100644
index 0000000..9756785
Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-Summary.db differ
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-TOC.txt
----------------------------------------------------------------------
diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-TOC.txt b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-TOC.txt
new file mode 100644
index 0000000..7c351d8
--- /dev/null
+++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14912/legacy_tables-legacy_ka_14912-ka-1-TOC.txt
@@ -0,0 +1,8 @@
+TOC.txt
+Filter.db
+Index.db
+Summary.db
+Data.db
+CompressionInfo.db
+Digest.sha1
+Statistics.db
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0a7fbee4/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java b/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java
index df042e7..4d99081 100644
--- a/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java
+++ b/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java
@@ -25,7 +25,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Random;
-import com.google.common.collect.Iterables;
import org.junit.After;
import org.junit.Assert;
import org.junit.BeforeClass;
@@ -36,20 +35,19 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.SchemaLoader;
-import org.apache.cassandra.cql3.QueryOptions;
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.config.ColumnDefinition;
import org.apache.cassandra.cql3.QueryProcessor;
import org.apache.cassandra.cql3.UntypedResultSet;
-import org.apache.cassandra.cql3.statements.SelectStatement;
import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Keyspace;
-import org.apache.cassandra.db.ReadOrderGroup;
-import org.apache.cassandra.db.SinglePartitionReadCommand;
import org.apache.cassandra.db.SinglePartitionSliceCommandTest;
import org.apache.cassandra.db.compaction.Verifier;
-import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.db.marshal.SetType;
+import org.apache.cassandra.db.marshal.UTF8Type;
import org.apache.cassandra.db.rows.RangeTombstoneMarker;
import org.apache.cassandra.db.rows.Unfiltered;
-import org.apache.cassandra.db.rows.UnfilteredRowIterator;
import org.apache.cassandra.dht.IPartitioner;
import org.apache.cassandra.dht.Range;
import org.apache.cassandra.dht.Token;
@@ -59,7 +57,6 @@ import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.sstable.format.Version;
import org.apache.cassandra.io.sstable.format.big.BigFormat;
import org.apache.cassandra.service.CacheService;
-import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.service.StorageService;
import org.apache.cassandra.streaming.StreamPlan;
import org.apache.cassandra.streaming.StreamSession;
@@ -345,6 +342,96 @@ public class LegacySSTableTest
}
}
+ @Test
+ public void test14912() throws Exception
+ {
+ /*
+ * When reading 2.1 sstables in 3.0, collection tombstones need to be checked against
+ * the dropped columns stored in table metadata. Failure to do so can result in unreadable
+ * rows if a column with the same name but incompatible type has subsequently been added.
+ *
+ * The original (i.e. pre-any ALTER statements) table definition for this test is:
+ * CREATE TABLE legacy_tables.legacy_ka_14912 (k int PRIMARY KEY, v1 set<text>, v2 text);
+ *
+ * The SSTable loaded emulates data being written before the table is ALTERed and contains:
+ *
+ * insert into legacy_tables.legacy_ka_14912 (k, v1, v2) values (0, {}, 'abc') USING TIMESTAMP 1543244999672280;
+ * insert into legacy_tables.legacy_ka_14912 (k, v1, v2) values (1, {'abc'}, 'abc') USING TIMESTAMP 1543244999672280;
+ *
+ * The timestamps of the (generated) collection tombstones are 1543244999672279, e.g. the <TIMESTAMP of the mutation> - 1
+ */
+
+ QueryProcessor.executeInternal("CREATE TABLE legacy_tables.legacy_ka_14912 (k int PRIMARY KEY, v1 text, v2 text)");
+ loadLegacyTable("legacy_%s_14912%s", "ka", "");
+ CFMetaData cfm = Keyspace.open("legacy_tables").getColumnFamilyStore("legacy_ka_14912").metadata;
+ ColumnDefinition columnToDrop;
+
+ /*
+ * This first variant simulates the original v1 set<text> column being dropped
+ * then re-added with the text type:
+ * CREATE TABLE legacy_tables.legacy_ka_14912 (k int PRIMARY KEY, v1 set<text>, v2 text);
+ * INSERT INTO legacy_tables.legacy)ka_14912 (k, v1, v2)...
+ * ALTER TABLE legacy_tables.legacy_ka_14912 DROP v1;
+ * ALTER TABLE legacy_tables.legacy_ka_14912 ADD v1 text;
+ */
+ columnToDrop = ColumnDefinition.regularDef(cfm,
+ UTF8Type.instance.fromString("v1"),
+ SetType.getInstance(UTF8Type.instance, true));
+ cfm.recordColumnDrop(columnToDrop, 1543244999700000L);
+ assertExpectedRowsWithDroppedCollection(true);
+ // repeat the query, but simulate clock drift by shifting the recorded
+ // drop time forward so that it occurs before the collection timestamp
+ cfm.recordColumnDrop(columnToDrop, 1543244999600000L);
+ assertExpectedRowsWithDroppedCollection(false);
+
+ /*
+ * This second test simulates the original v1 set<text> column being dropped
+ * then re-added with some other, non-collection type (overwriting the dropped
+ * columns record), then dropping and re-adding again as text type:
+ * CREATE TABLE legacy_tables.legacy_ka_14912 (k int PRIMARY KEY, v1 set<text>, v2 text);
+ * INSERT INTO legacy_tables.legacy_ka_14912 (k, v1, v2)...
+ * ALTER TABLE legacy_tables.legacy_ka_14912 DROP v1;
+ * ALTER TABLE legacy_tables.legacy_ka_14912 ADD v1 blob;
+ * ALTER TABLE legacy_tables.legacy_ka_14912 DROP v1;
+ * ALTER TABLE legacy_tables.legacy_ka_14912 ADD v1 text;
+ */
+ columnToDrop = ColumnDefinition.regularDef(cfm,
+ UTF8Type.instance.fromString("v1"),
+ BytesType.instance);
+ cfm.recordColumnDrop(columnToDrop, 1543244999700000L);
+ assertExpectedRowsWithDroppedCollection(true);
+ // repeat the query, but simulate clock drift by shifting the recorded
+ // drop time forward so that it occurs before the collection timestamp
+ cfm.recordColumnDrop(columnToDrop, 1543244999600000L);
+ assertExpectedRowsWithDroppedCollection(false);
+ }
+
+ private void assertExpectedRowsWithDroppedCollection(boolean droppedCheckSuccessful)
+ {
+ for (int i=0; i<=1; i++)
+ {
+ UntypedResultSet rows =
+ QueryProcessor.executeOnceInternal(
+ String.format("SELECT * FROM legacy_tables.legacy_ka_14912 WHERE k = %s;", i));
+ Assert.assertEquals(1, rows.size());
+ UntypedResultSet.Row row = rows.one();
+
+ // If the best-effort attempt to filter dropped columns was successful, then the row
+ // should not contain the v1 column at all. Likewise, if no column data was written,
+ // only a tombstone, then no v1 column should be present.
+ // However, if collection data was written (i.e. where k=1), then if the dropped column
+ // check didn't filter the legacy cells, we should expect an empty column value as the
+ // legacy collection tombstone won't cover it and the dropped column check doesn't filter
+ // it.
+ if (droppedCheckSuccessful || i == 0)
+ Assert.assertFalse(row.has("v1"));
+ else
+ Assert.assertEquals("", row.getString("v1"));
+
+ Assert.assertEquals("abc", row.getString("v2"));
+ }
+ }
+
private void streamLegacyTables(String legacyVersion) throws Exception
{
for (int compact = 0; compact <= 1; compact++)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org