You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by yu...@apache.org on 2014/09/30 17:59:45 UTC
[3/6] git commit: Fix possible infinite loop in create repair range
Fix possible infinite loop in create repair range
patch by Jimmy Mårdell; reviewed by yukim for CASSANDRA-7983
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/aa7794c8
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/aa7794c8
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/aa7794c8
Branch: refs/heads/trunk
Commit: aa7794c83fb2a876ce6613ba573cc4fd7472dc95
Parents: 6b8b8e0
Author: Jimmy Mårdell <ya...@spotify.com>
Authored: Mon Sep 29 14:56:33 2014 -0500
Committer: Yuki Morishita <yu...@apache.org>
Committed: Tue Sep 30 09:13:01 2014 -0500
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../cassandra/service/StorageService.java | 27 +++++++---
.../service/StorageServiceServerTest.java | 55 ++++++++++++++++++++
3 files changed, 75 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/aa7794c8/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index dd5f7dd..5902d75 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -25,6 +25,7 @@
* Make repair no-op when RF=1 (CASSANDRA-7864)
* Fix NPE when table dropped during streaming (CASSANDRA-7946)
* Fix wrong progress when streaming uncompressed (CASSANDRA-7878)
+ * Fix possible infinite loop in creating repair range (CASSANDRA-7983)
Merged from 1.2:
* Don't index tombstones (CASSANDRA-7828)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/aa7794c8/src/java/org/apache/cassandra/service/StorageService.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index 11eb91d..43bc198 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -2514,22 +2514,33 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
* @return collection of ranges that match ring layout in TokenMetadata
*/
@SuppressWarnings("unchecked")
- private Collection<Range<Token>> createRepairRangeFrom(String beginToken, String endToken)
+ @VisibleForTesting
+ Collection<Range<Token>> createRepairRangeFrom(String beginToken, String endToken)
{
Token parsedBeginToken = getPartitioner().getTokenFactory().fromString(beginToken);
Token parsedEndToken = getPartitioner().getTokenFactory().fromString(endToken);
- Deque<Range<Token>> repairingRange = new ArrayDeque<>();
// Break up given range to match ring layout in TokenMetadata
- Token previous = tokenMetadata.getPredecessor(TokenMetadata.firstToken(tokenMetadata.sortedTokens(), parsedEndToken));
- while (parsedBeginToken.compareTo(previous) < 0)
+ ArrayList<Range<Token>> repairingRange = new ArrayList<>();
+
+ ArrayList<Token> tokens = new ArrayList<>(tokenMetadata.sortedTokens());
+ if (!tokens.contains(parsedBeginToken))
+ {
+ tokens.add(parsedBeginToken);
+ }
+ if (!tokens.contains(parsedEndToken))
{
- repairingRange.addFirst(new Range<>(previous, parsedEndToken));
+ tokens.add(parsedEndToken);
+ }
+ // tokens now contain all tokens including our endpoints
+ Collections.sort(tokens);
- parsedEndToken = previous;
- previous = tokenMetadata.getPredecessor(previous);
+ int start = tokens.indexOf(parsedBeginToken), end = tokens.indexOf(parsedEndToken);
+ for (int i = start; i != end; i = (i+1) % tokens.size())
+ {
+ Range<Token> range = new Range<>(tokens.get(i), tokens.get((i+1) % tokens.size()));
+ repairingRange.add(range);
}
- repairingRange.addFirst(new Range<>(parsedBeginToken, parsedEndToken));
return repairingRange;
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/aa7794c8/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java
index 503a730..d78c7d6 100644
--- a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java
+++ b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java
@@ -26,6 +26,10 @@ import java.util.*;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;
+
+import org.apache.cassandra.dht.BigIntegerToken;
+import org.apache.cassandra.dht.LongToken;
+import org.apache.cassandra.dht.Murmur3Partitioner;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -263,4 +267,55 @@ public class StorageServiceServerTest
assert primaryRanges.size() == 1;
assert primaryRanges.contains(new Range<Token>(new StringToken("B"), new StringToken("C")));
}
+
+ @Test
+ public void testCreateRepairRangeFrom() throws Exception
+ {
+ StorageService.instance.setPartitionerUnsafe(new Murmur3Partitioner());
+
+ TokenMetadata metadata = StorageService.instance.getTokenMetadata();
+ metadata.clearUnsafe();
+
+ metadata.updateNormalToken(new LongToken(1000L), InetAddress.getByName("127.0.0.1"));
+ metadata.updateNormalToken(new LongToken(2000L), InetAddress.getByName("127.0.0.2"));
+ metadata.updateNormalToken(new LongToken(3000L), InetAddress.getByName("127.0.0.3"));
+ metadata.updateNormalToken(new LongToken(4000L), InetAddress.getByName("127.0.0.4"));
+
+ Map<String, String> configOptions = new HashMap<String, String>();
+ configOptions.put("replication_factor", "3");
+
+ Keyspace.clear("Keyspace1");
+ KSMetaData meta = KSMetaData.newKeyspace("Keyspace1", "SimpleStrategy", configOptions, false);
+ Schema.instance.setKeyspaceDefinition(meta);
+
+ Collection<Range<Token>> repairRangeFrom = StorageService.instance.createRepairRangeFrom("1500", "3700");
+ assert repairRangeFrom.size() == 3;
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(1500L), new LongToken(2000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(2000L), new LongToken(3000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(3000L), new LongToken(3700L)));
+
+ repairRangeFrom = StorageService.instance.createRepairRangeFrom("500", "700");
+ assert repairRangeFrom.size() == 1;
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(500L), new LongToken(700L)));
+
+ repairRangeFrom = StorageService.instance.createRepairRangeFrom("500", "1700");
+ assert repairRangeFrom.size() == 2;
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(500L), new LongToken(1000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(1000L), new LongToken(1700L)));
+
+ repairRangeFrom = StorageService.instance.createRepairRangeFrom("2500", "2300");
+ assert repairRangeFrom.size() == 5;
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(2500L), new LongToken(3000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(3000L), new LongToken(4000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(4000L), new LongToken(1000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(1000L), new LongToken(2000L)));
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(2000L), new LongToken(2300L)));
+
+ repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "3000");
+ assert repairRangeFrom.size() == 1;
+ assert repairRangeFrom.contains(new Range<Token>(new LongToken(2000L), new LongToken(3000L)));
+
+ repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "2000");
+ assert repairRangeFrom.size() == 0;
+ }
}