From 173f2552d10b5d081888f62864c40380622097e7 Mon Sep 17 00:00:00 2001 From: Ariel Weisberg Date: Fri, 29 May 2015 10:50:15 +0200 Subject: [PATCH] Fix condition in RangeTombstoneList.add that breaks test patch by aweisberg; reviewed by slebresne for CASSANDRA-9485 --- CHANGES.txt | 1 + .../cassandra/db/RangeTombstoneList.java | 4 +-- .../cassandra/db/RangeTombstoneListTest.java | 28 +++++++++++++++---- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 58f3c53d9f..b9a0131af4 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.0.16: + * Fix bad condition in RangeTombstoneList (CASSANDRA-9485) * Fix potential StackOverflow when setting CrcCheckChance over JMX (CASSANDRA-9488) * Fix null static columns in pages after the first, paged reversed queries (CASSANDRA-8502) diff --git a/src/java/org/apache/cassandra/db/RangeTombstoneList.java b/src/java/org/apache/cassandra/db/RangeTombstoneList.java index b49c2fc9aa..165718e8a7 100644 --- a/src/java/org/apache/cassandra/db/RangeTombstoneList.java +++ b/src/java/org/apache/cassandra/db/RangeTombstoneList.java @@ -116,7 +116,7 @@ public class RangeTombstoneList implements Iterable /** * Adds a new range tombstone. * - * This method will be faster if the new tombstone sort after all the currently existing ones (this is a common use case), + * This method will be faster if the new tombstone sort after all the currently existing ones (this is a common use case), * but it doesn't assume it. */ public void add(ByteBuffer start, ByteBuffer end, long markedAt, int delTime) @@ -130,7 +130,7 @@ public class RangeTombstoneList implements Iterable int c = comparator.compare(ends[size-1], start); // Fast path if we add in sorted order - if (c <= 0) + if (c < 0) { addInternal(size, start, end, markedAt, delTime); } diff --git a/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java b/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java index b0065e04d5..2a7c90f43f 100644 --- a/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java +++ b/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java @@ -30,7 +30,6 @@ import org.apache.cassandra.utils.ByteBufferUtil; public class RangeTombstoneListTest { private static final Comparator cmp = IntegerType.instance; - private static final Random rand = new Random(); @Test public void sortedAdditionTest() @@ -296,7 +295,23 @@ public class RangeTombstoneListTest assertEquals(6, l.maxMarkedAt()); } - private RangeTombstoneList makeRandom(int size, int maxItSize, int maxItDistance, int maxMarkedAt) + @Test + public void insertSameTest() + { + // Simple test that adding the same element multiple time ends up + // with that element only a single time (CASSANDRA-9485) + + RangeTombstoneList l = new RangeTombstoneList(cmp, 0); + l.add(rt(4, 4, 5, 100)); + l.add(rt(4, 4, 6, 110)); + l.add(rt(4, 4, 4, 90)); + + Iterator iter = l.iterator(); + assertRT(rt(4, 4, 6, 110), iter.next()); + assert !iter.hasNext(); + } + + private RangeTombstoneList makeRandom(Random rand, int size, int maxItSize, int maxItDistance, int maxMarkedAt) { RangeTombstoneList l = new RangeTombstoneList(cmp, size); @@ -329,10 +344,13 @@ public class RangeTombstoneListTest int MAX_IT_DISTANCE = 10; int MAX_MARKEDAT = 10; + long seed = System.nanoTime(); + Random rand = new Random(seed); + for (int i = 0; i < TEST_COUNT; i++) { - RangeTombstoneList l1 = makeRandom(rand.nextInt(MAX_LIST_SIZE) + 1, rand.nextInt(MAX_IT_SIZE) + 1, rand.nextInt(MAX_IT_DISTANCE) + 1, rand.nextInt(MAX_MARKEDAT) + 1); - RangeTombstoneList l2 = makeRandom(rand.nextInt(MAX_LIST_SIZE) + 1, rand.nextInt(MAX_IT_SIZE) + 1, rand.nextInt(MAX_IT_DISTANCE) + 1, rand.nextInt(MAX_MARKEDAT) + 1); + RangeTombstoneList l1 = makeRandom(rand, rand.nextInt(MAX_LIST_SIZE) + 1, rand.nextInt(MAX_IT_SIZE) + 1, rand.nextInt(MAX_IT_DISTANCE) + 1, rand.nextInt(MAX_MARKEDAT) + 1); + RangeTombstoneList l2 = makeRandom(rand, rand.nextInt(MAX_LIST_SIZE) + 1, rand.nextInt(MAX_IT_SIZE) + 1, rand.nextInt(MAX_IT_DISTANCE) + 1, rand.nextInt(MAX_MARKEDAT) + 1); RangeTombstoneList l1Initial = l1.copy(); @@ -346,7 +364,7 @@ public class RangeTombstoneListTest { System.out.println("Error merging:"); System.out.println(" l1: " + toString(l1Initial)); - System.out.println(" l2: " + toString(l2)); + System.out.println("Seed was: " + seed); throw e; } }