From a4487f5772cce8706a8952f284c7bbce6871d4dc Mon Sep 17 00:00:00 2001 From: Zhao Yang Date: Fri, 10 Oct 2025 16:00:35 +0800 Subject: [PATCH 1/4] CNDB-15594: make SimpleShardTracker public --- .../apache/cassandra/db/compaction/SimpleShardTracker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java b/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java index bdbf1704d5d9..1a9ca2887e4e 100644 --- a/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java +++ b/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java @@ -31,13 +31,13 @@ * A shard tracker that uses the provided tokens as a complete list of split points. The first token is typically * the minimum token. */ -class SimpleShardTracker implements ShardTracker +public class SimpleShardTracker implements ShardTracker { private final Token[] sortedTokens; private int index; private Token currentEnd; - SimpleShardTracker(Token[] sortedTokens) + public SimpleShardTracker(Token[] sortedTokens) { assert sortedTokens.length > 0; assert sortedTokens[0].isMinimum(); From 5cc4811a5228f74ce6e6288653b4f3a38a60cad9 Mon Sep 17 00:00:00 2001 From: Branimir Lambov Date: Mon, 13 Oct 2025 14:28:56 +0800 Subject: [PATCH 2/4] Add AbstractBounds#intersects and fix Range#intersects(Bounds that) to handle full range --- .../apache/cassandra/dht/AbstractBounds.java | 16 +++ src/java/org/apache/cassandra/dht/Range.java | 7 +- .../dht/AbstractBoundsQuickTest.java | 120 ++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java diff --git a/src/java/org/apache/cassandra/dht/AbstractBounds.java b/src/java/org/apache/cassandra/dht/AbstractBounds.java index 71181e3b2bd1..d0d7a191ec09 100644 --- a/src/java/org/apache/cassandra/dht/AbstractBounds.java +++ b/src/java/org/apache/cassandra/dht/AbstractBounds.java @@ -147,6 +147,22 @@ private String format(T value, AbstractType keyValidator) public abstract boolean isStartInclusive(); public abstract boolean isEndInclusive(); + public boolean intersects(AbstractBounds other) + { + // If one is a Range, it may be wraparound, thus we must defer to its implementation of intersects. + if (other instanceof Range) + return other.intersects(this); + + int cmp = other.right.isMinimum() ? -1 : left.compareTo(other.right); + if (cmp > 0 || (cmp == 0 && (!inclusiveLeft() || !other.inclusiveRight()))) + return false; + cmp = right.isMinimum() ? 1 : right.compareTo(other.left); + if (cmp < 0 || (cmp == 0 && (!inclusiveRight() || !other.inclusiveLeft()))) + return false; + + return true; + } + public abstract AbstractBounds withNewRight(T newRight); public static class AbstractBoundsSerializer> implements IPartitionerDependentSerializer> diff --git a/src/java/org/apache/cassandra/dht/Range.java b/src/java/org/apache/cassandra/dht/Range.java index ed3c8bd20ca4..2cf06357991b 100644 --- a/src/java/org/apache/cassandra/dht/Range.java +++ b/src/java/org/apache/cassandra/dht/Range.java @@ -164,7 +164,12 @@ public boolean intersects(Bounds that) // Same punishment than in Bounds.contains(), we must be carefull if that.left == that.right as // as new Range(that.left, that.right) will then cover the full ring which is not what we // want. - return contains(that.left) || (!that.left.equals(that.right) && intersects(new Range(that.left, that.right))); + if (contains(that.left)) + return true; + else if (that.left.equals(that.right)) // full range + return that.right.isMinimum(); + else + return intersects(new Range(that.left, that.right)); } public static boolean intersects(Iterable> l, Iterable> r) diff --git a/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java b/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java new file mode 100644 index 000000000000..b42ead5ddcd2 --- /dev/null +++ b/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java @@ -0,0 +1,120 @@ +/* + * 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.dht; + +import org.junit.Test; + +import org.quicktheories.core.Gen; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.quicktheories.QuickTheory.qt; +import static org.quicktheories.generators.SourceDSL.integers; +import static org.quicktheories.generators.SourceDSL.longs; + +public class AbstractBoundsQuickTest +{ + private static final long MAX_TOKEN = Murmur3Partitioner.MAXIMUM; + + @Test + public void testIntersects() + { + qt().forAll(bounds(), bounds()) + .check((r1, r2) -> { + boolean intersects = r1.intersects(r2); + // Check commutativity + assertThat(r2.intersects(r1)).isEqualTo(intersects); + + assertThat(intersects).isEqualTo(stupidIntersects(r1, r2)); + + return true; + }); + } + + boolean stupidIntersects(AbstractBounds l, AbstractBounds r) + { + if (isPoint(l)) + return l.left.isMinimum() || r.contains(l.left); + if (isPoint(r)) + return r.left.isMinimum() || l.contains(r.left); + + // Range.intersects is already tested + return toRange(l).intersects(r); + } + + private static Range toRange(AbstractBounds bounds) + { + if (bounds instanceof Range) + return (Range) bounds; + + Token l = bounds.left; + if (bounds.inclusiveLeft()) + l = l.prevValidToken(); + Token r = bounds.right; + if (!bounds.inclusiveRight()) + r = r.prevValidToken(); + + return new Range<>(l, r); + } + + private static boolean isPoint(AbstractBounds l) + { + return l.inclusiveLeft() && l.inclusiveRight() && l.left.equals(l.right); + } + + private Gen> bounds() + { + return longs().between(0, MAX_TOKEN) + .zip(integers().between(0, 6), this::createBoundary) // 14% chance min + .zip(longs().between(0, MAX_TOKEN) + .zip(integers().between(-1, 6), this::createBoundary), // 12.5% chance point, 12.5% chance min + this::createAbstractBounds); + } + + private AbstractBounds.Boundary createBoundary(long pos, int minOrInclusive) + { + if (minOrInclusive < 0) + return null; // point bounds + Token t; + if (minOrInclusive == 0) + t = Murmur3Partitioner.instance.getMinimumToken(); + else + t = new Murmur3Partitioner.LongToken(pos); + + return new AbstractBounds.Boundary(t, minOrInclusive % 2 == 0); + } + + private AbstractBounds createAbstractBounds(AbstractBounds.Boundary left, AbstractBounds.Boundary right) + { + if (right == null) + { + return AbstractBounds.bounds(left.boundary, true, left.boundary, true); + } + + if (!left.inclusive && right.inclusive) + return new Range<>(left.boundary, right.boundary); // ranges can be wraparound + + // For other cases, create a normal bounds + int cmp = left.boundary.compareTo(right.boundary); + if (cmp < 0) + return AbstractBounds.bounds(left, right); + else if (cmp > 0) + return AbstractBounds.bounds(right, left); + else + return AbstractBounds.bounds(left.boundary, true, left.boundary, true); + } +} From 10bd2df6a721e9c27e863bc2d9c8dc7f0ee0e40b Mon Sep 17 00:00:00 2001 From: Zhao Yang Date: Mon, 13 Oct 2025 14:34:22 +0800 Subject: [PATCH 3/4] fix compilation --- src/java/org/apache/cassandra/dht/Range.java | 1 + .../org/apache/cassandra/dht/AbstractBoundsQuickTest.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/dht/Range.java b/src/java/org/apache/cassandra/dht/Range.java index 2cf06357991b..e3ad95a2ca32 100644 --- a/src/java/org/apache/cassandra/dht/Range.java +++ b/src/java/org/apache/cassandra/dht/Range.java @@ -118,6 +118,7 @@ public boolean intersects(Range that) return intersectionWith(that).size() > 0; } + @Override public boolean intersects(AbstractBounds that) { // implemented for cleanup compaction membership test, so only Range + Bounds are supported for now diff --git a/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java b/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java index b42ead5ddcd2..6ffc3ecd5e60 100644 --- a/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java +++ b/test/unit/org/apache/cassandra/dht/AbstractBoundsQuickTest.java @@ -63,10 +63,10 @@ private static Range toRange(AbstractBounds bounds) Token l = bounds.left; if (bounds.inclusiveLeft()) - l = l.prevValidToken(); + l = new Murmur3Partitioner.LongToken(l.getLongValue() - 1); Token r = bounds.right; if (!bounds.inclusiveRight()) - r = r.prevValidToken(); + r = new Murmur3Partitioner.LongToken(r.getLongValue() - 1); return new Range<>(l, r); } From 8665f3b18e2862a80eba57db54384aab33b2b46f Mon Sep 17 00:00:00 2001 From: Zhao Yang Date: Mon, 13 Oct 2025 16:19:04 +0800 Subject: [PATCH 4/4] do not make SimpleShardTracker public and update comments in Range#intersects --- .../org/apache/cassandra/db/compaction/ShardTracker.java | 6 +++++- .../apache/cassandra/db/compaction/SimpleShardTracker.java | 4 ++-- src/java/org/apache/cassandra/dht/Range.java | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/java/org/apache/cassandra/db/compaction/ShardTracker.java b/src/java/org/apache/cassandra/db/compaction/ShardTracker.java index 1b11e90585cf..f7c92eec368e 100644 --- a/src/java/org/apache/cassandra/db/compaction/ShardTracker.java +++ b/src/java/org/apache/cassandra/db/compaction/ShardTracker.java @@ -24,7 +24,6 @@ import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.dht.Range; import org.apache.cassandra.dht.Token; -import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.io.sstable.format.SSTableWriter; public interface ShardTracker @@ -71,4 +70,9 @@ default void applyTokenSpaceCoverage(SSTableWriter writer) if (writer.first != null) writer.setTokenSpaceCoverage(rangeSpanned(writer.first, writer.last)); } + + static ShardTracker createPreassignedBoundaries(Token[] sortedTokens) + { + return new SimpleShardTracker(sortedTokens); + } } diff --git a/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java b/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java index 1a9ca2887e4e..bdbf1704d5d9 100644 --- a/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java +++ b/src/java/org/apache/cassandra/db/compaction/SimpleShardTracker.java @@ -31,13 +31,13 @@ * A shard tracker that uses the provided tokens as a complete list of split points. The first token is typically * the minimum token. */ -public class SimpleShardTracker implements ShardTracker +class SimpleShardTracker implements ShardTracker { private final Token[] sortedTokens; private int index; private Token currentEnd; - public SimpleShardTracker(Token[] sortedTokens) + SimpleShardTracker(Token[] sortedTokens) { assert sortedTokens.length > 0; assert sortedTokens[0].isMinimum(); diff --git a/src/java/org/apache/cassandra/dht/Range.java b/src/java/org/apache/cassandra/dht/Range.java index e3ad95a2ca32..2560f30c8d70 100644 --- a/src/java/org/apache/cassandra/dht/Range.java +++ b/src/java/org/apache/cassandra/dht/Range.java @@ -167,8 +167,8 @@ public boolean intersects(Bounds that) // want. if (contains(that.left)) return true; - else if (that.left.equals(that.right)) // full range - return that.right.isMinimum(); + else if (that.left.equals(that.right)) + return that.right.isMinimum(); // [x, x] denotes a single point only, but [min, min] covers the full token range else return intersects(new Range(that.left, that.right)); }