From c481e8dc84c713bda21724368094850ff9150011 Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Mon, 18 Jul 2016 18:22:48 +0200 Subject: [PATCH] Fix problem with undeleteable rows on upgrade to new sstable format. Patch by Alex Petrov; reviewed by Sylvain Lebresne for CASSANDRA-12144. --- CHANGES.txt | 1 + .../org/apache/cassandra/db/LegacyLayout.java | 19 +++- .../cassandra/db/compaction/Scrubber.java | 49 +++++++++- .../io/sstable/SSTableIdentityIterator.java | 9 +- .../lb-1-big-CompressionInfo.db | Bin 0 -> 43 bytes .../cf_with_duplicates_2_0/lb-1-big-Data.db | Bin 0 -> 84 bytes .../lb-1-big-Digest.adler32 | 1 + .../cf_with_duplicates_2_0/lb-1-big-Filter.db | Bin 0 -> 16 bytes .../cf_with_duplicates_2_0/lb-1-big-Index.db | Bin 0 -> 18 bytes .../lb-1-big-Statistics.db | Bin 0 -> 4474 bytes .../lb-1-big-Summary.db | Bin 0 -> 84 bytes .../cf_with_duplicates_2_0/lb-1-big-TOC.txt | 8 ++ .../mb-3-big-CompressionInfo.db | Bin 0 -> 51 bytes .../cf_with_duplicates_3_0/mb-3-big-Data.db | Bin 0 -> 72 bytes .../mb-3-big-Digest.crc32 | 1 + .../cf_with_duplicates_3_0/mb-3-big-Filter.db | Bin 0 -> 16 bytes .../cf_with_duplicates_3_0/mb-3-big-Index.db | Bin 0 -> 8 bytes .../mb-3-big-Statistics.db | Bin 0 -> 4664 bytes .../mb-3-big-Summary.db | Bin 0 -> 56 bytes .../cf_with_duplicates_3_0/mb-3-big-TOC.txt | 8 ++ .../org/apache/cassandra/db/ScrubTest.java | 88 +++++++++++++++++- 21 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-CompressionInfo.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Data.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Digest.adler32 create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Filter.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Index.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Statistics.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Summary.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-TOC.txt create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-CompressionInfo.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Data.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Digest.crc32 create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Filter.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Index.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Statistics.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Summary.db create mode 100644 test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-TOC.txt diff --git a/CHANGES.txt b/CHANGES.txt index 59f0a5fc78..f205e0b911 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.9 + * Fix problem with undeleteable rows on upgrade to new sstable format (CASSANDRA-12144) * Fix paging logic for deleted partitions with static columns (CASSANDRA-12107) * Wait until the message is being send to decide which serializer must be used (CASSANDRA-11393) * Fix migration of static thrift column names with non-text comparators (CASSANDRA-12147) diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java index 495a12ab72..3feb1f44cf 100644 --- a/src/java/org/apache/cassandra/db/LegacyLayout.java +++ b/src/java/org/apache/cassandra/db/LegacyLayout.java @@ -1205,9 +1205,26 @@ public abstract class LegacyLayout { if (tombstone.isRowDeletion(metadata)) { - // If we're already within a row, it can't be the same one if (clustering != null) + { + // If we're already in the row, there might be a chance that there were two range tombstones + // written, as 2.x storage format does not guarantee just one range tombstone, unlike 3.x. + // We have to make sure that clustering matches, which would mean that tombstone is for the + // same row. + if (rowDeletion != null && clustering.equals(tombstone.start.getAsClustering(metadata))) + { + // If the tombstone superceeds the previous delete, we discard the previous one + if (tombstone.deletionTime.supersedes(rowDeletion.deletionTime)) + { + builder.addRowDeletion(Row.Deletion.regular(tombstone.deletionTime)); + rowDeletion = tombstone; + } + return true; + } + + // If we're already within a row and there was no delete written before that one, it can't be the same one return false; + } clustering = tombstone.start.getAsClustering(metadata); builder.newRow(clustering); diff --git a/src/java/org/apache/cassandra/db/compaction/Scrubber.java b/src/java/org/apache/cassandra/db/compaction/Scrubber.java index d824d0456f..539c4c71af 100644 --- a/src/java/org/apache/cassandra/db/compaction/Scrubber.java +++ b/src/java/org/apache/cassandra/db/compaction/Scrubber.java @@ -34,10 +34,7 @@ import org.apache.cassandra.io.sstable.format.SSTableWriter; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.io.util.RandomAccessReader; import org.apache.cassandra.service.ActiveRepairService; -import org.apache.cassandra.utils.ByteBufferUtil; -import org.apache.cassandra.utils.JVMStabilityInspector; -import org.apache.cassandra.utils.OutputHandler; -import org.apache.cassandra.utils.UUIDGen; +import org.apache.cassandra.utils.*; public class Scrubber implements Closeable { @@ -216,7 +213,7 @@ public class Scrubber implements Closeable if (indexFile != null && dataStart != dataStartFromIndex) outputHandler.warn(String.format("Data file row position %d differs from index file row position %d", dataStart, dataStartFromIndex)); - try (UnfilteredRowIterator iterator = withValidation(new SSTableIdentityIterator(sstable, dataFile, key), dataFile.getPath())) + try (UnfilteredRowIterator iterator = withValidation(new RowMergingSSTableIterator(sstable, dataFile, key), dataFile.getPath())) { if (prevKey != null && prevKey.compareTo(key) > 0) { @@ -470,4 +467,46 @@ public class Scrubber implements Closeable this.emptyRows = scrubber.emptyRows; } } + + /** + * During 2.x migration, under some circumstances rows might have gotten duplicated. + * Merging iterator merges rows with same clustering. + * + * For more details, refer to CASSANDRA-12144. + */ + private static class RowMergingSSTableIterator extends SSTableIdentityIterator + { + RowMergingSSTableIterator(SSTableReader sstable, RandomAccessReader file, DecoratedKey key) + { + super(sstable, file, key); + } + + @Override + protected Unfiltered doCompute() + { + if (!iterator.hasNext()) + return endOfData(); + + Unfiltered next = iterator.next(); + if (!next.isRow()) + return next; + + while (iterator.hasNext()) + { + Unfiltered peek = iterator.peek(); + // If there was a duplicate row, merge it. + if (next.clustering().equals(peek.clustering()) && peek.isRow()) + { + iterator.next(); // Make sure that the peeked item was consumed. + next = Rows.merge((Row) next, (Row) peek, FBUtilities.nowInSeconds()); + } + else + { + break; + } + } + + return next; + } + } } diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java index 6fbc690b8b..a5af334af0 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java @@ -34,7 +34,7 @@ public class SSTableIdentityIterator extends AbstractIterator implem private final DeletionTime partitionLevelDeletion; private final String filename; - private final SSTableSimpleIterator iterator; + protected final SSTableSimpleIterator iterator; private final Row staticRow; /** @@ -97,7 +97,7 @@ public class SSTableIdentityIterator extends AbstractIterator implem { try { - return iterator.hasNext() ? iterator.next() : endOfData(); + return doCompute(); } catch (IndexOutOfBoundsException e) { @@ -118,6 +118,11 @@ public class SSTableIdentityIterator extends AbstractIterator implem } } + protected Unfiltered doCompute() + { + return iterator.hasNext() ? iterator.next() : endOfData(); + } + public void close() { // creator is responsible for closing file when finished diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-CompressionInfo.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-CompressionInfo.db new file mode 100644 index 0000000000000000000000000000000000000000..307eeb352b036a37d6f656855217b07a38062384 GIT binary patch literal 43 fcmZSJ^@%cZ&d)6;M1%-@w4gAjmGlV8ZlYfQuoBF}!8-A6W(!F+EWRet8B~Auc-x lCVhrNR`Z<2Db2OK3?kf;44RC|QVazkBbXT&t_JZp0RR|N5jp?> literal 0 HcmV?d00001 diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Digest.adler32 b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Digest.adler32 new file mode 100644 index 0000000000..ad624d2584 --- /dev/null +++ b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Digest.adler32 @@ -0,0 +1 @@ +408097082 \ No newline at end of file diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Filter.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Filter.db new file mode 100644 index 0000000000000000000000000000000000000000..00a88b4c70c08323c828c7192767bf3d8cbf7a7a GIT binary patch literal 16 XcmZQzU|?lnU|?iWVo+dWU|<3O0rCJ% literal 0 HcmV?d00001 diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Index.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Index.db new file mode 100644 index 0000000000000000000000000000000000000000..c3b42d852590c138eeebfc87cb56f246e91b25ae GIT binary patch literal 18 QcmZQzVPIfj1Y$S<003hE1^@s6 literal 0 HcmV?d00001 diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Statistics.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-Statistics.db new file mode 100644 index 0000000000000000000000000000000000000000..056cf17efb2bdc83b73c23c0f1129fca7cd7478b GIT binary patch literal 4474 zcmeI$dr(YK90%}w?(X(dOIxDQc;ykZ8A;4w#Wt1kNQu0yc1Ib?Y>OrgQ~9SMF_LB| zkD11#(U3>Q@CVBK(O@w&F)M~)F&>d=-PK?X30Ywd>81i3YPJ!;+De zX)@~ya$m%q2;3wc=uEy2hojRkn6V=}JG>>A{weJ`fh+rG=R6@z&J)ulUz}W~lryfq zin9l2PtIPPeK`AaR&(~}9LTvZ=K-9Da1Q4@mh&Xe@to&#Ud4H{%W^xHaPeBsRy5=K zgN^6n7S0#ZVjs?VXa)ECiUSx|rl3`kXphNgPx*db#a~k+@mu9tTy6!^w+5dxSQ?eDX`x+@tYciiAe$faow7&wbQC_S1O3 zF?xmlgp15L5Iko=jm<7~H6>FoY)56Y9rQYzNd z@#M0>)VE6uc2R#j(;Q38M)<9)r%szboz8z&m3WTMf9BY$rt@Dk*;>w%CiY!k^nu>L`icKr zdjJ0S<7+jfes4qJI{H2cKBjBXl6dI#HVy5sp`YUt3Q2r)iam_32cjb1HL6KmXMNqU zk$C2TnreDIi|rGm=y=Xd(pMIf`UNZNnp%m=pFO`&Nqq6h(HeTaw#(to(cN*T$-nh$ z8gatvuN`^a7P4J=d;M1& literal 0 HcmV?d00001 diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-TOC.txt b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-TOC.txt new file mode 100644 index 0000000000..ceb1dab7c8 --- /dev/null +++ b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_2_0/lb-1-big-TOC.txt @@ -0,0 +1,8 @@ +CompressionInfo.db +Digest.adler32 +TOC.txt +Filter.db +Data.db +Index.db +Statistics.db +Summary.db diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-CompressionInfo.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-CompressionInfo.db new file mode 100644 index 0000000000000000000000000000000000000000..3c39b5daacb50e7f7394e0dee0f548fd7d2800f6 GIT binary patch literal 51 kcmZSJ^@%cZ&d)6;M1%-@w4gARxfbu#kyeh~Z(2+4@mjwWGSZF6ukvc4$R7j1^ zqYq7NX=z@@Gw}q*a~LmWyoT{k#+8g4unBWL z$QUN>V0;0a@56Wlwv73H*a{9d@7#yXPis%|YytdDvm~ z*aM=mhaA8b(y(LqVNX7fo!A?DNhr2`7IvYCUCONAIs=Zow8%vJ2|H~z_C9HScAO@z zM<~uGOY64dG=BqImUai_%jJ0e%kSSpamBSFWS@^^$bQLgWdAk`a*wtkWR)4ON44P% ziU(JRAopzyL+;;@ggp2zzTQzS_BOPEDx;@S@lBCe}7kz z=lM_U0&OT3G@$YNWM}5VxNnv78??V+>KEu9+8T5}DbqCfV7!;iSPLB#8-dnyAJvcD zFy8Ojwztp&M<<+wj(nUt9y}%_WZgcj|w)7v&9;H-XYIGj|*FBds4d@=HC%t zb9wh%U_ty_doGgFxaf5>$FD5!7($4^DPor&F>hR(T2T3G9et@G4Nd=PzIJhLq>|c9 zDDCCq?X{yMbYU hejk%AWQjI=hPYfXS{=GkvsdL<$ls@OgqZ$5`!{e@itzvd literal 0 HcmV?d00001 diff --git a/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Summary.db b/test/data/invalid-legacy-sstables/Keyspace1/cf_with_duplicates_3_0/mb-3-big-Summary.db new file mode 100644 index 0000000000000000000000000000000000000000..4547a9451a6c383b420c1e8b235d7444d1e8c901 GIT binary patch literal 56 kcmZQzU}#`qU|