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
This commit is contained in:
Abe Ratnofsky 2023-12-04 23:15:05 -05:00 committed by Jacek Lewandowski
parent 13e5956285
commit a443990bfa
2 changed files with 14 additions and 7 deletions

View File

@ -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)

View File

@ -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,