From 18b992569f71e7c3450ab89346823f044178bc61 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Mon, 27 Jul 2026 11:04:17 -0700 Subject: [PATCH] Add tombstone-interleaving scenarios; fix live-marker stats poisoning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new differential scenarios cover row-liveness and TTL shapes: UPDATE-built rows without primary-key liveness and primary-key-only inserts with liveness but no cells (rowLivenessShapes), row-TTL vs cell-TTL mixtures including same-timestamp expiring ties with different TTLs (rowAndCellTtlMix), and same-timestamp ties between an expiring cell and a live one in both directions, the CASSANDRA-14592 rule (expiringVsLiveTies). Also fixes a stats-poisoning bug: DeletionTime.ReusableDeletionTime.reset(long, long) classified NO_DELETION_TIME (Long.MAX_VALUE, the canonical "no deletion" long) as invalid instead of mapping it to the LIVE marker the way Cell.deletionTimeLongToUnsignedInteger does, so resetting from LIVE's long values produced a NON-live deletion. A live marker slipping past MetadataCollector's isLive guard this way would record minTimestamp=Long.MIN_VALUE, a bogus tombstone count, and an encoding-stats minimum collapsed to the epoch — stats-only severity, but those stats feed compaction heuristics. DeletionTimeTest.resetWithLiveLongsStaysLive pins the round-trip directly. --- .../org/apache/cassandra/db/DeletionTime.java | 8 +- .../apache/cassandra/db/DeletionTimeTest.java | 70 ++++++++++++++ .../EdgeCaseDifferentialCompactionTest.java | 96 +++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 test/unit/org/apache/cassandra/db/DeletionTimeTest.java diff --git a/src/java/org/apache/cassandra/db/DeletionTime.java b/src/java/org/apache/cassandra/db/DeletionTime.java index 1d54a00fff..cf494c31d5 100644 --- a/src/java/org/apache/cassandra/db/DeletionTime.java +++ b/src/java/org/apache/cassandra/db/DeletionTime.java @@ -442,7 +442,13 @@ public abstract class DeletionTime implements Comparable, IMeasura public void reset(long markedForDeleteAt, long localDeletionTime) { this.markedForDeleteAt = markedForDeleteAt; - if (localDeletionTime < 0 || localDeletionTime > Cell.MAX_DELETION_TIME) // invalid + if (localDeletionTime == Cell.NO_DELETION_TIME) + // NO_DELETION_TIME (Long.MAX_VALUE) is the canonical "no deletion" long and must + // round-trip to the LIVE marker (deletionTimeLongToUnsignedInteger semantics); + // classifying it as invalid made reset(LIVE.markedForDeleteAt(), + // LIVE.localDeletionTime()) produce a NON-live deletion + this.localDeletionTimeUnsignedInteger = LOCAL_DELETION_TIME_LIVE; + else if (localDeletionTime < 0 || localDeletionTime > Cell.MAX_DELETION_TIME) // invalid this.localDeletionTimeUnsignedInteger = Cell.MAX_DELETION_TIME_UNSIGNED_INTEGER + 1; else this.localDeletionTimeUnsignedInteger = Cell.deletionTimeLongToUnsignedInteger(localDeletionTime); diff --git a/test/unit/org/apache/cassandra/db/DeletionTimeTest.java b/test/unit/org/apache/cassandra/db/DeletionTimeTest.java new file mode 100644 index 0000000000..42163fd09d --- /dev/null +++ b/test/unit/org/apache/cassandra/db/DeletionTimeTest.java @@ -0,0 +1,70 @@ +/* + * 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; + +import org.junit.Test; + +import org.apache.cassandra.db.rows.Cell; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class DeletionTimeTest +{ + /** + * NO_DELETION_TIME (Long.MAX_VALUE) is the canonical "no deletion" long value and must + * round-trip through the long-based reset to a LIVE deletion time, mirroring + * {@link Cell#deletionTimeLongToUnsignedInteger}. It used to be classified as invalid, + * so {@code reset(LIVE.markedForDeleteAt(), LIVE.localDeletionTime())} produced a + * NON-live deletion — which let a live marker slip past MetadataCollector's isLive + * guard during cursor compaction and poison minTimestamp/tombstone stats. + */ + @Test + public void resetWithLiveLongsStaysLive() + { + DeletionTime.ReusableDeletionTime reusable = DeletionTime.ReusableDeletionTime.live(); + assertTrue(reusable.isLive()); + + reusable.reset(DeletionTime.LIVE.markedForDeleteAt(), DeletionTime.LIVE.localDeletionTime()); + assertTrue("reset with LIVE's long values must stay live, got mfda=" + reusable.markedForDeleteAt() + + " ldt=" + reusable.localDeletionTime(), reusable.isLive()); + assertEquals(DeletionTime.LIVE.localDeletionTime(), reusable.localDeletionTime()); + } + + @Test + public void resetWithRealAndInvalidValues() + { + DeletionTime.ReusableDeletionTime reusable = DeletionTime.ReusableDeletionTime.live(); + + reusable.reset(123456789L, 1_700_000_000L); + assertFalse(reusable.isLive()); + assertEquals(123456789L, reusable.markedForDeleteAt()); + assertEquals(1_700_000_000L, reusable.localDeletionTime()); + + // negative and beyond-max (but not NO_DELETION_TIME) stay classified as invalid + reusable.reset(1L, -5L); + assertFalse(reusable.isLive()); + assertFalse(reusable.validate()); + + reusable.reset(1L, Cell.MAX_DELETION_TIME + 1); + assertFalse(reusable.isLive()); + assertFalse(reusable.validate()); + } +} diff --git a/test/unit/org/apache/cassandra/db/compaction/differential/EdgeCaseDifferentialCompactionTest.java b/test/unit/org/apache/cassandra/db/compaction/differential/EdgeCaseDifferentialCompactionTest.java index 6500a0afa5..455b41bcca 100644 --- a/test/unit/org/apache/cassandra/db/compaction/differential/EdgeCaseDifferentialCompactionTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/differential/EdgeCaseDifferentialCompactionTest.java @@ -443,4 +443,100 @@ public class EdgeCaseDifferentialCompactionTest extends DifferentialCompactionTe assertCursorMatchesIteratorAcrossGenerations(cfs, ALLOWLIST); } + + /** + * Row liveness shapes: UPDATE-built rows carry NO primary-key liveness (different row + * flags than INSERT-built rows), primary-key-only INSERTs carry liveness and ZERO cells, + * and merges must reconcile liveness presence/absence across sstables exactly. + */ + @Test + public void rowLivenessShapes() throws Exception + { + createTable("CREATE TABLE %s (pk bigint, ck bigint, v1 text, v2 text, PRIMARY KEY (pk, ck))"); + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // UPDATE-built rows (no liveness) and liveness-only rows in sstable 1 + for (long ck = 0; ck < 10; ck++) + execute("UPDATE %s SET v1 = ?, v2 = ? WHERE pk = ? AND ck = ?", "u" + ck, "w" + ck, 1L, ck); + for (long ck = 10; ck < 15; ck++) + execute("INSERT INTO %s (pk, ck) VALUES (?, ?)", 1L, ck); + flush(); + + // sstable 2: INSERT onto UPDATE-rows (liveness arrives later), cell tombstones onto + // liveness-only rows (row must survive on liveness alone), cell delete that empties + // an UPDATE-row entirely (no liveness + no cells = row vanishes) + for (long ck = 0; ck < 4; ck++) + execute("INSERT INTO %s (pk, ck, v1) VALUES (?, ?, ?)", 1L, ck, "i" + ck); + for (long ck = 10; ck < 13; ck++) + execute("INSERT INTO %s (pk, ck, v1, v2) VALUES (?, ?, null, null)", 1L, ck); + execute("DELETE v1, v2 FROM %s WHERE pk = ? AND ck = ?", 1L, 5L); + flush(); + + assertCursorMatchesIteratorAcrossGenerations(cfs, ALLOWLIST); + } + + /** + * Row-level TTL (INSERT USING TTL sets liveness TTL + cell TTLs) merged against + * cell-level TTL (UPDATE USING TTL sets only cell TTLs) and against plain writes; + * includes same-timestamp expiring writes whose TTLs differ (rules (c)/(d) of the + * resolveRegular decision table run off localExpirationTime/ttl, not just timestamps). + */ + @Test + public void rowAndCellTtlMix() throws Exception + { + createTable("CREATE TABLE %s (pk bigint, ck bigint, v1 text, v2 text, PRIMARY KEY (pk, ck))"); + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + for (long ck = 0; ck < 12; ck++) + execute("INSERT INTO %s (pk, ck, v1, v2) VALUES (?, ?, ?, ?) USING TTL 86400", 1L, ck, "a" + ck, "b" + ck); + flush(); + + // cell-level TTL different from the row TTL; plain overwrites clearing TTLs; + // expiring-vs-expiring same-timestamp ties with different TTLs + for (long ck = 0; ck < 6; ck++) + execute("UPDATE %s USING TTL 172800 SET v1 = ? WHERE pk = ? AND ck = ?", "c" + ck, 1L, ck); + for (long ck = 6; ck < 9; ck++) + execute("INSERT INTO %s (pk, ck, v1) VALUES (?, ?, ?)", 1L, ck, "plain" + ck); + for (long ck = 9; ck < 12; ck++) + execute("UPDATE %s USING TTL 100000 AND TIMESTAMP 5000 SET v2 = ? WHERE pk = ? AND ck = ?", "t1" + ck, 1L, ck); + flush(); + + for (long ck = 9; ck < 12; ck++) + execute("UPDATE %s USING TTL 50000 AND TIMESTAMP 5000 SET v2 = ? WHERE pk = ? AND ck = ?", "t2" + ck, 1L, ck); + flush(); + + assertCursorMatchesIteratorAcrossGenerations(cfs, ALLOWLIST); + } + + /** + * Expiring-vs-live cells at the SAME timestamp, both directions across sstables: the + * CASSANDRA-14592 rule — an expiring (or deleted) cell beats a live one on timestamp + * tie regardless of value. Implemented in resolveRegular rule (a); this pins it at the + * differential level (timestampTies only covered live-vs-live and delete-vs-live). + */ + @Test + public void expiringVsLiveTies() throws Exception + { + createTable("CREATE TABLE %s (pk bigint, ck bigint, v text, PRIMARY KEY (pk, ck))"); + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // direction 1: live first, expiring second + for (long ck = 0; ck < 5; ck++) + execute("INSERT INTO %s (pk, ck, v) VALUES (?, ?, ?) USING TIMESTAMP 1000", 1L, ck, "zzz-live" + ck); + // direction 2 partition: expiring first + for (long ck = 0; ck < 5; ck++) + execute("INSERT INTO %s (pk, ck, v) VALUES (?, ?, ?) USING TIMESTAMP 1000 AND TTL 86400", 2L, ck, "aaa-ttl" + ck); + flush(); + + for (long ck = 0; ck < 5; ck++) + execute("INSERT INTO %s (pk, ck, v) VALUES (?, ?, ?) USING TIMESTAMP 1000 AND TTL 86400", 1L, ck, "aaa-ttl" + ck); + for (long ck = 0; ck < 5; ck++) + execute("INSERT INTO %s (pk, ck, v) VALUES (?, ?, ?) USING TIMESTAMP 1000", 2L, ck, "zzz-live" + ck); + flush(); + + assertCursorMatchesIteratorAcrossGenerations(cfs, ALLOWLIST); + } }