cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yu...@apache.org
Subject [2/6] git commit: Fix possible infinite loop in create repair range
Date Tue, 30 Sep 2014 15:59:44 GMT
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/cassandra-2.1
Commit: aa7794c83fb2a876ce6613ba573cc4fd7472dc95
Parents: 6b8b8e0
Author: Jimmy Mårdell <yarin@spotify.com>
Authored: Mon Sep 29 14:56:33 2014 -0500
Committer: Yuki Morishita <yukim@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;
+    }
 }


Mime
View raw message