From 7eaf465f577e5fb38a964f0dff9bf89ff1f29384 Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 17:10:30 +0300 Subject: [PATCH 01/10] Fixed - Incorrect RedissonRedLock.tryLock behaviour. #624 --- .../java/org/redisson/RedissonMultiLock.java | 98 ++++++++++++------- .../java/org/redisson/RedissonRedLock.java | 33 +------ .../org/redisson/RedissonRedLockTest.java | 47 +++++++++ 3 files changed, 117 insertions(+), 61 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMultiLock.java b/redisson/src/main/java/org/redisson/RedissonMultiLock.java index 5b9e7613c..8d4e7abfc 100644 --- a/redisson/src/main/java/org/redisson/RedissonMultiLock.java +++ b/redisson/src/main/java/org/redisson/RedissonMultiLock.java @@ -20,8 +20,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.ListIterator; import java.util.Map; -import java.util.Map.Entry; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.TimeUnit; @@ -196,36 +196,12 @@ public class RedissonMultiLock implements Lock { @Override public boolean tryLock() { - Map> tryLockFutures = new HashMap>(locks.size()); - for (RLock lock : locks) { - tryLockFutures.put(lock, lock.tryLockAsync()); - } - - return sync(tryLockFutures); - } - - protected boolean sync(Map> tryLockFutures) { - List lockedLocks = new ArrayList(tryLockFutures.size()); - RuntimeException latestException = null; - for (Entry> entry : tryLockFutures.entrySet()) { - try { - if (entry.getValue().syncUninterruptibly().getNow()) { - lockedLocks.add(entry.getKey()); - } - } catch (RuntimeException e) { - latestException = e; - } - } - - if (lockedLocks.size() < tryLockFutures.size()) { - unlockInner(lockedLocks); - if (latestException != null) { - throw latestException; - } + try { + return tryLock(-1, -1, null); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); return false; } - - return true; } protected void unlockInner(Collection locks) { @@ -243,14 +219,70 @@ public class RedissonMultiLock implements Lock { public boolean tryLock(long waitTime, TimeUnit unit) throws InterruptedException { return tryLock(waitTime, -1, unit); } + + protected int failedLocksLimit() { + return 1; + } public boolean tryLock(long waitTime, long leaseTime, TimeUnit unit) throws InterruptedException { - Map> tryLockFutures = new HashMap>(locks.size()); - for (RLock lock : locks) { - tryLockFutures.put(lock, lock.tryLockAsync(waitTime, leaseTime, unit)); + long newLeaseTime = -1; + if (leaseTime != -1) { + newLeaseTime = waitTime*2; + } + + long time = System.currentTimeMillis(); + long remainTime = -1; + if (waitTime != -1) { + remainTime = unit.toMillis(waitTime); + } + int failedLocksLimit = failedLocksLimit(); + List lockedLocks = new ArrayList(locks.size()); + for (ListIterator iterator = locks.listIterator(); iterator.hasNext();) { + RLock lock = iterator.next(); + boolean lockAcquired; + if (waitTime == -1 && leaseTime == -1) { + lockAcquired = lock.tryLock(); + } else { + lockAcquired = lock.tryLock(unit.convert(remainTime, TimeUnit.MILLISECONDS), newLeaseTime, unit); + } + + if (lockAcquired) { + lockedLocks.add(lock); + } else { + failedLocksLimit--; + if (failedLocksLimit == 0) { + unlockInner(lockedLocks); + if (waitTime == -1 && leaseTime == -1) { + return false; + } + failedLocksLimit = failedLocksLimit(); + lockedLocks.clear(); + } + } + + if (remainTime != -1) { + remainTime -= (System.currentTimeMillis() - time); + time = System.currentTimeMillis(); + if (remainTime < 0) { + unlockInner(lockedLocks); + return false; + } + } } - return sync(tryLockFutures); + if (leaseTime != -1) { + List> futures = new ArrayList>(lockedLocks.size()); + for (RLock rLock : lockedLocks) { + RFuture future = rLock.expireAsync(unit.toMillis(leaseTime), TimeUnit.MILLISECONDS); + futures.add(future); + } + + for (RFuture rFuture : futures) { + rFuture.syncUninterruptibly(); + } + } + + return true; } diff --git a/redisson/src/main/java/org/redisson/RedissonRedLock.java b/redisson/src/main/java/org/redisson/RedissonRedLock.java index 25b296d01..de09713e8 100644 --- a/redisson/src/main/java/org/redisson/RedissonRedLock.java +++ b/redisson/src/main/java/org/redisson/RedissonRedLock.java @@ -15,14 +15,10 @@ */ package org.redisson; -import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import java.util.Queue; import java.util.concurrent.atomic.AtomicReference; -import org.redisson.api.RFuture; import org.redisson.api.RLock; import io.netty.util.concurrent.Future; @@ -47,31 +43,12 @@ public class RedissonRedLock extends RedissonMultiLock { public RedissonRedLock(RLock... locks) { super(locks); } - - protected boolean sync(Map> tryLockFutures) { - List lockedLocks = new ArrayList(tryLockFutures.size()); - RuntimeException latestException = null; - for (Entry> entry : tryLockFutures.entrySet()) { - try { - if (entry.getValue().syncUninterruptibly().getNow()) { - lockedLocks.add(entry.getKey()); - } - } catch (RuntimeException e) { - latestException = e; - } - } - - if (lockedLocks.size() < minLocksAmount(locks)) { - unlockInner(lockedLocks); - if (latestException != null) { - throw latestException; - } - return false; - } - - return true; - } + @Override + protected int failedLocksLimit() { + return locks.size() - minLocksAmount(locks); + } + protected int minLocksAmount(final List locks) { return locks.size()/2 + 1; } diff --git a/redisson/src/test/java/org/redisson/RedissonRedLockTest.java b/redisson/src/test/java/org/redisson/RedissonRedLockTest.java index bb2d76e44..0e29d6c55 100644 --- a/redisson/src/test/java/org/redisson/RedissonRedLockTest.java +++ b/redisson/src/test/java/org/redisson/RedissonRedLockTest.java @@ -1,8 +1,11 @@ package org.redisson; import java.io.IOException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Assert; import org.junit.Test; @@ -19,6 +22,50 @@ import static org.assertj.core.api.Assertions.assertThat; public class RedissonRedLockTest { + @Test + public void testTryLockLeasetime() throws IOException, InterruptedException { + RedisProcess redis1 = redisTestMultilockInstance(); + RedisProcess redis2 = redisTestMultilockInstance(); + + RedissonClient client1 = createClient(redis1.getRedisServerAddressAndPort()); + RedissonClient client2 = createClient(redis2.getRedisServerAddressAndPort()); + + RLock lock1 = client1.getLock("lock1"); + RLock lock2 = client1.getLock("lock2"); + RLock lock3 = client2.getLock("lock3"); + + RedissonRedLock lock = new RedissonRedLock(lock1, lock2, lock3); + + ExecutorService executor = Executors.newFixedThreadPool(10); + AtomicInteger counter = new AtomicInteger(); + for (int i = 0; i < 10; i++) { + executor.submit(() -> { + for (int j = 0; j < 5; j++) { + try { + if (lock.tryLock(4, 2, TimeUnit.SECONDS)) { + int nextValue = counter.get() + 1; + Thread.sleep(1000); + counter.set(nextValue); + lock.unlock(); + } else { + j--; + } + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + }); + } + + executor.shutdown(); + assertThat(executor.awaitTermination(2, TimeUnit.MINUTES)).isTrue(); + assertThat(counter.get()).isEqualTo(50); + + assertThat(redis1.stop()).isEqualTo(0); + assertThat(redis2.stop()).isEqualTo(0); + } + + @Test public void testLockFailed() throws IOException, InterruptedException { RedisProcess redis1 = redisTestMultilockInstance(); From 8a4a7fdd809f6dfeba0bd0c5d450e03f403ec3ea Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 17:17:22 +0300 Subject: [PATCH 02/10] refactoring --- .../src/main/java/org/redisson/RedissonLock.java | 4 +++- .../java/org/redisson/pubsub/LockPubSub.java | 16 ++++++++++------ .../org/redisson/pubsub/SemaphorePubSub.java | 16 ++++++++++------ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonLock.java b/redisson/src/main/java/org/redisson/RedissonLock.java index 3a9e09a2e..a9003364b 100644 --- a/redisson/src/main/java/org/redisson/RedissonLock.java +++ b/redisson/src/main/java/org/redisson/RedissonLock.java @@ -688,7 +688,9 @@ public class RedissonLock extends RedissonExpirable implements RLock { // lock acquired if (ttl == null) { unsubscribe(subscribeFuture, currentThreadId); - result.trySuccess(true); + if (!result.trySuccess(true)) { + unlockAsync(currentThreadId); + } return; } diff --git a/redisson/src/main/java/org/redisson/pubsub/LockPubSub.java b/redisson/src/main/java/org/redisson/pubsub/LockPubSub.java index cf701665e..e7cdfb6ba 100644 --- a/redisson/src/main/java/org/redisson/pubsub/LockPubSub.java +++ b/redisson/src/main/java/org/redisson/pubsub/LockPubSub.java @@ -32,20 +32,24 @@ public class LockPubSub extends PublishSubscribe { if (message.equals(unlockMessage)) { value.getLatch().release(); - synchronized (value) { - while (true) { + while (true) { + Runnable runnableToExecute = null; + synchronized (value) { Runnable runnable = value.getListeners().poll(); if (runnable != null) { if (value.getLatch().tryAcquire()) { - runnable.run(); + runnableToExecute = runnable; } else { value.addListener(runnable); - return; } - } else { - return; } } + + if (runnableToExecute != null) { + runnableToExecute.run(); + } else { + return; + } } } } diff --git a/redisson/src/main/java/org/redisson/pubsub/SemaphorePubSub.java b/redisson/src/main/java/org/redisson/pubsub/SemaphorePubSub.java index 650aed9f9..85a846b6a 100644 --- a/redisson/src/main/java/org/redisson/pubsub/SemaphorePubSub.java +++ b/redisson/src/main/java/org/redisson/pubsub/SemaphorePubSub.java @@ -29,20 +29,24 @@ public class SemaphorePubSub extends PublishSubscribe { protected void onMessage(RedissonLockEntry value, Long message) { value.getLatch().release(message.intValue()); - synchronized (value) { - while (true) { + while (true) { + Runnable runnableToExecute = null; + synchronized (value) { Runnable runnable = value.getListeners().poll(); if (runnable != null) { if (value.getLatch().tryAcquire()) { - runnable.run(); + runnableToExecute = runnable; } else { value.addListener(runnable); - return; } - } else { - return; } } + + if (runnableToExecute != null) { + runnableToExecute.run(); + } else { + return; + } } } From 2e61fa8c97560c49a750c8e247e7eb494ce22dea Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 17:54:16 +0300 Subject: [PATCH 03/10] BooleanNullSafeReplayConvertor introduced for some operations. --- .../client/protocol/RedisCommands.java | 5 ++-- .../BooleanNullSafeReplayConvertor.java | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 redisson/src/main/java/org/redisson/client/protocol/convertor/BooleanNullSafeReplayConvertor.java diff --git a/redisson/src/main/java/org/redisson/client/protocol/RedisCommands.java b/redisson/src/main/java/org/redisson/client/protocol/RedisCommands.java index 7f46d2a3c..f29885278 100644 --- a/redisson/src/main/java/org/redisson/client/protocol/RedisCommands.java +++ b/redisson/src/main/java/org/redisson/client/protocol/RedisCommands.java @@ -28,6 +28,7 @@ import org.redisson.client.protocol.convertor.BitsSizeReplayConvertor; import org.redisson.client.protocol.convertor.BooleanAmountReplayConvertor; import org.redisson.client.protocol.convertor.BooleanNotNullReplayConvertor; import org.redisson.client.protocol.convertor.BooleanNullReplayConvertor; +import org.redisson.client.protocol.convertor.BooleanNullSafeReplayConvertor; import org.redisson.client.protocol.convertor.BooleanNumberReplayConvertor; import org.redisson.client.protocol.convertor.BooleanReplayConvertor; import org.redisson.client.protocol.convertor.DoubleReplayConvertor; @@ -239,8 +240,8 @@ public interface RedisCommands { RedisStrictCommand DEL = new RedisStrictCommand("DEL"); RedisStrictCommand DBSIZE = new RedisStrictCommand("DBSIZE"); - RedisStrictCommand DEL_BOOL = new RedisStrictCommand("DEL", new BooleanReplayConvertor()); - RedisStrictCommand DEL_OBJECTS = new RedisStrictCommand("DEL", new BooleanAmountReplayConvertor()); + RedisStrictCommand DEL_BOOL = new RedisStrictCommand("DEL", new BooleanNullSafeReplayConvertor()); + RedisStrictCommand DEL_OBJECTS = new RedisStrictCommand("DEL", new BooleanNullSafeReplayConvertor()); RedisStrictCommand DEL_VOID = new RedisStrictCommand("DEL", new VoidReplayConvertor()); RedisCommand GET = new RedisCommand("GET"); diff --git a/redisson/src/main/java/org/redisson/client/protocol/convertor/BooleanNullSafeReplayConvertor.java b/redisson/src/main/java/org/redisson/client/protocol/convertor/BooleanNullSafeReplayConvertor.java new file mode 100644 index 000000000..878bccf11 --- /dev/null +++ b/redisson/src/main/java/org/redisson/client/protocol/convertor/BooleanNullSafeReplayConvertor.java @@ -0,0 +1,29 @@ +/** + * Copyright 2016 Nikita Koksharov + * + * 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.client.protocol.convertor; + +public class BooleanNullSafeReplayConvertor extends SingleConvertor { + + @Override + public Boolean convert(Object obj) { + if (obj == null) { + return false; + } + return Long.valueOf(1).equals(obj) || "OK".equals(obj); + } + + +} From d14b2d1cfcf6eadcceb66eb9bc5e36d2a70962e2 Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 18:19:07 +0300 Subject: [PATCH 04/10] Keep RMap insertion ordering #625 --- .../decoder/ObjectMapEntryReplayDecoder.java | 3 +- .../decoder/ObjectMapReplayDecoder.java | 4 +-- .../decoder/ObjectSetReplayDecoder.java | 3 +- .../java/org/redisson/RedissonMapTest.java | 34 +++++++++++++++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapEntryReplayDecoder.java b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapEntryReplayDecoder.java index a01db6a02..6054602f6 100644 --- a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapEntryReplayDecoder.java +++ b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapEntryReplayDecoder.java @@ -16,6 +16,7 @@ package org.redisson.client.protocol.decoder; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -34,7 +35,7 @@ public class ObjectMapEntryReplayDecoder implements MultiDecoder> decode(List parts, State state) { - Map result = new HashMap(parts.size()/2); + Map result = new LinkedHashMap(parts.size()/2); for (int i = 0; i < parts.size(); i++) { if (i % 2 != 0) { result.put(parts.get(i-1), parts.get(i)); diff --git a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapReplayDecoder.java b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapReplayDecoder.java index 884c72898..4e84eed88 100644 --- a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapReplayDecoder.java +++ b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectMapReplayDecoder.java @@ -15,7 +15,7 @@ */ package org.redisson.client.protocol.decoder; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -32,7 +32,7 @@ public class ObjectMapReplayDecoder implements MultiDecoder> @Override public Map decode(List parts, State state) { - Map result = new HashMap(parts.size()/2); + Map result = new LinkedHashMap(parts.size()/2); for (int i = 0; i < parts.size(); i++) { if (i % 2 != 0) { result.put(parts.get(i-1), parts.get(i)); diff --git a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectSetReplayDecoder.java b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectSetReplayDecoder.java index 3098f11dc..c3aebefdb 100644 --- a/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectSetReplayDecoder.java +++ b/redisson/src/main/java/org/redisson/client/protocol/decoder/ObjectSetReplayDecoder.java @@ -16,6 +16,7 @@ package org.redisson.client.protocol.decoder; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -32,7 +33,7 @@ public class ObjectSetReplayDecoder implements MultiDecoder> { @Override public Set decode(List parts, State state) { - return new HashSet(parts); + return new LinkedHashSet(parts); } @Override diff --git a/redisson/src/test/java/org/redisson/RedissonMapTest.java b/redisson/src/test/java/org/redisson/RedissonMapTest.java index c57434c73..fc1f31284 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapTest.java @@ -4,11 +4,14 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.Serializable; import java.util.AbstractMap; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ConcurrentMap; @@ -345,6 +348,37 @@ public class RedissonMapTest extends BaseTest { assertThat(map.size()).isEqualTo(1); } + @Test + public void testOrdering() { + Map map = new LinkedHashMap(); + + // General player data + map.put("name", "123"); + map.put("ip", "4124"); + map.put("rank", "none"); + map.put("tokens", "0"); + map.put("coins", "0"); + + // Arsenal player statistics + map.put("ar_score", "0"); + map.put("ar_gameswon", "0"); + map.put("ar_gameslost", "0"); + map.put("ar_kills", "0"); + map.put("ar_deaths", "0"); + + RMap rmap = redisson.getMap("123", StringCodec.INSTANCE); + rmap.putAll(map); + + assertThat(rmap.keySet()).containsExactlyElementsOf(map.keySet()); + assertThat(rmap.readAllKeySet()).containsExactlyElementsOf(map.keySet()); + + assertThat(rmap.values()).containsExactlyElementsOf(map.values()); + assertThat(rmap.readAllValues()).containsExactlyElementsOf(map.values()); + + assertThat(rmap.entrySet()).containsExactlyElementsOf(map.entrySet()); + assertThat(rmap.readAllEntrySet()).containsExactlyElementsOf(map.entrySet()); + } + @Test public void testPutAll() { Map map = redisson.getMap("simple"); From 2b5dabd40da1f2107e9369799a76f784d760b31e Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 21:25:10 +0300 Subject: [PATCH 05/10] NPE during RLocalCachedMap.clear invocation --- .../org/redisson/RedissonLocalCachedMap.java | 6 ++++-- .../redisson/RedissonLocalCachedMapTest.java | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonLocalCachedMap.java b/redisson/src/main/java/org/redisson/RedissonLocalCachedMap.java index 6073aecf5..4a0653404 100644 --- a/redisson/src/main/java/org/redisson/RedissonLocalCachedMap.java +++ b/redisson/src/main/java/org/redisson/RedissonLocalCachedMap.java @@ -438,8 +438,10 @@ public class RedissonLocalCachedMap extends RedissonMap implements R byte[] msgEncoded = encode(new LocalCachedMapClear()); return commandExecutor.evalWriteAsync(getName(), LongCodec.INSTANCE, RedisCommands.EVAL_BOOLEAN, "if redis.call('del', KEYS[1]) == 1 and ARGV[2] == '1' then " - + "redis.call('publish', KEYS[2], ARGV[1]); " - + "end; ", + + "redis.call('publish', KEYS[2], ARGV[1]); " + + "return 1;" + + "end; " + + "return 0;", Arrays.asList(getName(), invalidationTopic.getChannelNames().get(0)), msgEncoded, invalidateEntryOnChange); } diff --git a/redisson/src/test/java/org/redisson/RedissonLocalCachedMapTest.java b/redisson/src/test/java/org/redisson/RedissonLocalCachedMapTest.java index d98933d99..78efdfaa1 100644 --- a/redisson/src/test/java/org/redisson/RedissonLocalCachedMapTest.java +++ b/redisson/src/test/java/org/redisson/RedissonLocalCachedMapTest.java @@ -18,6 +18,7 @@ import org.redisson.api.LocalCachedMapOptions; import org.redisson.api.LocalCachedMapOptions.EvictionPolicy; import org.redisson.api.RLocalCachedMap; import org.redisson.api.RMap; +import org.redisson.api.RedissonClient; import org.redisson.misc.Cache; import mockit.Deencapsulation; @@ -46,6 +47,25 @@ public class RedissonLocalCachedMapTest extends BaseTest { } + @Test + public void testClearEmpty() { + RLocalCachedMap localCachedMap = redisson.getLocalCachedMap("udi-test", + LocalCachedMapOptions.defaults()); + + localCachedMap.clear(); + } + + @Test + public void testDelete() { + RLocalCachedMap localCachedMap = redisson.getLocalCachedMap("udi-test", + LocalCachedMapOptions.defaults()); + + assertThat(localCachedMap.delete()).isFalse(); + localCachedMap.put("1", "2"); + assertThat(localCachedMap.delete()).isTrue(); + } + + @Test public void testInvalidationOnUpdate() throws InterruptedException { LocalCachedMapOptions options = LocalCachedMapOptions.defaults().evictionPolicy(EvictionPolicy.LFU).cacheSize(5).invalidateEntryOnChange(true); From ddc7ad25a3671ae8a2a943053f8b272b31400909 Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 20 Sep 2016 21:25:36 +0300 Subject: [PATCH 06/10] testOrdering fixed --- redisson/src/test/java/org/redisson/RedissonMapTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisson/src/test/java/org/redisson/RedissonMapTest.java b/redisson/src/test/java/org/redisson/RedissonMapTest.java index fc1f31284..c0ff54004 100644 --- a/redisson/src/test/java/org/redisson/RedissonMapTest.java +++ b/redisson/src/test/java/org/redisson/RedissonMapTest.java @@ -366,7 +366,7 @@ public class RedissonMapTest extends BaseTest { map.put("ar_kills", "0"); map.put("ar_deaths", "0"); - RMap rmap = redisson.getMap("123", StringCodec.INSTANCE); + RMap rmap = redisson.getMap("123"); rmap.putAll(map); assertThat(rmap.keySet()).containsExactlyElementsOf(map.keySet()); From 382ad73c11504df7fcc205d8d38c431665e524c1 Mon Sep 17 00:00:00 2001 From: jackygurui Date: Wed, 21 Sep 2016 00:47:23 +0100 Subject: [PATCH 07/10] Fixed testing with travisEnv flag turned off --- .../executor/RedissonExecutorServiceTest.java | 22 ++++++++----------- .../RedissonScheduledExecutorServiceTest.java | 20 +++++++---------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java b/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java index 39e32cfd4..beab257f5 100644 --- a/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java +++ b/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java @@ -47,23 +47,19 @@ public class RedissonExecutorServiceTest extends BaseTest { @AfterClass public static void afterClass() throws IOException, InterruptedException { - if (!RedissonRuntimeEnvironment.isTravis) { - BaseTest.afterClass(); - node.shutdown(); - } + BaseTest.afterClass(); + node.shutdown(); } - + @Before @Override public void before() throws IOException, InterruptedException { - if (RedissonRuntimeEnvironment.isTravis) { - super.before(); - Config config = createConfig(); - RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); - nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); - node = RedissonNode.create(nodeConfig); - node.start(); - } + super.before(); + Config config = createConfig(); + RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); + nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); + node = RedissonNode.create(nodeConfig); + node.start(); } @After diff --git a/redisson/src/test/java/org/redisson/executor/RedissonScheduledExecutorServiceTest.java b/redisson/src/test/java/org/redisson/executor/RedissonScheduledExecutorServiceTest.java index f6cac1fa0..352ea5d1d 100644 --- a/redisson/src/test/java/org/redisson/executor/RedissonScheduledExecutorServiceTest.java +++ b/redisson/src/test/java/org/redisson/executor/RedissonScheduledExecutorServiceTest.java @@ -50,23 +50,19 @@ public class RedissonScheduledExecutorServiceTest extends BaseTest { @Before @Override public void before() throws IOException, InterruptedException { - if (RedissonRuntimeEnvironment.isTravis) { - super.before(); - Config config = createConfig(); - RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); - nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); - node = RedissonNode.create(nodeConfig); - node.start(); - } + super.before(); + Config config = createConfig(); + RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); + nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); + node = RedissonNode.create(nodeConfig); + node.start(); } @After @Override public void after() throws InterruptedException { - if (RedissonRuntimeEnvironment.isTravis) { - super.after(); - node.shutdown(); - } + super.after(); + node.shutdown(); } @Test From da28d25a9f7a3b7cc4004f59abf566add7995174 Mon Sep 17 00:00:00 2001 From: Nikita Date: Wed, 21 Sep 2016 12:54:26 +0300 Subject: [PATCH 08/10] test fixed --- .../executor/RedissonExecutorServiceTest.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java b/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java index beab257f5..0aa86d6db 100644 --- a/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java +++ b/redisson/src/test/java/org/redisson/executor/RedissonExecutorServiceTest.java @@ -35,8 +35,8 @@ public class RedissonExecutorServiceTest extends BaseTest { @BeforeClass public static void beforeClass() throws IOException, InterruptedException { + BaseTest.beforeClass(); if (!RedissonRuntimeEnvironment.isTravis) { - BaseTest.beforeClass(); Config config = createConfig(); RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); @@ -48,25 +48,29 @@ public class RedissonExecutorServiceTest extends BaseTest { @AfterClass public static void afterClass() throws IOException, InterruptedException { BaseTest.afterClass(); - node.shutdown(); + if (!RedissonRuntimeEnvironment.isTravis) { + node.shutdown(); + } } @Before @Override public void before() throws IOException, InterruptedException { super.before(); - Config config = createConfig(); - RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); - nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); - node = RedissonNode.create(nodeConfig); - node.start(); + if (RedissonRuntimeEnvironment.isTravis) { + Config config = createConfig(); + RedissonNodeConfig nodeConfig = new RedissonNodeConfig(config); + nodeConfig.setExecutorServiceWorkers(Collections.singletonMap("test", 1)); + node = RedissonNode.create(nodeConfig); + node.start(); + } } @After @Override public void after() throws InterruptedException { + super.after(); if (RedissonRuntimeEnvironment.isTravis) { - super.after(); node.shutdown(); } } From 5091f3d30c8525c734d85b07f8a380176adfda73 Mon Sep 17 00:00:00 2001 From: Nikita Date: Wed, 21 Sep 2016 12:55:48 +0300 Subject: [PATCH 09/10] RedLock and MultiLock.tryLock failed locks count fixed --- redisson/src/main/java/org/redisson/RedissonMultiLock.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonMultiLock.java b/redisson/src/main/java/org/redisson/RedissonMultiLock.java index 8d4e7abfc..e77f7df3f 100644 --- a/redisson/src/main/java/org/redisson/RedissonMultiLock.java +++ b/redisson/src/main/java/org/redisson/RedissonMultiLock.java @@ -221,7 +221,7 @@ public class RedissonMultiLock implements Lock { } protected int failedLocksLimit() { - return 1; + return 0; } public boolean tryLock(long waitTime, long leaseTime, TimeUnit unit) throws InterruptedException { @@ -249,7 +249,6 @@ public class RedissonMultiLock implements Lock { if (lockAcquired) { lockedLocks.add(lock); } else { - failedLocksLimit--; if (failedLocksLimit == 0) { unlockInner(lockedLocks); if (waitTime == -1 && leaseTime == -1) { @@ -257,6 +256,8 @@ public class RedissonMultiLock implements Lock { } failedLocksLimit = failedLocksLimit(); lockedLocks.clear(); + } else { + failedLocksLimit--; } } From 7fd29a2b49b650ec5004295b27992f036f39a5f5 Mon Sep 17 00:00:00 2001 From: Nikita Date: Thu, 22 Sep 2016 13:36:04 +0300 Subject: [PATCH 10/10] `Not all slots are covered` error should be more informative --- .../cluster/ClusterConnectionManager.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java b/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java index 9f2a497a0..c292a153b 100644 --- a/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java +++ b/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java @@ -76,7 +76,8 @@ public class ClusterConnectionManager extends MasterSlaveConnectionManager { this.config = create(cfg); init(this.config); - Exception lastException = null; + Throwable lastException = null; + List failedMasters = new ArrayList(); for (URI addr : cfg.getNodeAddresses()) { RFuture connectionFuture = connect(cfg, addr); try { @@ -97,6 +98,7 @@ public class ClusterConnectionManager extends MasterSlaveConnectionManager { List>>> futures = new ArrayList>>>(); for (ClusterPartition partition : partitions) { if (partition.isMasterFail()) { + failedMasters.add(partition.getMasterAddr().toString()); continue; } RFuture>> masterFuture = addMasterEntry(partition, cfg); @@ -106,6 +108,7 @@ public class ClusterConnectionManager extends MasterSlaveConnectionManager { for (RFuture>> masterFuture : futures) { masterFuture.awaitUninterruptibly(); if (!masterFuture.isSuccess()) { + lastException = masterFuture.cause(); continue; } for (RFuture future : masterFuture.getNow()) { @@ -124,12 +127,20 @@ public class ClusterConnectionManager extends MasterSlaveConnectionManager { if (lastPartitions.isEmpty()) { stopThreads(); - throw new RedisConnectionException("Can't connect to servers!", lastException); + if (failedMasters.isEmpty()) { + throw new RedisConnectionException("Can't connect to servers!", lastException); + } else { + throw new RedisConnectionException("Can't connect to servers! Failed masters according to cluster status: " + failedMasters, lastException); + } } if (lastPartitions.size() != MAX_SLOT) { stopThreads(); - throw new RedisConnectionException("Not all slots are covered! Only " + lastPartitions.size() + " slots are avaliable", lastException); + if (failedMasters.isEmpty()) { + throw new RedisConnectionException("Not all slots are covered! Only " + lastPartitions.size() + " slots are avaliable", lastException); + } else { + throw new RedisConnectionException("Not all slots are covered! Only " + lastPartitions.size() + " slots are avaliable. Failed masters according to cluster status: " + failedMasters, lastException); + } } scheduleClusterChangeCheck(cfg, null);