Merge branch 'cassandra-6.0' into trunk

netudima · netudima · commit c588b674bf0a · 2026-05-28T15:20:11.000+01:00
* cassandra-6.0: Avoid type lookup in SerializationHeader#getType if schema and SSTable are aligned
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -3,6 +3,7 @@
  * Allow nodetool garbagecollect to take a user defined list of SSTables (CASSANDRA-16767)
  * Add a guardrail for misprepared statements (CASSANDRA-21139)
 Merged from 6.0:
+ * Avoid type lookup in SerializationHeader#getType if schema and SSTable are aligned (CASSANDRA-21402)
  * Replace LongAdder in metric-like logic with ThreadLocalCounter (CASSANDRA-21400)
  * BTreeRow#hasLiveData: Avoid Cell iteration if there are no tombstones in a row (CASSANDRA-21363)
  * Reduce memory allocations in row merge logic (CASSANDRA-21359)
diff --git a/src/java/org/apache/cassandra/db/SerializationHeader.java b/src/java/org/apache/cassandra/db/SerializationHeader.java
@@ -349,6 +349,7 @@ public MetadataType getType()
         public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumnException
         {
             Map<ByteBuffer, AbstractType<?>> typeMap = new HashMap<>(staticColumns.size() + regularColumns.size());
+            boolean hasTypeMismatch = false;
 
             RegularAndStaticColumns.Builder builder = RegularAndStaticColumns.builder();
             for (Map<ByteBuffer, AbstractType<?>> map : ImmutableList.of(staticColumns, regularColumns))
@@ -357,9 +358,10 @@ public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumn
                 for (Map.Entry<ByteBuffer, AbstractType<?>> e : map.entrySet())
                 {
                     ByteBuffer name = e.getKey();
-                    AbstractType<?> other = typeMap.put(name, e.getValue());
-                    if (other != null && !other.equals(e.getValue()))
-                        throw new IllegalStateException("Column " + name + " occurs as both regular and static with types " + other + "and " + e.getValue());
+                    AbstractType<?> diskType = e.getValue();
+                    AbstractType<?> other = typeMap.put(name, diskType);
+                    if (other != null && !other.equals(diskType))
+                        throw new IllegalStateException("Column " + name + " occurs as both regular and static with types " + other + "and " + diskType);
 
                     ColumnMetadata column = metadata.getColumn(name);
                     if (column == null || column.isStatic() != isStatic)
@@ -376,11 +378,15 @@ public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumn
                         if (column == null)
                             throw new UnknownColumnException("Unknown column " + UTF8Type.instance.getString(name) + " during deserialization");
                     }
+                    if (!diskType.equals(column.type))
+                        hasTypeMismatch = true;
                     builder.add(column);
                 }
             }
 
-            return new SerializationHeader(true, keyType, clusteringTypes, builder.build(), stats, typeMap);
+            // if all on-disk types match the current schema then we can use
+            // fast column.type path on every cell read instead of doing a map lookup.
+            return new SerializationHeader(true, keyType, clusteringTypes, builder.build(), stats, hasTypeMismatch ? typeMap : null);
         }
 
         @Override
diff --git a/test/unit/org/apache/cassandra/db/SerializationHeaderTest.java b/test/unit/org/apache/cassandra/db/SerializationHeaderTest.java
@@ -36,12 +36,15 @@
 import org.apache.cassandra.db.compaction.OperationType;
 import org.apache.cassandra.db.lifecycle.LifecycleTransaction;
 import org.apache.cassandra.db.marshal.Int32Type;
+import org.apache.cassandra.db.marshal.ListType;
+import org.apache.cassandra.db.marshal.LongType;
 import org.apache.cassandra.db.partitions.PartitionUpdate;
 import org.apache.cassandra.db.rows.BTreeRow;
 import org.apache.cassandra.db.rows.BufferCell;
 import org.apache.cassandra.db.rows.Cell;
 import org.apache.cassandra.db.rows.Row;
 import org.apache.cassandra.db.rows.UnfilteredRowIterator;
+import org.apache.cassandra.exceptions.UnknownColumnException;
 import org.apache.cassandra.io.sstable.Descriptor;
 import org.apache.cassandra.io.sstable.ISSTableScanner;
 import org.apache.cassandra.io.sstable.SequenceBasedSSTableId;
@@ -63,7 +66,172 @@ public class SerializationHeaderTest
     {
         DatabaseDescriptor.daemonInitialization();
     }
-    
+
+    /**
+     * Common case: schema unchanged since the SSTable was written.
+     */
+    @Test
+    public void testTypeMapNullWhenNoSchemaChanges() throws UnknownColumnException
+    {
+        TableMetadata schema = TableMetadata.builder(KEYSPACE, "testTypeMapNull")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", Int32Type.instance)
+                .build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schema).toComponent();
+        SerializationHeader header = component.toHeader(schema);
+
+        ColumnMetadata v = schema.getColumn(ColumnIdentifier.getInterned("v", false));
+        // typeMap == null => getType() returns exactly column.type (same object reference)
+        Assert.assertSame(v.type, header.getType(v));
+    }
+
+    /**
+     * ALTER TABLE scenario: column was written as Int32Type, schema now has LongType.
+     * toHeader() must retain typeMap so getType() returns the on-disk Int32Type and
+     * not the current schema LongType, which would corrupt deserialization.
+     */
+    @Test
+    public void testTypeMapNonNullWhenColumnTypeChanged() throws UnknownColumnException
+    {
+        ColumnIdentifier v = ColumnIdentifier.getInterned("v", false);
+        TableMetadata schemaAtWrite = TableMetadata.builder(KEYSPACE, "testTypeMapNonNull")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", Int32Type.instance)
+                .build();
+        TableMetadata schemaAfterAlter = TableMetadata.builder(KEYSPACE, "testTypeMapNonNull")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", LongType.instance) // type is changed
+                .build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schemaAtWrite).toComponent();
+        SerializationHeader header = component.toHeader(schemaAfterAlter);
+
+        ColumnMetadata vNew = schemaAfterAlter.getColumn(v);
+        // getType() must return the on-disk Int32Type, not the current LongType
+        Assert.assertEquals(Int32Type.instance, header.getType(vNew));
+    }
+
+    /**
+     * Column was dropped at the same type it was written with — no actual type mismatch.
+     */
+    @Test
+    public void testTypeMapNullForDroppedColumnWithSameType() throws UnknownColumnException
+    {
+        ColumnIdentifier v = ColumnIdentifier.getInterned("v", false);
+        TableMetadata schemaAtWrite = TableMetadata.builder(KEYSPACE, "testTypeMapDroppedSame")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", Int32Type.instance)
+                .build();
+        ColumnMetadata vColumn = schemaAtWrite.getColumn(v);
+        TableMetadata schemaAfterDrop = schemaAtWrite.unbuild().recordColumnDrop(vColumn, 0L).build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schemaAtWrite).toComponent();
+        SerializationHeader header = component.toHeader(schemaAfterDrop);
+
+        ColumnMetadata vDropped = schemaAfterDrop.getDroppedColumn(vColumn.name.bytes);
+        // disk type == dropped type => typeMap null => same reference
+        Assert.assertSame(vDropped.type, header.getType(vDropped));
+        Assert.assertEquals(Int32Type.instance, header.getType(vDropped));
+    }
+
+    /**
+     * Column was altered from Int32Type to LongType and then dropped.
+     * An SSTable written before the ALTER stores Int32Type on disk, but the drop record
+     * has LongType. The type mismatch must be detected and typeMap retained so
+     * the Int32Type bytes can still be skipped correctly.
+     */
+    @Test
+    public void testTypeMapNonNullForDroppedColumnWithChangedType() throws UnknownColumnException
+    {
+        ColumnIdentifier v = ColumnIdentifier.getInterned("v", false);
+        TableMetadata schemaAtWrite = TableMetadata.builder(KEYSPACE, "testTypeMapDroppedChanged")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", Int32Type.instance)
+                .build();
+        TableMetadata schemaWithAlteredType = TableMetadata.builder(KEYSPACE, "testTypeMapDroppedChanged")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", LongType.instance) // type is changed
+                .build();
+        ColumnMetadata vLong = schemaWithAlteredType.getColumn(v);
+        TableMetadata schemaAfterDrop = schemaWithAlteredType.unbuild()
+                                                             .recordColumnDrop(vLong, 0L) // drop the value column
+                                                             .build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schemaAtWrite).toComponent();
+        SerializationHeader header = component.toHeader(schemaAfterDrop);
+
+        ColumnMetadata vDropped = schemaAfterDrop.getDroppedColumn(vLong.name.bytes);
+        // disk type (Int32) != drop-record type (Long) => typeMap retained, returns Int32
+        Assert.assertEquals(Int32Type.instance, header.getType(vDropped));
+        Assert.assertNotSame(vDropped.type, header.getType(vDropped));
+    }
+
+    /**
+     * Static-column variant of {@link #testTypeMapNonNullWhenColumnTypeChanged}.
+     * The mismatch detection in toHeader() iterates staticColumns and regularColumns through
+     * the same loop; this covers the static branch.
+     */
+    @Test
+    public void testTypeMapNonNullWhenStaticColumnTypeChanged() throws UnknownColumnException
+    {
+        ColumnIdentifier s = ColumnIdentifier.getInterned("s", false);
+        TableMetadata schemaAtWrite = TableMetadata.builder(KEYSPACE, "testTypeMapNonNullStatic")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addStaticColumn("s", Int32Type.instance)
+                .build();
+        TableMetadata schemaAfterAlter = TableMetadata.builder(KEYSPACE, "testTypeMapNonNullStatic")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addStaticColumn("s", LongType.instance) // static column type is changed
+                .build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schemaAtWrite).toComponent();
+        SerializationHeader header = component.toHeader(schemaAfterAlter);
+
+        ColumnMetadata sNew = schemaAfterAlter.getColumn(s);
+        // getType() must return the on-disk Int32Type, not the current LongType
+        Assert.assertEquals(Int32Type.instance, header.getType(sNew));
+    }
+
+    /**
+     * Multi-cell vs frozen collections serialize differently and CollectionType.equals
+     * checks isMultiCell. A schema flipping a list from multi-cell to frozen (or vice versa)
+     * must be detected as a type mismatch so typeMap is retained.
+     */
+    @Test
+    public void testTypeMapNonNullWhenCollectionMultiCellChanged() throws UnknownColumnException
+    {
+        ColumnIdentifier v = ColumnIdentifier.getInterned("v", false);
+        ListType<Integer> multiCellList = ListType.getInstance(Int32Type.instance, true);
+        ListType<Integer> frozenList = ListType.getInstance(Int32Type.instance, false);
+
+        TableMetadata schemaAtWrite = TableMetadata.builder(KEYSPACE, "testTypeMapNonNullCollection")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", multiCellList)
+                .build();
+        TableMetadata schemaAfterFreeze = TableMetadata.builder(KEYSPACE, "testTypeMapNonNullCollection")
+                .addPartitionKeyColumn("k", Int32Type.instance)
+                .addClusteringColumn("c", Int32Type.instance)
+                .addRegularColumn("v", frozenList) // multi-cell -> frozen
+                .build();
+
+        SerializationHeader.Component component = SerializationHeader.makeWithoutStats(schemaAtWrite).toComponent();
+        SerializationHeader header = component.toHeader(schemaAfterFreeze);
+
+        ColumnMetadata vNew = schemaAfterFreeze.getColumn(v);
+        // getType() must return the on-disk multi-cell list, not the frozen schema type
+        Assert.assertEquals(multiCellList, header.getType(vNew));
+    }
+
     @Test
     public void testWrittenAsDifferentKind() throws Exception
     {

Original file line number	Diff line number	Diff line change
`@@ -349,6 +349,7 @@ public MetadataType getType()`
`349`	`349`	`public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumnException`
`350`	`350`	`{`
`351`	`351`	`Map<ByteBuffer, AbstractType<?>> typeMap = new HashMap<>(staticColumns.size() + regularColumns.size());`
	`352`	`+ boolean hasTypeMismatch = false;`
`352`	`353`
`353`	`354`	`RegularAndStaticColumns.Builder builder = RegularAndStaticColumns.builder();`
`354`	`355`	`for (Map<ByteBuffer, AbstractType<?>> map : ImmutableList.of(staticColumns, regularColumns))`
`@@ -357,9 +358,10 @@ public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumn`
`357`	`358`	`for (Map.Entry<ByteBuffer, AbstractType<?>> e : map.entrySet())`
`358`	`359`	`{`
`359`	`360`	`ByteBuffer name = e.getKey();`
`360`		`- AbstractType<?> other = typeMap.put(name, e.getValue());`
`361`		`- if (other != null && !other.equals(e.getValue()))`
`362`		`- throw new IllegalStateException("Column " + name + " occurs as both regular and static with types " + other + "and " + e.getValue());`
	`361`	`+ AbstractType<?> diskType = e.getValue();`
	`362`	`+ AbstractType<?> other = typeMap.put(name, diskType);`
	`363`	`+ if (other != null && !other.equals(diskType))`
	`364`	`+ throw new IllegalStateException("Column " + name + " occurs as both regular and static with types " + other + "and " + diskType);`
`363`	`365`
`364`	`366`	`ColumnMetadata column = metadata.getColumn(name);`
`365`	`367`	`if (column == null \|\| column.isStatic() != isStatic)`
`@@ -376,11 +378,15 @@ public SerializationHeader toHeader(TableMetadata metadata) throws UnknownColumn`
`376`	`378`	`if (column == null)`
`377`	`379`	`throw new UnknownColumnException("Unknown column " + UTF8Type.instance.getString(name) + " during deserialization");`
`378`	`380`	`}`
	`381`	`+ if (!diskType.equals(column.type))`
	`382`	`+ hasTypeMismatch = true;`
`379`	`383`	`builder.add(column);`
`380`	`384`	`}`
`381`	`385`	`}`
`382`	`386`
`383`		`- return new SerializationHeader(true, keyType, clusteringTypes, builder.build(), stats, typeMap);`
	`387`	`+ // if all on-disk types match the current schema then we can use`
	`388`	`+ // fast column.type path on every cell read instead of doing a map lookup.`
	`389`	`+ return new SerializationHeader(true, keyType, clusteringTypes, builder.build(), stats, hasTypeMismatch ? typeMap : null);`
`384`	`390`	`}`
`385`	`391`
`386`	`392`	`@Override`