Repository: cassandra Updated Branches: refs/heads/trunk c055ab997 -> e338d2fa8
Fix flapping RangeTombstoneListTest patch by slebresne; reviewed by JoshuaMcKenzie for CASSANDRA-9799 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e338d2fa Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e338d2fa Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e338d2fa Branch: refs/heads/trunk Commit: e338d2fa8ba1eeb085290612559ab6782fecdd1d Parents: c055ab9 Author: Sylvain Lebresne <[email protected]> Authored: Tue Jul 14 13:31:22 2015 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Fri Jul 24 15:08:16 2015 +0200 ---------------------------------------------------------------------- .../apache/cassandra/db/RangeTombstoneList.java | 113 ++----------------- .../cassandra/db/RangeTombstoneListTest.java | 45 +++++++- 2 files changed, 50 insertions(+), 108 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/e338d2fa/src/java/org/apache/cassandra/db/RangeTombstoneList.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/RangeTombstoneList.java b/src/java/org/apache/cassandra/db/RangeTombstoneList.java index 59bebf7..32b3625 100644 --- a/src/java/org/apache/cassandra/db/RangeTombstoneList.java +++ b/src/java/org/apache/cassandra/db/RangeTombstoneList.java @@ -576,24 +576,18 @@ public class RangeTombstoneList implements Iterable<RangeTombstone>, IMeasurable if (cmp <= 0) { // We do overwrite fully: - // update the current element until it's end and continue - // on with the next element (with the new inserted start == current end). + // update the current element until it's end and continue on with the next element (with the new inserted start == current end). - // If we're on the last element, we can optimize - if (i == size-1) + // If we're on the last element, or if we stop before the next start, we set the current element and are done + // Note that the comparison below is inclusive: if a end equals a start, this means they form a boundary, or + // in other words that they are for the same element but one is inclusive while the other exclusive. In which case we know + // we're good with the next element + if (i == size-1 || comparator.compare(end, starts[i+1]) <= 0) { setInternal(i, start, end, markedAt, delTime); return; } - // Otherwise, the new element overwite until the min(end, next start) - if (comparator.compare(end, starts[i+1]) < 0) - { - setInternal(i, start, end, markedAt, delTime); - // We have fully handled the new element so we're done - return; - } - setInternal(i, start, starts[i+1].invert(), markedAt, delTime); start = starts[i+1]; i++; @@ -619,10 +613,9 @@ public class RangeTombstoneList implements Iterable<RangeTombstone>, IMeasurable // If the new interval starts before the current one, insert that new interval if (comparator.compare(start, starts[i]) < 0) { - // If we stop before the start of the current element, just insert the new - // interval and we're done; otherwise insert until the beginning of the - // current element - if (comparator.compare(end, starts[i]) < 0) + // If we stop before the start of the current element, just insert the new interval and we're done; + // otherwise insert until the beginning of the current element + if (comparator.compare(end, starts[i]) <= 0) { addInternal(i, start, end, markedAt, delTime); return; @@ -783,92 +776,4 @@ public class RangeTombstoneList implements Iterable<RangeTombstone>, IMeasurable + ObjectSizes.sizeOfArray(markedAts) + ObjectSizes.sizeOfArray(delTimes); } - - // TODO: This should be moved someplace else as it shouldn't be used directly: some ranges might become - // complex deletion times. We'll also only need this for backward compatibility, this isn't used otherwise. - public static class Serializer implements IVersionedSerializer<RangeTombstoneList> - { - private final LegacyLayout layout; - - public Serializer(LegacyLayout layout) - { - this.layout = layout; - } - - public void serialize(RangeTombstoneList tombstones, DataOutputPlus out, int version) throws IOException - { - // TODO - throw new UnsupportedOperationException(); - //if (tombstones == null) - //{ - // out.writeInt(0); - // return; - //} - - //out.writeInt(tombstones.size); - //for (int i = 0; i < tombstones.size; i++) - //{ - // layout.serializer().serialize(tombstones.starts[i], out); - // layout.serializer().serialize(tombstones.ends[i], out); - // out.writeInt(tombstones.delTimes[i]); - // out.writeLong(tombstones.markedAts[i]); - //} - } - - public RangeTombstoneList deserialize(DataInputPlus in, int version) throws IOException - { - // TODO - throw new UnsupportedOperationException(); - - //int size = in.readInt(); - //if (size == 0) - // return null; - - //RangeTombstoneList tombstones = new RangeTombstoneList(layout, size); - - //for (int i = 0; i < size; i++) - //{ - // Slice.Bound start = layout.serializer().deserialize(in); - // Slice.Bound end = layout.serializer().deserialize(in); - // int delTime = in.readInt(); - // long markedAt = in.readLong(); - - // if (version >= MessagingService.VERSION_20) - // { - // tombstones.setInternal(i, start, end, markedAt, delTime); - // } - // else - // { - // /* - // * The old implementation used to have range sorted by left value, but with potentially - // * overlapping range. So we need to use the "slow" path. - // */ - // tombstones.add(start, end, markedAt, delTime); - // } - //} - - //// The "slow" path take care of updating the size, but not the fast one - //if (version >= MessagingService.VERSION_20) - // tombstones.size = size; - //return tombstones; - } - - public long serializedSize(RangeTombstoneList tombstones, int version) - { - // TODO - throw new UnsupportedOperationException(); - //if (tombstones == null) - // return TypeSizes.sizeof(0); - - //long size = TypeSizes.sizeof(tombstones.size); - //for (int i = 0; i < tombstones.size; i++) - //{ - // size += type.serializer().serializedSize(tombstones.starts[i], typeSizes); - // size += type.serializer().serializedSize(tombstones.ends[i], typeSizes); - // size += TypeSizes.sizeof(tombstones.delTimes[i]); - // size += TypeSizes.sizeof(tombstones.markedAts[i]); - //} - //return size; - } - } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/e338d2fa/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java b/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java index d6d6d07..79efb87 100644 --- a/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java +++ b/test/unit/org/apache/cassandra/db/RangeTombstoneListTest.java @@ -20,6 +20,10 @@ package org.apache.cassandra.db; import java.nio.ByteBuffer; import java.util.*; +import java.util.regex.Pattern; +import java.util.regex.Matcher; + +import com.google.common.base.Joiner; import org.junit.Test; import static org.junit.Assert.*; @@ -257,6 +261,15 @@ public class RangeTombstoneListTest assert !iter.hasNext(); } + @Test + public void addAllBugFrom9799() + { + RangeTombstoneList l1 = fromString("{ (6, 7]@4 - (7, 8)@1 - [12, 12]@0 - [13, 13]@0 - (20, 21)@3 - [27, 27]@2 - (33, 34)@2 - (35, 36]@4 - (40, 41]@0 - (42, 43)@2 - (44, 45)@3 - [47, 47]@1 - (47, 48)@0 - [55, 55]@4 - [61, 61]@4 - [67, 67]@0 - [70, 70]@4 - [77, 77]@1 - (83, 84)@1 - [90, 90]@0 - (91, 92]@4 - [93, 93]@0 - (94, 95)@2 - (100, 101]@3 - (103, 104]@0 - (108, 109]@2 - (115, 116]@3 - (116, 117]@3 - (118, 119)@4 - (125, 126)@2 - [131, 131]@1 - [132, 132]@3 - [139, 139]@0 - [145, 145]@1 - (145, 146]@3 - (147, 148]@4 - (150, 151]@1 - (156, 157)@2 - (158, 159)@2 - [164, 164]@4 - (168, 169)@0 - (171, 172)@4 - (173, 174]@0 - [179, 179]@1 - (186, 187]@4 - [191, 191]@1 }"); + RangeTombstoneList l2 = fromString("{ (1, 12)@8 - [12, 13)@8 - [13, 18]@7 }"); + l1.addAll(l2); + assertValid(l1); + } + private RangeTombstoneList makeRandom(Random rand, int size, int maxItSize, int maxItDistance, int maxMarkedAt) { RangeTombstoneList l = new RangeTombstoneList(cmp, size); @@ -366,13 +379,37 @@ public class RangeTombstoneListTest private static String toString(RangeTombstoneList l) { - StringBuilder sb = new StringBuilder(); - sb.append("{"); + String[] ranges = new String[l.size()]; + int i = 0; for (RangeTombstone rt : l) - sb.append(" ").append(toString(rt)); - return sb.append(" }").toString(); + ranges[i++] = toString(rt); + + return "{ " + Joiner.on(" - ").join(ranges) + " }"; } + private static RangeTombstone rangeFromString(String range) + { + Matcher matcher = Pattern.compile("([\\[(])(\\d+), (\\d+)([)\\]])@(\\d+)").matcher(range.trim()); + matcher.matches(); + boolean isOpenInclusive = matcher.group(1).equals("["); + int start = Integer.valueOf(matcher.group(2)); + int end = Integer.valueOf(matcher.group(3)); + boolean isCloseInclusive = matcher.group(4).equals("]"); + long timestamp = Long.valueOf(matcher.group(5)); + return rt(start, isOpenInclusive, end, isCloseInclusive, timestamp); + } + + private static RangeTombstoneList fromString(String str) + { + str = str.trim(); + String[] ranges = str.substring(1, str.length() - 1).split("-", 0); + RangeTombstoneList l = new RangeTombstoneList(cmp, ranges.length); + for (String range : ranges) + l.add(rangeFromString(range)); + return l; + } + + private static Clustering clustering(int i) { return new Clustering(bb(i));
