diff --git a/CHANGES.txt b/CHANGES.txt
index c402eb2ea7..e45bbb09c7 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -2,6 +2,7 @@
* Add Paxos v2 option and informatin in cassandra.yaml (CASSANDRA-21316)
* Harden data resurrection startup check with atomic heartbeat file write with fallback (CASSANDRA-21290)
Merged from 4.0:
+ * Fix negative memtable allocator ownership when an update is shadowed by an existing row deletion (CASSANDRA-21469)
* Consider first token of SSTable when calculating SSTable intersection in LeveledScanner (CASSANDRA-21369)
* Remove inFlightEcho entry on ECHO_REQ failure (CASSANDRA-21428)
* Validate snapshot names (CASSANDRA-21389)
diff --git a/src/java/org/apache/cassandra/db/rows/BTreeRow.java b/src/java/org/apache/cassandra/db/rows/BTreeRow.java
index 125932be0f..c92a698bd3 100644
--- a/src/java/org/apache/cassandra/db/rows/BTreeRow.java
+++ b/src/java/org/apache/cassandra/db/rows/BTreeRow.java
@@ -603,10 +603,16 @@ public class BTreeRow extends AbstractRow
{
if (rowDeletion == existing.deletion())
{
- updateBtree = BTree.transformAndFilter(updateBtree, reconciler::retain);
+ // The existing row's deletion shadows part of the update. Filter those cells out of
+ // the UPDATE (incoming) side, but do NOT record their removal: that data was never
+ // owned by the memtable, so accounting its removal would drive the allocator's
+ // ownership negative and crash the next flush (CASSANDRA-21469).
+ updateBtree = BTree.transformAndFilter(updateBtree, reconciler::removeShadowed);
}
else
{
+ // The update's deletion shadows part of the existing row. Those cells ARE owned by
+ // the memtable, so record their removal via retain().
existingBtree = BTree.transformAndFilter(existingBtree, reconciler::retain);
}
}
diff --git a/src/java/org/apache/cassandra/db/rows/ColumnData.java b/src/java/org/apache/cassandra/db/rows/ColumnData.java
index e0bc552322..adf6db8713 100644
--- a/src/java/org/apache/cassandra/db/rows/ColumnData.java
+++ b/src/java/org/apache/cassandra/db/rows/ColumnData.java
@@ -179,7 +179,15 @@ public abstract class ColumnData implements IMeasurableMemory
return removeShadowed(existing, postReconcile);
}
- private ColumnData removeShadowed(ColumnData existing)
+ /**
+ * Like {@link #retain} but does NOT notify the {@link PostReconciliationFunction} of removed
+ * data. Use this e.g. when filtering shadowed cells out of the UPDATE (incoming) side of a merge:
+ * that data was never allocated to / owned by the memtable, so recording its removal would
+ * make the memtable allocator under-count what it owns and eventually report a negative
+ * release at flush (CASSANDRA-21469). Recording removals (via {@link #retain}) is only correct
+ * for the EXISTING side, whose data the memtable already owns.
+ */
+ public ColumnData removeShadowed(ColumnData existing)
{
return removeShadowed(existing, ColumnData.noOp);
}
diff --git a/test/unit/org/apache/cassandra/db/partitions/MemtableNegativeReleasedCQLReproTest.java b/test/unit/org/apache/cassandra/db/partitions/MemtableNegativeReleasedCQLReproTest.java
new file mode 100644
index 0000000000..7e36ffbae7
--- /dev/null
+++ b/test/unit/org/apache/cassandra/db/partitions/MemtableNegativeReleasedCQLReproTest.java
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.db.partitions;
+
+import java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.memtable.Memtable;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * CQL-driven regression test for CASSANDRA-21469 ("MemtableReclaimMemory AssertionError:
+ * Negative released in MemtablePool$SubPool").
+ *
+ * It drives overwrite/delete churn through the public CQL write path (with explicit
+ * {@code USING TIMESTAMP} so reconciliation is deterministic), inspects the live memtable's
+ * on-heap ownership after every statement, and forces a flush to exercise the real discard/release path.
+ *
+ * Root cause: when {@link org.apache.cassandra.db.rows.BTreeRow}{@code .merge} reconciles an update into a
+ * row whose existing deletion shadows the update's cells, it filtered the update (incoming) side of the
+ * merge with {@code Reconciler.retain}, which records the removal via {@code PostReconciliationFunction.delete}.
+ * On the memtable write path that subtracts the shadowed cells' on-heap size from the allocator's ownership
+ * even though that incoming data was never allocated to the memtable. Under overwrite/delete churn that
+ * re-applies cells already covered by a newer row/partition deletion (e.g. repair re-streaming), the
+ * allocator's {@code owns} counter drifts below zero and the next flush trips
+ * {@code AssertionError: Negative released} in {@code MemtablePool$SubPool.released} via
+ * {@code MemtableAllocator$SubAllocator.releaseAll} during discard.
+ *
+ * The test asserts the same invariant the production code asserts at flush
+ * ({@code memtable on-heap owns >= 0}) and then flushes, which runs the real discard path and reproduces
+ * the exact production stack when {@code owns} has gone negative.
+ *
+ * Note on minimisation: a single shadowed update nets close to zero because the CQL path first clones the
+ * incoming row into the memtable allocator and only then merges. The drift only becomes observable across
+ * mixed churn with non-monotonic timestamps — exactly the repair re-streaming workload that hit this in the
+ * field — hence the fixed-seed churn below rather than a one-liner.
+ */
+public class MemtableNegativeReleasedCQLReproTest extends CQLTester
+{
+ /**
+ * Deterministic fixed-seed churn of overwrites / row tombstones / partition deletions against a
+ * single partition (what repair re-streaming does). On buggy code this drives the memtable's
+ * on-heap ownership negative; the per-statement assertion catches the drift and the trailing flush
+ * reproduces the exact production discard crash.
+ */
+ @Test
+ public void repeatedOverwriteAndDeleteChurnKeepsOwnsNonNegative() throws Throwable
+ {
+ for (int seed = 0; seed < 50; seed++)
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, r1 int, c2 set, PRIMARY KEY (pk, ck))");
+ ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
+ cfs.disableAutoCompaction();
+ Random rnd = new Random(seed);
+ for (int i = 0; i < 40; i++)
+ {
+ mutateSamePartition(rnd);
+ assertThat(currentOnHeapOwns(cfs))
+ .as("memtable on-heap owns went NEGATIVE during overwrite/delete churn (seed=%d, mutation=%d) " +
+ "— the memtable reported releasing more on-heap memory than it allocated (CASSANDRA-21469)",
+ seed, i)
+ .isGreaterThanOrEqualTo(0L);
+ }
+ // The real crash path: flush discards the memtable, running releaseAll() -> SubPool.released(owns),
+ // which throws AssertionError("Negative released: ...") under -ea if owns ever went negative.
+ flush();
+ }
+ }
+
+ private long currentOnHeapOwns(ColumnFamilyStore cfs)
+ {
+ Memtable memtable = cfs.getTracker().getView().getCurrentMemtable();
+ return Memtable.getMemoryUsage(memtable).ownsOnHeap;
+ }
+
+ /**
+ * A varying mutation of the same partition: live overwrite, row tombstone, or partition deletion.
+ */
+ private void mutateSamePartition(Random rnd) throws Throwable
+ {
+ // RANDOM (non-monotonic) timestamp is essential: it makes reconciliation sometimes keep the
+ // existing cell and sometimes shadow/replace it, producing the merges that over-subtract.
+ long ts = 1000 + rnd.nextInt(3000);
+ switch (rnd.nextInt(6))
+ {
+ case 0: // whole-partition deletion
+ execute("DELETE FROM %s USING TIMESTAMP " + ts + " WHERE pk = 0");
+ break;
+ case 1: // whole-row tombstone
+ execute("DELETE FROM %s USING TIMESTAMP " + ts + " WHERE pk = 0 AND ck = 0");
+ break;
+ default: // live overwrite of varying shape
+ {
+ StringBuilder set = new StringBuilder("r1 = ").append(rnd.nextInt());
+ int cells = rnd.nextInt(5);
+ StringBuilder elements = new StringBuilder();
+ for (int k = 0; k < cells; k++)
+ {
+ if (elements.length() > 0)
+ elements.append(", ");
+ elements.append(1000 + rnd.nextInt(8));
+ }
+ if (cells > 0)
+ {
+ if (rnd.nextInt(3) == 0)
+ set.append(", c2 = {").append(elements).append('}'); // full overwrite -> complex deletion
+ else
+ set.append(", c2 = c2 + {").append(elements).append('}'); // append cells, no complex deletion
+ }
+ execute("UPDATE %s USING TIMESTAMP " + ts + " SET " + set + " WHERE pk = 0 AND ck = 0");
+ break;
+ }
+ }
+ }
+}