From aa7794c83fb2a876ce6613ba573cc4fd7472dc95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jimmy=20M=C3=A5rdell?= Date: Mon, 29 Sep 2014 14:56:33 -0500 Subject: [PATCH] Fix possible infinite loop in create repair range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Jimmy Mårdell; reviewed by yukim for CASSANDRA-7983 --- CHANGES.txt | 1 + .../cassandra/service/StorageService.java | 29 +++++++--- .../service/StorageServiceServerTest.java | 55 +++++++++++++++++++ 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index dd5f7ddf98..5902d753db 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) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 11eb91d4f6..43bc1985a4 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> createRepairRangeFrom(String beginToken, String endToken) + @VisibleForTesting + Collection> createRepairRangeFrom(String beginToken, String endToken) { Token parsedBeginToken = getPartitioner().getTokenFactory().fromString(beginToken); Token parsedEndToken = getPartitioner().getTokenFactory().fromString(endToken); - Deque> 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) - { - repairingRange.addFirst(new Range<>(previous, parsedEndToken)); + ArrayList> repairingRange = new ArrayList<>(); - parsedEndToken = previous; - previous = tokenMetadata.getPredecessor(previous); + ArrayList tokens = new ArrayList<>(tokenMetadata.sortedTokens()); + if (!tokens.contains(parsedBeginToken)) + { + tokens.add(parsedBeginToken); + } + if (!tokens.contains(parsedEndToken)) + { + tokens.add(parsedEndToken); + } + // tokens now contain all tokens including our endpoints + Collections.sort(tokens); + + int start = tokens.indexOf(parsedBeginToken), end = tokens.indexOf(parsedEndToken); + for (int i = start; i != end; i = (i+1) % tokens.size()) + { + Range range = new Range<>(tokens.get(i), tokens.get((i+1) % tokens.size())); + repairingRange.add(range); } - repairingRange.addFirst(new Range<>(parsedBeginToken, parsedEndToken)); return repairingRange; } diff --git a/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java b/test/unit/org/apache/cassandra/service/StorageServiceServerTest.java index 503a730f5a..d78c7d6aa3 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(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 configOptions = new HashMap(); + configOptions.put("replication_factor", "3"); + + Keyspace.clear("Keyspace1"); + KSMetaData meta = KSMetaData.newKeyspace("Keyspace1", "SimpleStrategy", configOptions, false); + Schema.instance.setKeyspaceDefinition(meta); + + Collection> repairRangeFrom = StorageService.instance.createRepairRangeFrom("1500", "3700"); + assert repairRangeFrom.size() == 3; + assert repairRangeFrom.contains(new Range(new LongToken(1500L), new LongToken(2000L))); + assert repairRangeFrom.contains(new Range(new LongToken(2000L), new LongToken(3000L))); + assert repairRangeFrom.contains(new Range(new LongToken(3000L), new LongToken(3700L))); + + repairRangeFrom = StorageService.instance.createRepairRangeFrom("500", "700"); + assert repairRangeFrom.size() == 1; + assert repairRangeFrom.contains(new Range(new LongToken(500L), new LongToken(700L))); + + repairRangeFrom = StorageService.instance.createRepairRangeFrom("500", "1700"); + assert repairRangeFrom.size() == 2; + assert repairRangeFrom.contains(new Range(new LongToken(500L), new LongToken(1000L))); + assert repairRangeFrom.contains(new Range(new LongToken(1000L), new LongToken(1700L))); + + repairRangeFrom = StorageService.instance.createRepairRangeFrom("2500", "2300"); + assert repairRangeFrom.size() == 5; + assert repairRangeFrom.contains(new Range(new LongToken(2500L), new LongToken(3000L))); + assert repairRangeFrom.contains(new Range(new LongToken(3000L), new LongToken(4000L))); + assert repairRangeFrom.contains(new Range(new LongToken(4000L), new LongToken(1000L))); + assert repairRangeFrom.contains(new Range(new LongToken(1000L), new LongToken(2000L))); + assert repairRangeFrom.contains(new Range(new LongToken(2000L), new LongToken(2300L))); + + repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "3000"); + assert repairRangeFrom.size() == 1; + assert repairRangeFrom.contains(new Range(new LongToken(2000L), new LongToken(3000L))); + + repairRangeFrom = StorageService.instance.createRepairRangeFrom("2000", "2000"); + assert repairRangeFrom.size() == 0; + } }