From c4064dd80e427aec7c04e8e2e1e4630d6c8087b6 Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Mon, 18 May 2020 16:53:08 +0200 Subject: [PATCH] Allow recovery from the cases when CQL-created compact sense tables have bytes in EmptyType columns. Patch by Alex Petrov; reviewed by Sylvain Lebresne for CASSANDRA-15778. --- .../apache/cassandra/config/CFMetaData.java | 21 ++++++++++ .../cql3/statements/AlterTableStatement.java | 23 ++++++++++- .../cassandra/db/marshal/AbstractType.java | 12 +++++- .../schema/LegacySchemaMigrator.java | 9 +++-- .../serializers/EmptySerializer.java | 6 ++- ...e_table_with_bytes-ka-1-CompressionInfo.db | Bin 0 -> 43 bytes ...reated_dense_table_with_bytes-ka-1-Data.db | Bin 0 -> 56 bytes ...ed_dense_table_with_bytes-ka-1-Digest.sha1 | 1 + ...ated_dense_table_with_bytes-ka-1-Filter.db | Bin 0 -> 16 bytes ...eated_dense_table_with_bytes-ka-1-Index.db | Bin 0 -> 19 bytes ..._dense_table_with_bytes-ka-1-Statistics.db | Bin 0 -> 4450 bytes ...ted_dense_table_with_bytes-ka-1-Summary.db | Bin 0 -> 95 bytes ...reated_dense_table_with_bytes-ka-1-TOC.txt | 8 ++++ ...nse_table_with_int-ka-1-CompressionInfo.db | Bin 0 -> 43 bytes ..._created_dense_table_with_int-ka-1-Data.db | Bin 0 -> 49 bytes ...ated_dense_table_with_int-ka-1-Digest.sha1 | 1 + ...reated_dense_table_with_int-ka-1-Filter.db | Bin 0 -> 16 bytes ...created_dense_table_with_int-ka-1-Index.db | Bin 0 -> 19 bytes ...ed_dense_table_with_int-ka-1-Statistics.db | Bin 0 -> 4450 bytes ...eated_dense_table_with_int-ka-1-Summary.db | Bin 0 -> 95 bytes ..._created_dense_table_with_int-ka-1-TOC.txt | 8 ++++ .../cql3/validation/operations/AlterTest.java | 22 +++++++++++ .../org/apache/cassandra/db/ScrubTest.java | 3 +- .../cassandra/db/marshal/EmptyTypeTest.java | 2 +- .../io/sstable/LegacySSTableTest.java | 37 ++++++++++++++++++ 25 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-CompressionInfo.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Data.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Digest.sha1 create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Filter.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Index.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Statistics.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Summary.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-TOC.txt create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-CompressionInfo.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Data.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Digest.sha1 create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Filter.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Index.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Statistics.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Summary.db create mode 100644 test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-TOC.txt diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java index 68e06be7f3..19f744ed10 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -615,6 +615,27 @@ public final class CFMetaData this); } + public CFMetaData copyWithNewCompactValueType(AbstractType type) + { + assert isDense && compactValueColumn.type instanceof EmptyType && partitionColumns.size() == 1; + return copyOpts(new CFMetaData(ksName, + cfName, + cfId, + isSuper, + isCounter, + isDense, + isCompound, + isView, + copy(partitionKeyColumns), + copy(clusteringColumns), + PartitionColumns.of(compactValueColumn.withNewType(type)), + partitioner, + superCfKeyColumn, + superCfValueColumn), + this); + } + + private static List copy(List l) { List copied = new ArrayList<>(l.size()); diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java index 5bff24ee98..193c24c5e7 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java @@ -30,6 +30,8 @@ import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.BytesType; +import org.apache.cassandra.db.marshal.EmptyType; import org.apache.cassandra.db.view.View; import org.apache.cassandra.exceptions.*; import org.apache.cassandra.schema.IndexMetadata; @@ -109,7 +111,26 @@ public class AlterTableStatement extends SchemaAlteringStatement switch (oType) { case ALTER: - throw new InvalidRequestException("Altering of types is not allowed"); + // We do not support altering of types and only allow this to for people who have already one + // through the upgrade of 2.x CQL-created SSTables with Thrift writes, affected by CASSANDRA-15778. + if (meta.isDense() + && meta.compactValueColumn().equals(def) + && meta.compactValueColumn().type instanceof EmptyType + && validator != null) + { + if (validator.getType() instanceof BytesType) + { + cfm = meta.copyWithNewCompactValueType(validator.getType()); + break; + } + + throw new InvalidRequestException(String.format("Compact value type can only be changed to BytesType, but %s was given.", + validator.getType())); + } + else + { + throw new InvalidRequestException("Altering of types is not allowed"); + } case ADD: assert columnName != null; if (meta.isDense()) diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java b/src/java/org/apache/cassandra/db/marshal/AbstractType.java index 72bfa66b35..a15dd48b0d 100644 --- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java +++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java @@ -394,7 +394,11 @@ public abstract class AbstractType implements Comparator public void writeValue(ByteBuffer value, DataOutputPlus out) throws IOException { assert value.hasRemaining(); - if (valueLengthIfFixed() >= 0) + int valueLengthIfFixed = valueLengthIfFixed(); + assert valueLengthIfFixed < 0 || value.remaining() == valueLengthIfFixed : String.format("Expected exactly %d bytes, but was %d", + valueLengthIfFixed, value.remaining()); + + if (valueLengthIfFixed >= 0) out.write(value); else ByteBufferUtil.writeWithVIntLength(value, out); @@ -403,7 +407,11 @@ public abstract class AbstractType implements Comparator public long writtenLength(ByteBuffer value) { assert value.hasRemaining(); - return valueLengthIfFixed() >= 0 + int valueLengthIfFixed = valueLengthIfFixed(); + assert valueLengthIfFixed < 0 || value.remaining() == valueLengthIfFixed : String.format("Expected exactly %d bytes, but was %d", + valueLengthIfFixed, value.remaining()); + + return valueLengthIfFixed >= 0 ? value.remaining() : TypeSizes.sizeofWithVIntLength(value); } diff --git a/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java b/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java index 59df65b162..b7f7e73d0e 100644 --- a/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java +++ b/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java @@ -632,9 +632,12 @@ public final class LegacySchemaMigrator } else { - // For dense compact tables, we get here if we don't have a compact value column, in which case we should add it - // (we use EmptyType to recognize that the compact value was not declared by the use (see CreateTableStatement too)) - defs.add(ColumnDefinition.regularDef(ksName, cfName, names.defaultCompactValueName(), EmptyType.instance)); + // For dense compact tables, we get here if we don't have a compact value column, in which case we should add it. + // We use EmptyType to recognize that the compact value was not declared by the user (see CreateTableStatement). + // If user made any writes to this column, compact value column should be initialized as bytes (see CASSANDRA-15778). + AbstractType compactColumnType = Boolean.getBoolean("cassandra.init_dense_table_compact_value_as_bytes") + ? BytesType.instance : EmptyType.instance; + defs.add(ColumnDefinition.regularDef(ksName, cfName, names.defaultCompactValueName(), compactColumnType)); } } diff --git a/src/java/org/apache/cassandra/serializers/EmptySerializer.java b/src/java/org/apache/cassandra/serializers/EmptySerializer.java index 733e179ef0..352ef2c3e1 100644 --- a/src/java/org/apache/cassandra/serializers/EmptySerializer.java +++ b/src/java/org/apache/cassandra/serializers/EmptySerializer.java @@ -40,7 +40,11 @@ public class EmptySerializer implements TypeSerializer public void validate(ByteBuffer bytes) throws MarshalException { if (bytes.remaining() > 0) - throw new MarshalException("EmptyType only accept empty values"); + { + throw new MarshalException("EmptyType only accept empty values. " + + "A non-empty value can be a result of a Thrift write into CQL-created dense table. " + + "See CASSANDRA-15778 for details."); + } } public String toString(Void value) diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-CompressionInfo.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-CompressionInfo.db new file mode 100644 index 0000000000000000000000000000000000000000..01bde1010210b431b759c4834acef8a4172bfb71 GIT binary patch literal 43 fcmZSJ^@%cZ&d)6b%7 literal 0 HcmV?d00001 diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Statistics.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_bytes/legacy_tables-legacy_ka_cql_created_dense_table_with_bytes-ka-1-Statistics.db new file mode 100644 index 0000000000000000000000000000000000000000..1361f7cc8839b1c6d10b4063791663b7e67b6079 GIT binary patch literal 4450 zcmeI$`%6<%902fp?>6VuEuCg*Ddi(gVp?VlX{N z$Sm;D14LA`cWP45LwlDJS&?C|EQ*>%nP#}|+%A5<=Rb&c;BwCAe9v~zIrp5q!zhXp zQI0g7&uIQy#a|46Eq)I6^vFV(FsAC zd$DQadC&`pxQXx$Y-S4K{n#Sn`$bhaF19a7kR-7He9>(Ywv-sJ`zVfkbYXj0u_qnG zR-VT8UxyvogFPb}dsZ#BDjPe#8at%}J7Y5T<^XKt3hYuHb{(;PyEHg%4MwJezhP&u z#I6?Br=yy-zCdv%OIWv#YUTm9DEkR&FBaqVFYbPX;*xu1$ZmrT$nIG-WZ4*t>^bI( ztT5p9=%e|F;(l%Z$Wz}2A_w(mB8NW3&pT=uKkwM`o2dPw{`bhsl5zjm23k-&?~4+7 zUwRAjsgZ@qt#|PJ^tGb-q&>WU!uGT}^)T|@kb2~Yw_~80h->Km%A8Oht$YUK-Xd)$v~PSkTF+Ay zBPU^eTI11A&@<9bRYq!Ti>%*@uhTid8J^)>I zw;>g}G3!(a^y5nN5$K^SJxhV*PFc}r_oKXA>PzK%NJ8)wh8!2VipNA<){#A)S$ zM}Ja)wRt~B_fMEII?KX4n9PNyj8Jgq|Ld7cm&!)Skq71uP}0%u{9hGZr1=nk9&+S+`9Cz&gBjzAAD#gWQ7F=scpFZk97#QTldE^04 ChYWuJ literal 0 HcmV?d00001 diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Digest.sha1 b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Digest.sha1 new file mode 100644 index 0000000000..f9e4b9c152 --- /dev/null +++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Digest.sha1 @@ -0,0 +1 @@ +1334250623 \ No newline at end of file diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Filter.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Filter.db new file mode 100644 index 0000000000000000000000000000000000000000..b6728a157b88565629cb22beeda809b9781db2cc GIT binary patch literal 16 VcmZQzU|?lnU|?)u079k)1^^0R0to;B literal 0 HcmV?d00001 diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Index.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Index.db new file mode 100644 index 0000000000000000000000000000000000000000..64e12cd4ca60f343b292a8d7931f8103ff7e39ee GIT binary patch literal 19 QcmZQz%}%W}G-N;m02&$ru>b%7 literal 0 HcmV?d00001 diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Statistics.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_cql_created_dense_table_with_int/legacy_tables-legacy_ka_cql_created_dense_table_with_int-ka-1-Statistics.db new file mode 100644 index 0000000000000000000000000000000000000000..13dc64aff359fba35e611ea2a61301a02b5e5eb4 GIT binary patch literal 4450 zcmeI$`%6<%902fp?_SQw(rNi9M-NStn3b6Y3QxgwS8D6_&y{YAno zG81|S5++(%`A7`1w6_l;D=G}uilSzbWf`u!woBjd`46HUxSaDj-?QCw&OPVuFp8o? zlrv4^Gn&7$_>18$6BS*+ZBes@Y;K-Woy!&#vH1p$RU7im>cm}UV;W~Laz;Zs%b87P zQ$fCw)0CII)-?s}=0EI6-gdiv!Y7!fZ(bbm=3V=z)_D_G@lWQsDNK%=!z6T2s1pkX zS1uteB`hQCLD-Y9qpvV2Ao#K$;Q+!xghL6>A{^ziFb@le_)@|tgx3(>LU<418p17v zhp=hldC-f9xS4PlHWNsAKemYYe$jCp7dsXtNRk);z9?OWEhEP3F^=P&z1ZG1>`4{a z%G20W)?tSXVo#68o_z{it;dc#ft}ciojw_RQ!sYXD(p%YyPjCTy;>Z%sgP;aXKejy z>=Sr>(zYik&SVPf*jdfo!xrfuq4r|2u>PIZ;(-S!F1b^MEFEb;_Q`0g?G~P&mu+Z1X;0r#*q+wt4j`A#Jdb?8dmc0s(Td)$%u&T-7#D>UqxB(b z&ymBpxL6(!E!i-H#w#r_6~nl!&GHReu1)?5?WO8K`^iVI?1yn*QECsge_S|P&w+{` z$6$Qwg+uS5XDm#)3?1<_dnt5ue_j*xl1+P0LTjJT%ZAQ4wLKO(H~iXi=p8?Otk6}r z8+6bYGV5kSKddc32>tEqU@|Zr>9e^VIyWf^?ti)=y%p|%+I~a<_djE_yt)oP&kQ%l zmjH{*wB<0cIIK^Z3OzQq2R`yrd-Z%qL0J!&tNTmkk>JJZ7fdvC1z1jny< z=Jy_s-)|tLMG4wZW-ANf??Lc+O@|u9Rd=mQ_`Jft=u#^|d~TL49G(YaV?OjKKwLBQ zwqqyo%Hz$Aus`c~Bpx^;AX|6fx@uCnkBCUc=FBNSZu|8nLMhP^;2xN@||`8xNR3r&#w%!Q_da-X@- ze7MhC?lTuy5dYShi{>KB=36P*_;&uU3NErDh(8ZH^HTl~4b4bSVDf-Dhaa8+3{fc3 Nlz5wQ*Lcurrent * sstable format (version) into {@code test/data/legacy-sstables/VERSION}, where