Skip to content

Commit cd0c28a

Browse files
Bj0rnenBj0rnen
committed
New tests and safety checks for the new generateSegments
Fixed a bug subtracting RANGE_MAX instead of RANGE_SIZE to out of bound tokens.
1 parent b433a86 commit cd0c28a

File tree

3 files changed

+109
-76
lines changed

3 files changed

+109
-76
lines changed

‎src/main/java/com/spotify/reaper/core/RepairSegment.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ public class RepairSegment {
88
private Long id;
99
private final ColumnFamily columnFamily;
1010
private final long runID;
11-
private final BigInteger startToken;
12-
private final BigInteger endToken;
11+
private final BigInteger startToken; // closed/inclusive
12+
private final BigInteger endToken; // open/exclusive
1313
private final State state;
1414
private final DateTime startTime;
1515
private final DateTime endTime;
@@ -128,6 +128,6 @@ public RepairSegment build() {
128128

129129
@Override
130130
public String toString() {
131-
return String.format("(%s,%s)", startToken.toString(), endToken.toString());
131+
return String.format("[%s,%s)", startToken.toString(), endToken.toString());
132132
}
133133
}

‎src/main/java/com/spotify/reaper/service/SegmentGenerator.java

+27-20
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,11 @@ public SegmentGenerator(String partitioner) throws ReaperException {
5252
public List<RepairSegment> generateSegments(int totalSegmentCount, List<BigInteger> ringTokens)
5353
throws ReaperException {
5454
int tokenRangeCount = ringTokens.size();
55-
BigInteger start;
56-
BigInteger stop;
5755

58-
// BigInteger total = BigInteger.ZERO;
5956
List<RepairSegment> repairSegments = Lists.newArrayList();
60-
6157
for (int i = 0; i < tokenRangeCount; i++) {
62-
start = ringTokens.get(i);
63-
stop = ringTokens.get((i + 1) % tokenRangeCount);
58+
BigInteger start = ringTokens.get(i);
59+
BigInteger stop = ringTokens.get((i + 1) % tokenRangeCount);
6460

6561
if (!inRange(start) || !inRange(stop)) {
6662
throw new ReaperException(String.format("Tokens (%s,%s) not in range of %s",
@@ -70,12 +66,13 @@ public List<RepairSegment> generateSegments(int totalSegmentCount, List<BigInteg
7066
throw new ReaperException(String.format("Tokens (%s,%s): two nodes have the same token",
7167
start, stop));
7268
}
73-
if (lowerThan(stop, start)) {
69+
70+
BigInteger rangeSize = stop.subtract(start);
71+
if (lowerThan(rangeSize, BigInteger.ZERO)) {
7472
// wrap around case
75-
stop = stop.add(RANGE_SIZE);
73+
rangeSize = rangeSize.add(RANGE_SIZE);
7674
}
7775

78-
BigInteger rangeSize = stop.subtract(start);
7976
// the below, in essence, does this:
8077
// segmentCount = ceiling((rangeSize / RANGE_SIZE) * totalSegmentCount)
8178
BigInteger[] segmentCountAndRemainder =
@@ -85,32 +82,42 @@ public List<RepairSegment> generateSegments(int totalSegmentCount, List<BigInteg
8582

8683
LOG.info("Dividing token range [{},{}) into {} segments", start, stop, segmentCount);
8784

88-
List<BigInteger> reaperTokens = Lists.newArrayList();
85+
// Make a list of all the endpoints for the repair segments, including both start and stop
86+
List<BigInteger> endpointTokens = Lists.newArrayList();
8987
for (int j = 0; j <= segmentCount; j++) {
9088
BigInteger reaperToken =
9189
start.add(
9290
rangeSize
9391
.multiply(BigInteger.valueOf(j))
9492
.divide(BigInteger.valueOf(segmentCount)));
9593
if (greaterThan(reaperToken, RANGE_MAX))
96-
reaperToken = reaperToken.subtract(RANGE_MAX);
97-
reaperTokens.add(reaperToken);
94+
reaperToken = reaperToken.subtract(RANGE_SIZE);
95+
endpointTokens.add(reaperToken);
9896
}
9997

98+
// Append the segments between the endpoints
10099
for (int j = 0; j < segmentCount; j++)
101100
{
102101
repairSegments.add(new RepairSegment.RepairSegmentBuilder()
103-
.startToken(reaperTokens.get(j))
104-
.endToken(reaperTokens.get(j + 1))
105-
.build());
106-
LOG.debug("Segment #{}: [{},{})", j + 1, reaperTokens.get(j),
107-
reaperTokens.get(j + 1));
102+
.startToken(endpointTokens.get(j))
103+
.endToken(endpointTokens.get(j + 1))
104+
.build());
105+
LOG.debug("Segment #{}: [{},{})", j + 1, endpointTokens.get(j),
106+
endpointTokens.get(j + 1));
108107
}
109108
}
110109

111-
// if (!total.equals(RANGE_SIZE)) {
112-
// throw new ReaperException("Not entire ring would get repaired");
113-
// }
110+
// verify that the whole range is repaired
111+
BigInteger total = BigInteger.ZERO;
112+
for (RepairSegment segment : repairSegments) {
113+
BigInteger size = segment.getEndToken().subtract(segment.getStartToken());
114+
if (lowerThan(size, BigInteger.ZERO))
115+
size = size.add(RANGE_SIZE);
116+
total = total.add(size);
117+
}
118+
if (!total.equals(RANGE_SIZE)) {
119+
throw new ReaperException("Not entire ring would get repaired");
120+
}
114121
return repairSegments;
115122
}
116123

‎src/test/java/com/spotify/reaper/service/SegmentGeneratorTest.java

+79-53
Original file line numberDiff line numberDiff line change
@@ -19,80 +19,104 @@ public class SegmentGeneratorTest {
1919

2020
@Test
2121
public void testGenerateSegments() throws Exception {
22-
/*
23-
List<String> tokenStrings = Lists.newArrayList("0", "1",
24-
"56713727820156410577229101238628035242", "56713727820156410577229101238628035243",
25-
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485");
26-
List<BigInteger> tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
27-
@Nullable
28-
@Override
29-
public BigInteger apply(@Nullable String s) {
30-
return new BigInteger(s);
31-
}
32-
});
22+
List<BigInteger> tokens = Lists.transform(
23+
Lists.newArrayList(
24+
"0", "1",
25+
"56713727820156410577229101238628035242", "56713727820156410577229101238628035243",
26+
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485"),
27+
new Function<String, BigInteger>() {
28+
@Nullable
29+
@Override
30+
public BigInteger apply(@Nullable String s) {
31+
return new BigInteger(s);
32+
}
33+
}
34+
);
3335

3436
SegmentGenerator generator = new SegmentGenerator("foo.bar.RandomPartitioner");
35-
List<RepairSegment> segments = generator.generateSegments(3, tokens);
36-
assertEquals(11, segments.size());
37-
assertEquals("(0,1)", segments.get(0).toString());
38-
assertEquals("(1,18904575940052136859076367079542678415)", segments.get(1).toString());
39-
assertEquals("(18904575940052136859076367079542678415,37809151880104273718152734159085356829)",
40-
segments.get(2).toString());
41-
assertEquals("(37809151880104273718152734159085356829,56713727820156410577229101238628035242)",
42-
segments.get(3).toString());
43-
assertEquals("(56713727820156410577229101238628035242,75618303760208547436305468318170713657)",
44-
segments.get(4).toString());
45-
assertEquals("(75618303760208547436305468318170713657,94522879700260684295381835397713392072)",
37+
List<RepairSegment> segments = generator.generateSegments(10, tokens);
38+
assertEquals(15, segments.size());
39+
assertEquals("[0,1)",
40+
segments.get(0).toString());
41+
assertEquals("[56713727820156410577229101238628035242,56713727820156410577229101238628035243)",
4642
segments.get(5).toString());
47-
assertEquals("(94522879700260684295381835397713392072,113427455640312821154458202477256070484)",
48-
segments.get(6).toString());
49-
assertEquals("(113427455640312821154458202477256070484,113427455640312821154458202477256070485)",
50-
segments.get(7).toString());
51-
assertEquals("(113427455640312821154458202477256070485,132332031580364958013534569556798748900)",
52-
segments.get(8).toString());
53-
assertEquals("(132332031580364958013534569556798748900,151236607520417094872610936636341427315)",
54-
segments.get(9).toString());
55-
assertEquals("(151236607520417094872610936636341427315,0)", segments.get(10).toString());
43+
assertEquals("[113427455640312821154458202477256070484,113427455640312821154458202477256070485)",
44+
segments.get(10).toString());
45+
46+
47+
tokens = Lists.transform(
48+
Lists.newArrayList(
49+
"5", "6",
50+
"56713727820156410577229101238628035242", "56713727820156410577229101238628035243",
51+
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485"),
52+
new Function<String, BigInteger>() {
53+
@Nullable
54+
@Override
55+
public BigInteger apply(@Nullable String s) {
56+
return new BigInteger(s);
57+
}
58+
}
59+
);
60+
61+
segments = generator.generateSegments(10, tokens);
62+
assertEquals(15, segments.size());
63+
assertEquals("[5,6)",
64+
segments.get(0).toString());
65+
assertEquals("[56713727820156410577229101238628035242,56713727820156410577229101238628035243)",
66+
segments.get(5).toString());
67+
assertEquals("[113427455640312821154458202477256070484,113427455640312821154458202477256070485)",
68+
segments.get(10).toString());
69+
}
5670

57-
tokenStrings = Lists.newArrayList("5", "6",
58-
"56713727820156410577229101238628035242", "56713727820156410577229101238628035242",
59-
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485");
60-
tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
71+
@Test(expected=ReaperException.class)
72+
public void testZeroSizeRange() throws Exception {
73+
List<String> tokenStrings = Lists.newArrayList(
74+
"0", "1",
75+
"56713727820156410577229101238628035242", "56713727820156410577229101238628035242",
76+
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485");
77+
List<BigInteger> tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
6178
@Nullable
6279
@Override
6380
public BigInteger apply(@Nullable String s) {
6481
return new BigInteger(s);
6582
}
6683
});
6784

68-
generator = new SegmentGenerator("foo.bar.RandomPartitioner");
69-
segments = generator.generateSegments(3, tokens);
70-
assertEquals(11, segments.size());
71-
assertEquals("(5,6)", segments.get(0).toString());
72-
assertEquals("(6,18904575940052136859076367079542678419)", segments.get(1).toString());
73-
assertEquals("(151236607520417094872610936636341427319,5)", segments.get(10).toString());
85+
SegmentGenerator generator = new SegmentGenerator("foo.bar.RandomPartitioner");
86+
generator.generateSegments(10, tokens);
87+
}
7488

75-
tokenStrings = Lists.newArrayList("-9223372036854775808", "-9223372036854775807",
76-
"-3074457345618258603", "-3074457345618258602", "3074457345618258602", "3074457345618258603");
77-
tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
89+
@Test
90+
public void testRotatedRing() throws Exception {
91+
List<String> tokenStrings = Lists.newArrayList(
92+
"56713727820156410577229101238628035243", "113427455640312821154458202477256070484",
93+
"113427455640312821154458202477256070485", "5",
94+
"6", "56713727820156410577229101238628035242");
95+
List<BigInteger> tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
7896
@Nullable
7997
@Override
8098
public BigInteger apply(@Nullable String s) {
8199
return new BigInteger(s);
82100
}
83101
});
84102

85-
generator = new SegmentGenerator("foo.bar.Murmur3Partitioner");
86-
segments = generator.generateSegments(3, tokens);
87-
assertEquals(12, segments.size());
88-
*/
103+
SegmentGenerator generator = new SegmentGenerator("foo.bar.RandomPartitioner");
104+
List<RepairSegment> segments = generator.generateSegments(10, tokens);
105+
assertEquals(15, segments.size());
106+
assertEquals("[113427455640312821154458202477256070484,113427455640312821154458202477256070485)",
107+
segments.get(4).toString());
108+
assertEquals("[5,6)",
109+
segments.get(9).toString());
110+
assertEquals("[56713727820156410577229101238628035242,56713727820156410577229101238628035243)",
111+
segments.get(14).toString());
89112
}
90113

91114
@Test(expected=ReaperException.class)
92-
public void testZeroSizeRange() throws Exception {
93-
List<String> tokenStrings = Lists.newArrayList("0", "1",
94-
"56713727820156410577229101238628035242", "56713727820156410577229101238628035242",
95-
"113427455640312821154458202477256070484", "113427455640312821154458202477256070485");
115+
public void testDisorderedRing() throws Exception {
116+
List<String> tokenStrings = Lists.newArrayList(
117+
"0", "113427455640312821154458202477256070485", "1",
118+
"56713727820156410577229101238628035242", "56713727820156410577229101238628035243",
119+
"113427455640312821154458202477256070484");
96120
List<BigInteger> tokens = Lists.transform(tokenStrings, new Function<String, BigInteger>() {
97121
@Nullable
98122
@Override
@@ -102,7 +126,9 @@ public BigInteger apply(@Nullable String s) {
102126
});
103127

104128
SegmentGenerator generator = new SegmentGenerator("foo.bar.RandomPartitioner");
105-
List<RepairSegment> segments = generator.generateSegments(3, tokens);
129+
generator.generateSegments(10, tokens);
130+
// Will throw an exception when concluding that the repair segments don't add up.
131+
// This is because the tokens were supplied out of order.
106132
}
107133

108134
@Test

0 commit comments

Comments
 (0)