This is an automated email from the ASF dual-hosted git repository.
maedhroz pushed a commit to branch cassandra-5.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-5.0 by this push:
new 1d47fab638 Serialization can lose complex deletions in a mutation with
multiple collections in a row
1d47fab638 is described below
commit 1d47fab638e16e103cbeb19fe979806c16b26b45
Author: Caleb Rackliffe <calebrackli...@gmail.com>
AuthorDate: Mon Mar 17 22:49:23 2025 -0500
Serialization can lose complex deletions in a mutation with multiple
collections in a row
patch by Caleb Rackliffe; reviewed by Berenguer Blasi and Abe Ratnofsky for
CASSANDRA-20449
---
CHANGES.txt | 1 +
.../org/apache/cassandra/db/rows/BTreeRow.java | 6 ++-
.../org/apache/cassandra/utils/ByteBufferUtil.java | 15 ++++++
.../org/apache/cassandra/utils/btree/BTree.java | 5 +-
.../distributed/test/CollectionsTest.java | 55 ++++++++++++++++++++++
5 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index a7b52483c8..bb53fe0107 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
5.0.4
+ * Serialization can lose complex deletions in a mutation with multiple
collections in a row (CASSANDRA-20449)
* Improve error messages when initializing auth classes (CASSANDRA-20368)
* Prioritize legacy 2i over SAI for columns with multiple indexes
(CASSANDRA-20334)
* Ensure only offline tools can build IntervalTrees without first/last key
fields (CASSANDRA-20407)
diff --git a/src/java/org/apache/cassandra/db/rows/BTreeRow.java
b/src/java/org/apache/cassandra/db/rows/BTreeRow.java
index 075a4f67fe..52f0639e8e 100644
--- a/src/java/org/apache/cassandra/db/rows/BTreeRow.java
+++ b/src/java/org/apache/cassandra/db/rows/BTreeRow.java
@@ -60,6 +60,8 @@ import org.apache.cassandra.utils.btree.BTreeSearchIterator;
import org.apache.cassandra.utils.btree.UpdateFunction;
import org.apache.cassandra.utils.memory.Cloner;
+import static org.apache.cassandra.utils.btree.BTree.STOP_SENTINEL_VALUE;
+
/**
* Immutable implementation of a Row object.
*/
@@ -399,9 +401,9 @@ public class BTreeRow extends AbstractRow
public boolean hasComplexDeletion()
{
- long result = accumulate((cd, v) -> ((ComplexColumnData)
cd).complexDeletion().isLive() ? 0 : Cell.MAX_DELETION_TIME,
+ long result = accumulate((cd, v) -> ((ComplexColumnData)
cd).complexDeletion().isLive() ? 0 : STOP_SENTINEL_VALUE,
COLUMN_COMPARATOR, isStatic() ?
FIRST_COMPLEX_STATIC : FIRST_COMPLEX_REGULAR, 0L);
- return result == Cell.MAX_DELETION_TIME;
+ return result == STOP_SENTINEL_VALUE;
}
public Row markCounterLocalToBeCleared()
diff --git a/src/java/org/apache/cassandra/utils/ByteBufferUtil.java
b/src/java/org/apache/cassandra/utils/ByteBufferUtil.java
index 156c1c4bc8..4d3d0ca0f3 100644
--- a/src/java/org/apache/cassandra/utils/ByteBufferUtil.java
+++ b/src/java/org/apache/cassandra/utils/ByteBufferUtil.java
@@ -34,10 +34,14 @@ import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
+import java.util.LinkedHashSet;
+import java.util.Set;
import java.util.UUID;
import net.nicoulaj.compilecommand.annotations.Inline;
import org.apache.cassandra.db.TypeSizes;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.db.marshal.SetType;
import org.apache.cassandra.io.compress.BufferType;
import org.apache.cassandra.io.util.DataInputPlus;
import org.apache.cassandra.io.util.DataOutputPlus;
@@ -544,6 +548,17 @@ public class ByteBufferUtil
return ByteBuffer.wrap((byte[]) obj);
else if (obj instanceof ByteBuffer)
return (ByteBuffer) obj;
+ else if (obj instanceof Set)
+ {
+ Set<?> set = (Set<?>) obj;
+ // convert subtypes to BB
+ Set<ByteBuffer> bbs = new LinkedHashSet<>();
+ for (Object o : set)
+ if (!bbs.add(objectToBytes(o)))
+ throw new IllegalStateException("Object " + o + " maps to
a buffer that already exists in the set");
+ // decompose/serializer doesn't use the isMultiCell, so safe to do
this
+ return SetType.getInstance(BytesType.instance,
false).decompose(bbs);
+ }
else
throw new IllegalArgumentException(String.format("Cannot convert
value %s of type %s",
obj,
diff --git a/src/java/org/apache/cassandra/utils/btree/BTree.java
b/src/java/org/apache/cassandra/utils/btree/BTree.java
index 2ac80df48e..8674d714da 100644
--- a/src/java/org/apache/cassandra/utils/btree/BTree.java
+++ b/src/java/org/apache/cassandra/utils/btree/BTree.java
@@ -68,6 +68,7 @@ public class BTree
private static final int BRANCH_FACTOR = 1 << BRANCH_SHIFT;
public static final int MIN_KEYS = BRANCH_FACTOR / 2 - 1;
public static final int MAX_KEYS = BRANCH_FACTOR - 1;
+ public static final long STOP_SENTINEL_VALUE = Long.MAX_VALUE;
// An empty BTree Leaf - which is the same as an empty BTree
private static final Object[] EMPTY_LEAF = new Object[1];
@@ -1823,7 +1824,7 @@ public class BTree
private static boolean isStopSentinel(long v)
{
- return v == Long.MAX_VALUE;
+ return v == STOP_SENTINEL_VALUE;
}
private static <V, A> long accumulateLeaf(Object[] btree,
BiLongAccumulator<A, V> accumulator, A arg, Comparator<V> comparator, V from,
long initialValue)
@@ -1852,7 +1853,7 @@ public class BTree
/**
* Walk the btree and accumulate a long value using the supplied
accumulator function. Iteration will stop if the
- * accumulator function returns the sentinel values Long.MIN_VALUE or
Long.MAX_VALUE
+ * accumulator function returns the sentinel value {@link
#STOP_SENTINEL_VALUE}
* <p>
* If the optional from argument is not null, iteration will start from
that value (or the one after it's insertion
* point if an exact match isn't found)
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/CollectionsTest.java
b/test/distributed/org/apache/cassandra/distributed/test/CollectionsTest.java
new file mode 100644
index 0000000000..8e8ab24240
--- /dev/null
+++
b/test/distributed/org/apache/cassandra/distributed/test/CollectionsTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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.distributed.test;
+
+import java.io.IOException;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.ConsistencyLevel;
+
+import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows;
+import static org.apache.cassandra.distributed.shared.AssertUtils.row;
+
+public class CollectionsTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
+
+ @BeforeClass
+ public static void setUpCluster() throws IOException
+ {
+ CLUSTER = init(Cluster.build(2).start());
+ }
+
+ @Test
+ public void testMultipleSetsComplexDeletion()
+ {
+ CLUSTER.schemaChange(withKeyspace("CREATE TABLE %s.multi_collection (k
int, c int, s1 set<int>, s2 set<int>, s3 set<int>, PRIMARY KEY (k, c)) WITH
read_repair = 'NONE'"));
+ CLUSTER.coordinator(1).execute(withKeyspace("INSERT INTO
%s.multi_collection (k, c, s1, s2, s3) VALUES (?, ?, ?, ?, ?)"),
ConsistencyLevel.ALL, 0, 0, set(1), set(1), set(1));
+ CLUSTER.coordinator(1).execute(withKeyspace("UPDATE
%s.multi_collection SET s2 = ?, s1 = s1 + ?, s3 = s3 + ? WHERE k = ? AND c =
?"), ConsistencyLevel.ALL, set(2), set(2), set(2), 0, 0);
+
+ String select = withKeyspace("SELECT k, c, s1, s2, s3 FROM
%s.multi_collection");
+ assertRows(CLUSTER.get(1).executeInternal(select), row(0, 0, set(1,
2), set(2), set(1, 2)));
+
+ // If the complex deletion is not properly serialized, node 2 will
think the update on s2 was an append...
+ assertRows(CLUSTER.get(2).executeInternal(select), row(0, 0, set(1,
2), set(2), set(1, 2)));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
