Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

INT-4248: Refactor RedisLockRegistry

Browse Source 
JIRA: https://jira.spring.io/browse/INT-4248

To avoid unexpected double locking behavior in the cluster,
remove the local cache functionality.

Now with the new `clientId` property, the `expire` for the record in
store is always update on each lock operation
This commit is contained in:
Vedran Pavic
2017-03-31 23:47:27 +02:00
committed by Artem Bilan
parent 28058884f4
commit f7f7bdd067
4 changed files with 347 additions and 532 deletions
Download Patch File Download Diff File Expand all files Collapse all files

175
spring-integration-redis/src/test/java/org/springframework/integration/redis/leader/RedisLockRegistryLeaderInitiatorTests.java Normal file
View File

@@ -0,0 +1,175 @@
/*
* Copyright 2016-2017 the original author or authors.
*
* 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.springframework.integration.redis.leader;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.springframework.integration.leader.Context;
import org.springframework.integration.leader.DefaultCandidate;
import org.springframework.integration.leader.event.LeaderEventPublisher;
import org.springframework.integration.redis.rules.RedisAvailable;
import org.springframework.integration.redis.rules.RedisAvailableTests;
import org.springframework.integration.redis.util.RedisLockRegistry;
import org.springframework.integration.support.leader.LockRegistryLeaderInitiator;
/**
* @author Artem Bilan
* @author Gary Russell
*
* @since 4.3.9
*/
public class RedisLockRegistryLeaderInitiatorTests extends RedisAvailableTests {
@Test
@RedisAvailable
public void testDistributedLeaderElection() throws Exception {
CountDownLatch granted = new CountDownLatch(1);
CountingPublisher countingPublisher = new CountingPublisher(granted);
List<LockRegistryLeaderInitiator> initiators = new ArrayList<>();
for (int i = 0; i < 2; i++) {
RedisLockRegistry registry = new RedisLockRegistry(getConnectionFactoryForTest(), "LeaderInitiator");
LockRegistryLeaderInitiator initiator =
new LockRegistryLeaderInitiator(registry, new DefaultCandidate("foo", "bar"));
initiator.setLeaderEventPublisher(countingPublisher);
initiators.add(initiator);
}
for (LockRegistryLeaderInitiator initiator : initiators) {
initiator.start();
}
assertThat(granted.await(10, TimeUnit.SECONDS), is(true));
LockRegistryLeaderInitiator initiator1 = countingPublisher.initiator;
LockRegistryLeaderInitiator initiator2 = null;
for (LockRegistryLeaderInitiator initiator : initiators) {
if (initiator != initiator1) {
initiator2 = initiator;
break;
}
}
assertNotNull(initiator2);
assertThat(initiator1.getContext().isLeader(), is(true));
assertThat(initiator2.getContext().isLeader(), is(false));
final CountDownLatch granted1 = new CountDownLatch(1);
final CountDownLatch granted2 = new CountDownLatch(1);
CountDownLatch revoked1 = new CountDownLatch(1);
CountDownLatch revoked2 = new CountDownLatch(1);
initiator1.setLeaderEventPublisher(new CountingPublisher(granted1, revoked1) {
@Override
public void publishOnRevoked(Object source, Context context, String role) {
try {
// It's difficult to see round-robin election, so block one initiator until the second is elected.
assertThat(granted2.await(10, TimeUnit.SECONDS), is(true));
}
catch (InterruptedException e) {
// No op
}
super.publishOnRevoked(source, context, role);
}
});
initiator2.setLeaderEventPublisher(new CountingPublisher(granted2, revoked2) {
@Override
public void publishOnRevoked(Object source, Context context, String role) {
try {
// It's difficult to see round-robin election, so block one initiator until the second is elected.
assertThat(granted1.await(10, TimeUnit.SECONDS), is(true));
}
catch (InterruptedException e) {
// No op
}
super.publishOnRevoked(source, context, role);
}
});
initiator1.getContext().yield();
assertThat(revoked1.await(10, TimeUnit.SECONDS), is(true));
assertThat(initiator2.getContext().isLeader(), is(true));
assertThat(initiator1.getContext().isLeader(), is(false));
initiator2.getContext().yield();
assertThat(revoked2.await(10, TimeUnit.SECONDS), is(true));
assertThat(initiator1.getContext().isLeader(), is(true));
assertThat(initiator2.getContext().isLeader(), is(false));
initiator2.stop();
CountDownLatch revoked11 = new CountDownLatch(1);
initiator1.setLeaderEventPublisher(new CountingPublisher(new CountDownLatch(1), revoked11));
initiator1.getContext().yield();
assertThat(revoked11.await(10, TimeUnit.SECONDS), is(true));
assertThat(initiator1.getContext().isLeader(), is(false));
initiator1.stop();
}
private static class CountingPublisher implements LeaderEventPublisher {
private CountDownLatch granted;
private CountDownLatch revoked;
private volatile LockRegistryLeaderInitiator initiator;
CountingPublisher(CountDownLatch granted, CountDownLatch revoked) {
this.granted = granted;
this.revoked = revoked;
}
CountingPublisher(CountDownLatch granted) {
this(granted, new CountDownLatch(1));
}
@Override
public void publishOnRevoked(Object source, Context context, String role) {
this.revoked.countDown();
}
@Override
public void publishOnGranted(Object source, Context context, String role) {
this.initiator = (LockRegistryLeaderInitiator) source;
this.granted.countDown();
}
}
}

170
spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -21,15 +21,12 @@ import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.util.Collection;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;
@@ -45,10 +42,10 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.serializer.StringRedisSerializer;
import org.springframework.data.redis.core.StringRedisTemplate;
import org.springframework.integration.redis.rules.RedisAvailable;
import org.springframework.integration.redis.rules.RedisAvailableTests;
import org.springframework.integration.test.rule.Log4jLevelAdjuster;
@@ -58,6 +55,7 @@ import org.springframework.integration.test.util.TestUtils;
* @author Gary Russell
* @author Konstantin Yakimov
* @author Artem Bilan
* @author Vedran Pavic
* @since 4.0
*
*/
@@ -72,20 +70,19 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
@Rule
public Log4jLevelAdjuster adjuster = new Log4jLevelAdjuster(Level.TRACE, "org.springframework.integration.redis");
@Rule
public ExpectedException thrown = ExpectedException.none();
@Before
@After
public void setupShutDown() {
RedisTemplate<String, ?> template = this.createTemplate();
StringRedisTemplate template = this.createTemplate();
template.delete(this.registryKey + ":*");
template.delete(this.registryKey2 + ":*");
}
private RedisTemplate<String, ?> createTemplate() {
RedisTemplate<String, ?> template = new RedisTemplate<>();
template.setConnectionFactory(this.getConnectionFactoryForTest());
template.setKeySerializer(new StringRedisSerializer());
template.afterPropertiesSet();
return template;
private StringRedisTemplate createTemplate() {
return new StringRedisTemplate(this.getConnectionFactoryForTest());
}
@Test
@@ -96,13 +93,14 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
Lock lock = registry.obtain("foo");
lock.lock();
try {
assertNotNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
finally {
lock.unlock();
}
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -113,13 +111,14 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
Lock lock = registry.obtain("foo");
lock.lockInterruptibly();
try {
assertNotNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
finally {
lock.unlock();
}
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -144,7 +143,8 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
lock1.unlock();
}
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -169,7 +169,8 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
lock1.unlock();
}
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -194,7 +195,8 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
lock1.unlock();
}
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -222,8 +224,9 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
lock1.unlock();
Object ise = result.get(10, TimeUnit.SECONDS);
assertThat(ise, instanceOf(IllegalStateException.class));
assertThat(((Exception) ise).getMessage(), containsString("Lock is not locked"));
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertThat(((Exception) ise).getMessage(), containsString("You do not own lock at"));
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -236,13 +239,13 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
final CountDownLatch latch2 = new CountDownLatch(1);
final CountDownLatch latch3 = new CountDownLatch(1);
lock1.lockInterruptibly();
assertNotNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
Executors.newSingleThreadExecutor().execute(() -> {
Lock lock2 = registry.obtain("foo");
try {
latch1.countDown();
lock2.lockInterruptibly();
assertNotNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
latch2.await(10, TimeUnit.SECONDS);
locked.set(true);
}
@@ -260,7 +263,8 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
latch2.countDown();
assertTrue(latch3.await(10, TimeUnit.SECONDS));
assertTrue(locked.get());
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -274,19 +278,19 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
final CountDownLatch latch2 = new CountDownLatch(1);
final CountDownLatch latch3 = new CountDownLatch(1);
lock1.lockInterruptibly();
assertNotNull(TestUtils.getPropertyValue(registry1, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry1, "locks", Map.class).size());
Executors.newSingleThreadExecutor().execute(() -> {
Lock lock2 = registry2.obtain("foo");
try {
latch1.countDown();
lock2.lockInterruptibly();
assertNotNull(TestUtils.getPropertyValue(registry2, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(1, TestUtils.getPropertyValue(registry2, "locks", Map.class).size());
latch2.await(10, TimeUnit.SECONDS);
locked.set(true);
}
catch (InterruptedException e1) {
Thread.currentThread().interrupt();
logger.error("Interrupted while locking: " + lock2, e1);
this.logger.error("Interrupted while locking: " + lock2, e1);
}
finally {
try {
@@ -294,7 +298,7 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
latch3.countDown();
}
catch (IllegalStateException e2) {
logger.error("Failed to unlock: " + lock2, e2);
this.logger.error("Failed to unlock: " + lock2, e2);
}
}
});
@@ -304,8 +308,10 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
latch2.countDown();
assertTrue(latch3.await(10, TimeUnit.SECONDS));
assertTrue(locked.get());
assertNull(TestUtils.getPropertyValue(registry1, "hardThreadLocks", ThreadLocal.class).get());
assertNull(TestUtils.getPropertyValue(registry2, "hardThreadLocks", ThreadLocal.class).get());
registry1.expireUnusedOlderThan(-1000);
registry2.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry1, "locks", Map.class).size());
assertEquals(0, TestUtils.getPropertyValue(registry2, "locks", Map.class).size());
}
@Test
@@ -331,84 +337,23 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
lock.unlock();
Object ise = result.get(10, TimeUnit.SECONDS);
assertThat(ise, instanceOf(IllegalStateException.class));
assertThat(((Exception) ise).getMessage(), containsString("Lock is owned by"));
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertThat(((Exception) ise).getMessage(), containsString("You do not own lock at"));
registry.expireUnusedOlderThan(-1000);
assertEquals(0, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@RedisAvailable
public void testList() throws Exception {
RedisLockRegistry registry = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey);
Lock foo = registry.obtain("foo");
foo.lockInterruptibly();
Lock bar = registry.obtain("bar");
bar.lockInterruptibly();
Lock baz = registry.obtain("baz");
baz.lockInterruptibly();
Collection<Lock> locks = registry.listLocks();
assertEquals(3, locks.size());
foo.unlock();
bar.unlock();
baz.unlock();
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
}
@Test
@RedisAvailable
public void testExpireNoLockInStore() throws Exception {
RedisLockRegistry registry = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
Lock foo = registry.obtain("foo");
foo.lockInterruptibly();
public void testExpireTwoRegistries() throws Exception {
RedisLockRegistry registry1 = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
RedisLockRegistry registry2 = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
Lock lock1 = registry1.obtain("foo");
Lock lock2 = registry2.obtain("foo");
assertTrue(lock1.tryLock());
assertFalse(lock2.tryLock());
waitForExpire("foo");
try {
foo.unlock();
fail("Expected exception");
}
catch (IllegalStateException e) {
assertThat(e.getMessage(), containsString("Lock was released due to expiration"));
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
}
@Test
@RedisAvailable
public void testExpireDuringSecondObtain() throws Exception {
RedisLockRegistry registry = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
registry.setUseWeakReferences(true);
Lock foo = registry.obtain("foo");
foo.lockInterruptibly();
waitForExpire("foo");
Lock foo1 = registry.obtain("foo");
assertNotSame(foo, foo1);
try {
foo.unlock();
fail("IllegalStateException");
}
catch (IllegalStateException e) {
assertThat(e.getMessage(), containsString("Lock is not locked"));
}
}
@Test
@RedisAvailable
public void testExpireNewLockInStore() throws Exception {
RedisLockRegistry registry = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
Lock foo1 = registry.obtain("foo");
foo1.lockInterruptibly();
waitForExpire("foo");
Lock foo2 = registry.obtain("foo");
assertNotSame(foo1, foo2);
foo2.lockInterruptibly();
try {
foo1.unlock();
fail("Expected exception");
}
catch (IllegalStateException e) {
assertThat(e.getMessage(), containsString("Lock is not locked"));
}
foo2.unlock();
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertTrue(lock2.tryLock());
assertFalse(lock1.tryLock());
}
@Test
@@ -416,7 +361,6 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
public void testEquals() throws Exception {
RedisConnectionFactory connectionFactory = this.getConnectionFactoryForTest();
RedisLockRegistry registry1 = new RedisLockRegistry(connectionFactory, this.registryKey);
registry1.setUseWeakReferences(true);
RedisLockRegistry registry2 = new RedisLockRegistry(connectionFactory, this.registryKey);
RedisLockRegistry registry3 = new RedisLockRegistry(connectionFactory, this.registryKey2);
Lock lock1 = registry1.obtain("foo");
@@ -449,27 +393,23 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
@RedisAvailable
public void testThreadLocalListLeaks() {
RedisLockRegistry registry = new RedisLockRegistry(this.getConnectionFactoryForTest(), this.registryKey, 100);
registry.setUseWeakReferences(true);
for (int i = 0; i < 10; i++) {
registry.obtain("foo" + i);
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(10, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
for (int i = 0; i < 10; i++) {
Lock lock = registry.obtain("foo" + i);
lock.lock();
}
assertEquals(10,
((Collection<?>) TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get()).size());
assertNull(TestUtils.getPropertyValue(registry, "weakThreadLocks", ThreadLocal.class).get());
assertEquals(10, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
for (int i = 0; i < 10; i++) {
Lock lock = registry.obtain("foo" + i);
assertNotNull(TestUtils.getPropertyValue(lock, "thread", Thread.class));
lock.unlock();
}
assertNull(TestUtils.getPropertyValue(registry, "hardThreadLocks", ThreadLocal.class).get());
assertEquals(10, TestUtils.getPropertyValue(registry, "locks", Map.class).size());
}
@Test
@@ -493,13 +433,13 @@ public class RedisLockRegistryTests extends RedisAvailableTests {
}
private Long getExpire(RedisLockRegistry registry, String lockKey) {
RedisTemplate<String, ?> template = this.createTemplate();
StringRedisTemplate template = this.createTemplate();
String registryKey = TestUtils.getPropertyValue(registry, "registryKey", String.class);
return template.getExpire(registryKey + ":" + lockKey);
}
private void waitForExpire(String key) throws Exception {
RedisTemplate<String, ?> template = this.createTemplate();
StringRedisTemplate template = this.createTemplate();
int n = 0;
while (n++ < 100 && template.keys(this.registryKey + ":" + key).size() > 0) {
Thread.sleep(100);