From a443990bfa64e239810876121f2877064f2d9ae8 Mon Sep 17 00:00:00 2001
From: Abe Ratnofsky <abe@aber.io>
Date: Mon, 4 Dec 2023 23:15:05 -0500
Subject: [PATCH] Fix StackOverflowError on ALTER after many previous schema
 changes

Fix re-wrapping of TableMetadataRefCache fields in
Collections.UnmodifiableMap, which was causing long reference chains
where the underlying map was another instance of an UnmodifiableMap.
Calls to `get` had to traverse all the way down the chain, and would
eventually overflow the stack (and created a fair bit of extra garbage
on the heap).

Patch by Abe Ratnofsky; reviewed by Caleb Rackliffe, Jacek Lewandowski for CASSANDRA-19166
---
 CHANGES.txt                                   |  1 +
 .../schema/TableMetadataRefCache.java         | 20 ++++++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 79e8ee7a84..3be443e292 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.1.4
+ * Fix StackOverflowError on ALTER after many previous schema changes (CASSANDRA-19166)
  * Fixed the inconsistency between distributedKeyspaces and distributedAndLocalKeyspaces (CASSANDRA-18747)
  * Internode legacy SSL storage port certificate is not hot reloaded on update (CASSANDRA-18681)
  * Nodetool paxos-only repair is no longer incremental (CASSANDRA-18466)
diff --git a/src/java/org/apache/cassandra/schema/TableMetadataRefCache.java b/src/java/org/apache/cassandra/schema/TableMetadataRefCache.java
index d947d1d690..ab5f283a42 100644
--- a/src/java/org/apache/cassandra/schema/TableMetadataRefCache.java
+++ b/src/java/org/apache/cassandra/schema/TableMetadataRefCache.java
@@ -18,7 +18,6 @@
 
 package org.apache.cassandra.schema;
 
-import java.util.Collections;
 import java.util.Map;
 
 import com.google.common.collect.MapDifference;
@@ -26,6 +25,9 @@ import com.google.common.collect.Maps;
 
 import org.apache.cassandra.utils.Pair;
 
+import static java.util.Collections.emptyMap;
+import static java.util.Collections.unmodifiableMap;
+
 /**
  * Manages the cached {@link TableMetadataRef} objects which holds the references to {@link TableMetadata} objects.
  * <p>
@@ -35,7 +37,7 @@ import org.apache.cassandra.utils.Pair;
  */
 class TableMetadataRefCache
 {
-    public final static TableMetadataRefCache EMPTY = new TableMetadataRefCache(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
+    public final static TableMetadataRefCache EMPTY = new TableMetadataRefCache(emptyMap(), emptyMap(), emptyMap());
 
     // UUID -> mutable metadata ref map. We have to update these in place every time a table changes.
     private final Map<TableId, TableMetadataRef> metadataRefs;
@@ -46,13 +48,13 @@ class TableMetadataRefCache
     // (keyspace name, index name) -> mutable metadata ref map. We have to update these in place every time an index changes.
     private final Map<Pair<String, String>, TableMetadataRef> indexMetadataRefs;
 
-    public TableMetadataRefCache(Map<TableId, TableMetadataRef> metadataRefs,
+    private TableMetadataRefCache(Map<TableId, TableMetadataRef> metadataRefs,
                                  Map<Pair<String, String>, TableMetadataRef> metadataRefsByName,
                                  Map<Pair<String, String>, TableMetadataRef> indexMetadataRefs)
     {
-        this.metadataRefs = Collections.unmodifiableMap(metadataRefs);
-        this.metadataRefsByName = Collections.unmodifiableMap(metadataRefsByName);
-        this.indexMetadataRefs = Collections.unmodifiableMap(indexMetadataRefs);
+        this.metadataRefs = metadataRefs;
+        this.metadataRefsByName = metadataRefsByName;
+        this.indexMetadataRefs = indexMetadataRefs;
     }
 
     /**
@@ -107,7 +109,11 @@ class TableMetadataRefCache
                    .map(MapDifference.ValueDifference::rightValue)
                    .forEach(indexTable -> indexMetadataRefs.get(Pair.create(indexTable.keyspace, indexTable.indexName().get())).set(indexTable));
 
-        return new TableMetadataRefCache(metadataRefs, metadataRefsByName, indexMetadataRefs);
+        // Avoid re-wrapping the map if no changes
+        return new TableMetadataRefCache(
+            hasCreatedOrDroppedTablesOrViews ? unmodifiableMap(metadataRefs) : metadataRefs,
+            hasCreatedOrDroppedTablesOrViews ? unmodifiableMap(metadataRefsByName) : metadataRefsByName,
+            hasCreatedOrDroppedIndexes ? unmodifiableMap(indexMetadataRefs) : indexMetadataRefs);
     }
 
     private void putRef(Map<TableId, TableMetadataRef> metadataRefs,