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/cassandra-3.11 Commit: 0a7fbee43f25b6ad3172825cd29bae455223ab33 Parents: c9dc69d Author: Sam Tunnicliffe <s...@beobal.com> Authored: Mon Nov 26 17:41:06 2018 +0000 Committer: Sam Tunnicliffe <s...@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-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org