From 7c4deafc7484bb8e84b4e69c04aef842814e7049 Mon Sep 17 00:00:00 2001 From: Nikita Date: Mon, 4 Apr 2016 11:56:10 +0300 Subject: [PATCH] SCAN commands result handling fixed. #463 --- .../org/redisson/RedissonBaseIterator.java | 96 +++++++++++++++++++ .../org/redisson/RedissonBaseMapIterator.java | 58 +++++++---- src/main/java/org/redisson/RedissonKeys.java | 51 ++-------- .../org/redisson/RedissonMapIterator.java | 2 +- .../RedissonMultiMapKeysIterator.java | 2 +- .../org/redisson/RedissonScoredSortedSet.java | 47 +-------- src/main/java/org/redisson/RedissonSet.java | 58 +---------- .../java/org/redisson/RedissonSetCache.java | 58 +---------- .../redisson/RedissonSetMultimapValues.java | 59 +----------- .../java/org/redisson/RedissonMapTest.java | 21 ++++ 10 files changed, 185 insertions(+), 267 deletions(-) create mode 100644 src/main/java/org/redisson/RedissonBaseIterator.java diff --git a/src/main/java/org/redisson/RedissonBaseIterator.java b/src/main/java/org/redisson/RedissonBaseIterator.java new file mode 100644 index 000000000..ab1eaa044 --- /dev/null +++ b/src/main/java/org/redisson/RedissonBaseIterator.java @@ -0,0 +1,96 @@ +/** + * Copyright 2014 Nikita Koksharov, Nickolay Borbit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.redisson; + +import java.net.InetSocketAddress; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; + +import org.redisson.client.protocol.decoder.ListScanResult; + +abstract class RedissonBaseIterator implements Iterator { + + private List firstValues; + private Iterator iter; + private InetSocketAddress client; + private long nextIterPos; + private long startPos = -1; + + private boolean currentElementRemoved; + private boolean removeExecuted; + private V value; + + @Override + public boolean hasNext() { + if (iter == null || !iter.hasNext()) { + if (nextIterPos == -1) { + return false; + } + long prevIterPos; + do { + prevIterPos = nextIterPos; + ListScanResult res = iterator(client, nextIterPos); + client = res.getRedisClient(); + if (startPos == -1) { + startPos = res.getPos(); + } + if (nextIterPos == 0 && firstValues == null) { + firstValues = res.getValues(); + } else if (res.getValues().equals(firstValues) && res.getPos() == startPos) { + return false; + } + iter = res.getValues().iterator(); + nextIterPos = res.getPos(); + } while (!iter.hasNext() && nextIterPos != prevIterPos); + if (prevIterPos == nextIterPos && !removeExecuted) { + nextIterPos = -1; + } + } + return iter.hasNext(); + } + + abstract ListScanResult iterator(InetSocketAddress client, long nextIterPos); + + @Override + public V next() { + if (!hasNext()) { + throw new NoSuchElementException("No such element"); + } + + value = iter.next(); + currentElementRemoved = false; + return value; + } + + @Override + public void remove() { + if (currentElementRemoved) { + throw new IllegalStateException("Element been already deleted"); + } + if (iter == null) { + throw new IllegalStateException(); + } + + iter.remove(); + remove(value); + currentElementRemoved = true; + removeExecuted = true; + } + + abstract void remove(V value); + +} diff --git a/src/main/java/org/redisson/RedissonBaseMapIterator.java b/src/main/java/org/redisson/RedissonBaseMapIterator.java index 9c2b9c073..66cf04465 100644 --- a/src/main/java/org/redisson/RedissonBaseMapIterator.java +++ b/src/main/java/org/redisson/RedissonBaseMapIterator.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.NoSuchElementException; +import org.redisson.client.protocol.decoder.ListScanResult; import org.redisson.client.protocol.decoder.MapScanResult; import org.redisson.client.protocol.decoder.ScanObjectEntry; @@ -32,10 +33,12 @@ abstract class RedissonBaseMapIterator implements Iterator { private Map firstValues; private Iterator> iter; - protected long iterPos = 0; + protected long nextIterPos; + protected long startPos = -1; protected InetSocketAddress client; private boolean finished; + private boolean currentElementRemoved; private boolean removeExecuted; protected Map.Entry entry; @@ -44,26 +47,41 @@ abstract class RedissonBaseMapIterator implements Iterator { if (finished) { return false; } + if (iter == null || !iter.hasNext()) { - MapScanResult res = iterator(); - client = res.getRedisClient(); - if (iterPos == 0 && firstValues == null) { - firstValues = convert(res.getMap()); - } else { - Map newValues = convert(res.getMap()); - if (newValues.equals(firstValues)) { - finished = true; - free(firstValues); + if (nextIterPos == -1) { + return false; + } + long prevIterPos; + do { + prevIterPos = nextIterPos; + MapScanResult res = iterator(); + client = res.getRedisClient(); + if (startPos == -1) { + startPos = res.getPos(); + } + if (nextIterPos == 0 && firstValues == null) { + firstValues = convert(res.getMap()); + } else { + Map newValues = convert(res.getMap()); + if (newValues.equals(firstValues)) { + finished = true; + free(firstValues); + free(newValues); + firstValues = null; + return false; + } free(newValues); - firstValues = null; - return false; } - free(newValues); + iter = res.getMap().entrySet().iterator(); + nextIterPos = res.getPos(); + } while (!iter.hasNext() && nextIterPos != prevIterPos); + if (prevIterPos == nextIterPos && !removeExecuted) { + nextIterPos = -1; } - iter = res.getMap().entrySet().iterator(); - iterPos = res.getPos(); } return iter.hasNext(); + } protected abstract MapScanResult iterator(); @@ -90,7 +108,7 @@ abstract class RedissonBaseMapIterator implements Iterator { } entry = iter.next(); - removeExecuted = false; + currentElementRemoved = false; return getValue(entry); } @@ -108,14 +126,16 @@ abstract class RedissonBaseMapIterator implements Iterator { @Override public void remove() { - if (removeExecuted) { + if (currentElementRemoved) { throw new IllegalStateException("Element been already deleted"); } + if (iter == null) { + throw new IllegalStateException(); + } - // lazy init iterator - hasNext(); iter.remove(); removeKey(); + currentElementRemoved = true; removeExecuted = true; } diff --git a/src/main/java/org/redisson/RedissonKeys.java b/src/main/java/org/redisson/RedissonKeys.java index e553bb169..afab001b7 100644 --- a/src/main/java/org/redisson/RedissonKeys.java +++ b/src/main/java/org/redisson/RedissonKeys.java @@ -15,6 +15,7 @@ */ package org.redisson; +import java.net.InetSocketAddress; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -23,7 +24,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.NoSuchElementException; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -90,55 +90,18 @@ public class RedissonKeys implements RKeys { } private Iterator createKeysIterator(final int slot, final String pattern) { - return new Iterator() { - - private List firstValues; - private Iterator iter; - private long iterPos; - - private boolean removeExecuted; - private String value; + return new RedissonBaseIterator() { @Override - public boolean hasNext() { - if (iter == null || !iter.hasNext()) { - ListScanResult res = scanIterator(slot, iterPos, pattern); - if (iterPos == 0 && firstValues == null) { - firstValues = res.getValues(); - } else if (res.getValues().equals(firstValues)) { - return false; - } - iter = res.getValues().iterator(); - iterPos = res.getPos(); - } - return iter.hasNext(); + ListScanResult iterator(InetSocketAddress client, long nextIterPos) { + return RedissonKeys.this.scanIterator(slot, nextIterPos, pattern); } @Override - public String next() { - if (!hasNext()) { - throw new NoSuchElementException("No such element"); - } - - value = iter.next(); - removeExecuted = false; - return value; + void remove(String value) { + RedissonKeys.this.delete(value); } - - @Override - public void remove() { - if (removeExecuted) { - throw new IllegalStateException("Element been already deleted"); - } - if (iter == null) { - throw new IllegalStateException(); - } - - iter.remove(); - delete(value); - removeExecuted = true; - } - + }; } diff --git a/src/main/java/org/redisson/RedissonMapIterator.java b/src/main/java/org/redisson/RedissonMapIterator.java index 4290e58e4..a16cfa3f5 100644 --- a/src/main/java/org/redisson/RedissonMapIterator.java +++ b/src/main/java/org/redisson/RedissonMapIterator.java @@ -29,7 +29,7 @@ public class RedissonMapIterator extends RedissonBaseMapIterator iterator() { - return map.scanIterator(client, iterPos); + return map.scanIterator(client, nextIterPos); } protected void removeKey() { diff --git a/src/main/java/org/redisson/RedissonMultiMapKeysIterator.java b/src/main/java/org/redisson/RedissonMultiMapKeysIterator.java index 03918ee60..375349e95 100644 --- a/src/main/java/org/redisson/RedissonMultiMapKeysIterator.java +++ b/src/main/java/org/redisson/RedissonMultiMapKeysIterator.java @@ -29,7 +29,7 @@ public class RedissonMultiMapKeysIterator extends RedissonBaseMapIterat } protected MapScanResult iterator() { - return map.scanIterator(client, iterPos); + return map.scanIterator(client, nextIterPos); } protected void removeKey() { diff --git a/src/main/java/org/redisson/RedissonScoredSortedSet.java b/src/main/java/org/redisson/RedissonScoredSortedSet.java index 0ffb1122d..188460143 100644 --- a/src/main/java/org/redisson/RedissonScoredSortedSet.java +++ b/src/main/java/org/redisson/RedissonScoredSortedSet.java @@ -25,7 +25,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.NoSuchElementException; import org.redisson.client.codec.Codec; import org.redisson.client.codec.ScoredCodec; @@ -245,54 +244,18 @@ public class RedissonScoredSortedSet extends RedissonExpirable implements RSc @Override public Iterator iterator() { - return new Iterator() { - - private List firstValues; - private Iterator iter; - private InetSocketAddress client; - private long iterPos; - - private boolean removeExecuted; - private V value; + return new RedissonBaseIterator() { @Override - public boolean hasNext() { - if (iter == null || !iter.hasNext()) { - ListScanResult res = scanIterator(client, iterPos); - client = res.getRedisClient(); - if (iterPos == 0 && firstValues == null) { - firstValues = res.getValues(); - } else if (res.getValues().equals(firstValues)) { - return false; - } - iter = res.getValues().iterator(); - iterPos = res.getPos(); - } - return iter.hasNext(); + ListScanResult iterator(InetSocketAddress client, long nextIterPos) { + return scanIterator(client, nextIterPos); } @Override - public V next() { - if (!hasNext()) { - throw new NoSuchElementException("No such element at index"); - } - - value = iter.next(); - removeExecuted = false; - return value; - } - - @Override - public void remove() { - if (removeExecuted) { - throw new IllegalStateException("Element been already deleted"); - } - - iter.remove(); + void remove(V value) { RedissonScoredSortedSet.this.remove(value); - removeExecuted = true; } - + }; } diff --git a/src/main/java/org/redisson/RedissonSet.java b/src/main/java/org/redisson/RedissonSet.java index 9904bcc30..2afeeb563 100644 --- a/src/main/java/org/redisson/RedissonSet.java +++ b/src/main/java/org/redisson/RedissonSet.java @@ -82,66 +82,18 @@ public class RedissonSet extends RedissonExpirable implements RSet { @Override public Iterator iterator() { - return new Iterator() { - - private List firstValues; - private Iterator iter; - private InetSocketAddress client; - private long nextIterPos; - - private boolean currentElementRemoved; - private boolean removeExecuted; - private V value; - - @Override - public boolean hasNext() { - if (iter == null || !iter.hasNext()) { - if (nextIterPos == -1) { - return false; - } - long prevIterPos = nextIterPos; - ListScanResult res = scanIterator(client, nextIterPos); - client = res.getRedisClient(); - if (nextIterPos == 0 && firstValues == null) { - firstValues = res.getValues(); - } else if (res.getValues().equals(firstValues)) { - return false; - } - iter = res.getValues().iterator(); - nextIterPos = res.getPos(); - if (prevIterPos == nextIterPos && !removeExecuted) { - nextIterPos = -1; - } - } - return iter.hasNext(); - } + return new RedissonBaseIterator() { @Override - public V next() { - if (!hasNext()) { - throw new NoSuchElementException("No such element at index"); - } - - value = iter.next(); - currentElementRemoved = false; - return value; + ListScanResult iterator(InetSocketAddress client, long nextIterPos) { + return scanIterator(client, nextIterPos); } @Override - public void remove() { - if (currentElementRemoved) { - throw new IllegalStateException("Element been already deleted"); - } - if (iter == null) { - throw new IllegalStateException(); - } - - iter.remove(); + void remove(V value) { RedissonSet.this.remove(value); - currentElementRemoved = true; - removeExecuted = true; } - + }; } diff --git a/src/main/java/org/redisson/RedissonSetCache.java b/src/main/java/org/redisson/RedissonSetCache.java index 34bc65af4..124fbfbc9 100644 --- a/src/main/java/org/redisson/RedissonSetCache.java +++ b/src/main/java/org/redisson/RedissonSetCache.java @@ -161,66 +161,18 @@ public class RedissonSetCache extends RedissonExpirable implements RSetCache< @Override public Iterator iterator() { - return new Iterator() { - - private List firstValues; - private Iterator iter; - private InetSocketAddress client; - private long nextIterPos; - - private boolean currentElementRemoved; - private boolean removeExecuted; - private V value; - - @Override - public boolean hasNext() { - if (iter == null || !iter.hasNext()) { - if (nextIterPos == -1) { - return false; - } - long prevIterPos = nextIterPos; - ListScanResult res = scanIterator(client, nextIterPos); - client = res.getRedisClient(); - if (nextIterPos == 0 && firstValues == null) { - firstValues = res.getValues(); - } else if (res.getValues().equals(firstValues)) { - return false; - } - iter = res.getValues().iterator(); - nextIterPos = res.getPos(); - if (prevIterPos == nextIterPos && !removeExecuted) { - nextIterPos = -1; - } - } - return iter.hasNext(); - } + return new RedissonBaseIterator() { @Override - public V next() { - if (!hasNext()) { - throw new NoSuchElementException("No such element at index"); - } - - value = iter.next(); - currentElementRemoved = false; - return value; + ListScanResult iterator(InetSocketAddress client, long nextIterPos) { + return scanIterator(client, nextIterPos); } @Override - public void remove() { - if (currentElementRemoved) { - throw new IllegalStateException("Element been already deleted"); - } - if (iter == null) { - throw new IllegalStateException(); - } - - iter.remove(); + void remove(V value) { RedissonSetCache.this.remove(value); - currentElementRemoved = true; - removeExecuted = true; } - + }; } diff --git a/src/main/java/org/redisson/RedissonSetMultimapValues.java b/src/main/java/org/redisson/RedissonSetMultimapValues.java index 215c82cd4..36d0fefee 100644 --- a/src/main/java/org/redisson/RedissonSetMultimapValues.java +++ b/src/main/java/org/redisson/RedissonSetMultimapValues.java @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Iterator; import java.util.List; -import java.util.NoSuchElementException; import java.util.Set; import org.redisson.client.codec.Codec; @@ -128,66 +127,18 @@ public class RedissonSetMultimapValues extends RedissonExpirable implements R @Override public Iterator iterator() { - return new Iterator() { - - private List firstValues; - private Iterator iter; - private InetSocketAddress client; - private long nextIterPos; - - private boolean currentElementRemoved; - private boolean removeExecuted; - private V value; - - @Override - public boolean hasNext() { - if (iter == null || !iter.hasNext()) { - if (nextIterPos == -1) { - return false; - } - long prevIterPos = nextIterPos; - ListScanResult res = scanIterator(client, nextIterPos); - client = res.getRedisClient(); - if (nextIterPos == 0 && firstValues == null) { - firstValues = res.getValues(); - } else if (res.getValues().equals(firstValues)) { - return false; - } - iter = res.getValues().iterator(); - nextIterPos = res.getPos(); - if (prevIterPos == nextIterPos && !removeExecuted) { - nextIterPos = -1; - } - } - return iter.hasNext(); - } + return new RedissonBaseIterator() { @Override - public V next() { - if (!hasNext()) { - throw new NoSuchElementException("No such element at index"); - } - - value = iter.next(); - currentElementRemoved = false; - return value; + ListScanResult iterator(InetSocketAddress client, long nextIterPos) { + return scanIterator(client, nextIterPos); } @Override - public void remove() { - if (currentElementRemoved) { - throw new IllegalStateException("Element been already deleted"); - } - if (iter == null) { - throw new IllegalStateException(); - } - - iter.remove(); + void remove(V value) { RedissonSetMultimapValues.this.remove(value); - currentElementRemoved = true; - removeExecuted = true; } - + }; } diff --git a/src/test/java/org/redisson/RedissonMapTest.java b/src/test/java/org/redisson/RedissonMapTest.java index 968f0e5d3..0a46a6558 100644 --- a/src/test/java/org/redisson/RedissonMapTest.java +++ b/src/test/java/org/redisson/RedissonMapTest.java @@ -23,6 +23,7 @@ import org.redisson.client.codec.StringCodec; import org.redisson.codec.JsonJacksonCodec; import org.redisson.core.Predicate; import org.redisson.core.RMap; +import org.redisson.core.RSet; import io.netty.util.concurrent.Future; @@ -257,6 +258,26 @@ public class RedissonMapTest extends BaseTest { assertThat(val2).isEqualTo(4); } + @Test + public void testIteratorRemoveHighVolume() throws InterruptedException { + RMap map = redisson.getMap("simpleMap"); + for (int i = 0; i < 10000; i++) { + map.put(i, i*10); + } + + int cnt = 0; + Iterator iterator = map.keySet().iterator(); + while (iterator.hasNext()) { + Integer integer = iterator.next(); + iterator.remove(); + cnt++; + } + Assert.assertEquals(10000, cnt); + assertThat(map).isEmpty(); + Assert.assertEquals(0, map.size()); + } + + @Test public void testIterator() { RMap rMap = redisson.getMap("123");