INT-3846: SimpleMessageStore Improvements
JIRAs: https://jira.spring.io/browse/INT-3830 https://jira.spring.io/browse/INT-3523 https://jira.spring.io/browse/INT-3846 * Fix the OOM condition, when we `release()` `UpperBound` independently of the previous `remove` result (https://jira.spring.io/browse/INT-3846) * Fix "confuse" around `groupCapacity`, when we really didn't care about individual groups (https://jira.spring.io/browse/INT-3523) * Add `upperBoundTimeout` to have a hook to wait some time for the empty slot in the store (https://jira.spring.io/browse/INT-3830) * Fix some JavaDocs warnings * Fix some typos * Fix inconsistency in the `DelayHandler` around `removeMessageFromGroup` when `MS` is `SimpleMessageStore` * Remove `UpperBound.release()` operation from `SimpleMessageStore.removeGroup()`. The waiting process should worry about the new `UpperBound` instance. Some other polishing `tryAcquire` outside of the `lock` Move `tryAcquire` within the `addMessageToGroup` outside of `lock`. But do that only for groups which already exist. For the new groups we have a fresh `UpperBound`, so no need to worry about dead lock and we can obtain a permit immediately. SimpleMessageGroup: BlockingQueue -> LinkedHashSet `SimpleMessageStore`: use "unsynchonized" `SimpleMessageGroup` Make some synchronization fixes according to the migration to the `LinkedHashSet` Avoid extra `Collection` `ResequencingMessageHandler`: compare `size()` of collections instead of `containsAll()` Fix `ConcurrentModificationException` in the `AbstractKeyValueMessageStore` Add `SimpleMessageStore.clearMessageGroup()` Accept polishing and fix `RedisChannelMessageStoreTests` `@Deprecated` `MessageGroupStore.removeMessageFromGroup()` Fix some typos Introduce `SimpleMessageGroupFactory` Extract `MessageGroupFactory` and address PR comments Polishing after rebase JavaDocs and Reference Manual Fix JavaDocs
This commit is contained in:
committed by
Gary Russell
parent
5d5fa86e6e
commit
201f0fc2b1
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2014 the original author or authors.
|
||||
* Copyright 2014-2016 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.
|
||||
@@ -13,6 +13,7 @@
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.integration.redis.store;
|
||||
|
||||
import java.util.List;
|
||||
@@ -27,7 +28,8 @@ import org.springframework.data.redis.serializer.RedisSerializer;
|
||||
import org.springframework.data.redis.serializer.StringRedisSerializer;
|
||||
import org.springframework.integration.store.ChannelMessageStore;
|
||||
import org.springframework.integration.store.MessageGroup;
|
||||
import org.springframework.integration.store.SimpleMessageGroup;
|
||||
import org.springframework.integration.store.MessageGroupFactory;
|
||||
import org.springframework.integration.store.SimpleMessageGroupFactory;
|
||||
import org.springframework.jmx.export.annotation.ManagedAttribute;
|
||||
import org.springframework.messaging.Message;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -38,6 +40,7 @@ import org.springframework.util.Assert;
|
||||
* Requires {@link #setBeanName(String)} which is used as part of the key.
|
||||
*
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
* @since 4.0
|
||||
*
|
||||
*/
|
||||
@@ -45,6 +48,8 @@ public class RedisChannelMessageStore implements ChannelMessageStore, BeanNameAw
|
||||
|
||||
private final RedisTemplate<Object, Message<?>> redisTemplate;
|
||||
|
||||
private volatile MessageGroupFactory messageGroupFactory = new SimpleMessageGroupFactory();
|
||||
|
||||
private String beanName;
|
||||
|
||||
/**
|
||||
@@ -71,6 +76,22 @@ public class RedisChannelMessageStore implements ChannelMessageStore, BeanNameAw
|
||||
this.redisTemplate.setValueSerializer(valueSerializer);
|
||||
}
|
||||
|
||||
/**
|
||||
* Specify the {@link MessageGroupFactory} to create {@link MessageGroup} object where
|
||||
* it is necessary.
|
||||
* Defaults to {@link SimpleMessageGroupFactory}.
|
||||
* @param messageGroupFactory the {@link MessageGroupFactory} to use.
|
||||
* @since 4.3
|
||||
*/
|
||||
public void setMessageGroupFactory(MessageGroupFactory messageGroupFactory) {
|
||||
Assert.notNull(messageGroupFactory, "'messageGroupFactory' must not be null");
|
||||
this.messageGroupFactory = messageGroupFactory;
|
||||
}
|
||||
|
||||
protected MessageGroupFactory getMessageGroupFactory() {
|
||||
return this.messageGroupFactory;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setBeanName(String name) {
|
||||
Assert.notNull(name, "'beanName' must not be null");
|
||||
@@ -78,11 +99,11 @@ public class RedisChannelMessageStore implements ChannelMessageStore, BeanNameAw
|
||||
}
|
||||
|
||||
protected String getBeanName() {
|
||||
return beanName;
|
||||
return this.beanName;
|
||||
}
|
||||
|
||||
protected RedisTemplate<Object, Message<?>> getRedisTemplate() {
|
||||
return redisTemplate;
|
||||
return this.redisTemplate;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -99,7 +120,7 @@ public class RedisChannelMessageStore implements ChannelMessageStore, BeanNameAw
|
||||
@Override
|
||||
public MessageGroup getMessageGroup(Object groupId) {
|
||||
List<Message<?>> messages = this.redisTemplate.boundListOps(groupId).range(0, -1);
|
||||
return new SimpleMessageGroup(messages, groupId);
|
||||
return getMessageGroupFactory().create(messages, groupId);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2014 the original author or authors.
|
||||
* Copyright 2014-2016 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.
|
||||
@@ -27,7 +27,6 @@ import org.springframework.data.redis.connection.RedisConnectionFactory;
|
||||
import org.springframework.integration.IntegrationMessageHeaderAccessor;
|
||||
import org.springframework.integration.store.MessageGroup;
|
||||
import org.springframework.integration.store.PriorityCapableChannelMessageStore;
|
||||
import org.springframework.integration.store.SimpleMessageGroup;
|
||||
import org.springframework.jmx.export.annotation.ManagedAttribute;
|
||||
import org.springframework.messaging.Message;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -41,10 +40,12 @@ import org.springframework.util.Assert;
|
||||
* Requires that groupId is a String.
|
||||
*
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
* @since 4.0
|
||||
*
|
||||
*/
|
||||
public class RedisChannelPriorityMessageStore extends RedisChannelMessageStore implements PriorityCapableChannelMessageStore {
|
||||
public class RedisChannelPriorityMessageStore extends RedisChannelMessageStore
|
||||
implements PriorityCapableChannelMessageStore {
|
||||
|
||||
private final Comparator<String> keysComparator = new Comparator<String>() {
|
||||
|
||||
@@ -85,7 +86,7 @@ public class RedisChannelPriorityMessageStore extends RedisChannelMessageStore i
|
||||
List<Message<?>> messages = this.getRedisTemplate().boundListOps(key).range(0, -1);
|
||||
allMessages.addAll(messages);
|
||||
}
|
||||
return new SimpleMessageGroup(allMessages, groupId);
|
||||
return getMessageGroupFactory().create(allMessages, groupId);
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2014 the original author or authors.
|
||||
* Copyright 2014-2016 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.
|
||||
@@ -13,6 +13,7 @@
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.integration.redis.store;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
@@ -118,9 +119,9 @@ public class RedisChannelMessageStoreTests extends RedisAvailableTests {
|
||||
@RedisAvailable
|
||||
public void testPriority() {
|
||||
for (int i = 0; i < 10; i++) {
|
||||
Message<Integer> message = MessageBuilder.withPayload(i).setPriority(i).build();
|
||||
this.testChannel3.send(message);
|
||||
this.testChannel3.send(message);
|
||||
this.testChannel3.send(MessageBuilder.withPayload(i).setPriority(i).build());
|
||||
//We need unique messages
|
||||
this.testChannel3.send(MessageBuilder.withPayload(i).setPriority(i).build());
|
||||
}
|
||||
this.testChannel3.send(MessageBuilder.withPayload(99).setPriority(199).build());
|
||||
this.testChannel3.send(MessageBuilder.withPayload(98).build());
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2007-2015 the original author or authors
|
||||
* Copyright 2007-2016 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.
|
||||
@@ -13,6 +13,7 @@
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.integration.redis.store;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
@@ -29,7 +30,6 @@ import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import junit.framework.AssertionFailedError;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Ignore;
|
||||
@@ -50,11 +50,12 @@ import org.springframework.messaging.Message;
|
||||
import org.springframework.messaging.MessageChannel;
|
||||
import org.springframework.messaging.support.GenericMessage;
|
||||
|
||||
import junit.framework.AssertionFailedError;
|
||||
|
||||
/**
|
||||
* @author Oleg Zhurakousky
|
||||
* @author Artem Bilan
|
||||
* @author Gary Russell
|
||||
*
|
||||
*/
|
||||
public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
|
||||
@@ -190,7 +191,8 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
messageGroup = store.addMessageToGroup(messageGroup.getGroupId(), new GenericMessage<String>("3"));
|
||||
assertEquals(3, messageGroup.size());
|
||||
|
||||
messageGroup = store.removeMessageFromGroup(1, message);
|
||||
store.removeMessagesFromGroup(1, message);
|
||||
messageGroup = store.getMessageGroup(1);
|
||||
assertEquals(2, messageGroup.size());
|
||||
|
||||
// make sure the store is properly rebuild from Redis
|
||||
@@ -235,7 +237,7 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
|
||||
MessageGroup messageGroup = store.getMessageGroup(1);
|
||||
store.addMessageToGroup(messageGroup.getGroupId(), new GenericMessage<String>("1"));
|
||||
store.removeMessageFromGroup(1, new GenericMessage<String>("2"));
|
||||
store.removeMessagesFromGroup(1, new GenericMessage<String>("2"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -243,7 +245,7 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
public void testRemoveNonExistingMessageFromNonExistingTheGroup() throws Exception{
|
||||
RedisConnectionFactory jcf = getConnectionFactoryForTest();
|
||||
RedisMessageStore store = new RedisMessageStore(jcf);
|
||||
store.removeMessageFromGroup(1, new GenericMessage<String>("2"));
|
||||
store.removeMessagesFromGroup(1, new GenericMessage<String>("2"));
|
||||
}
|
||||
|
||||
|
||||
@@ -264,7 +266,8 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
|
||||
RedisMessageStore store3 = new RedisMessageStore(jcf);
|
||||
|
||||
messageGroup = store3.removeMessageFromGroup(1, message);
|
||||
store3.removeMessagesFromGroup(1, message);
|
||||
messageGroup = store3.getMessageGroup(1);
|
||||
|
||||
assertEquals(1, messageGroup.getMessages().size());
|
||||
}
|
||||
@@ -340,7 +343,8 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
executor.execute(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
MessageGroup group = store2.removeMessageFromGroup(1, message);
|
||||
store2.removeMessagesFromGroup(1, message);
|
||||
MessageGroup group = store2.getMessageGroup(1);
|
||||
if (group.getMessages().size() != 0){
|
||||
failures.add("REMOVE");
|
||||
throw new AssertionFailedError("Failed on Remove");
|
||||
@@ -350,7 +354,7 @@ public class RedisMessageGroupStoreTests extends RedisAvailableTests {
|
||||
|
||||
executor.shutdown();
|
||||
executor.awaitTermination(10, TimeUnit.SECONDS);
|
||||
store2.removeMessageFromGroup(1, message); // ensures that if ADD thread executed after REMOVE, the store is empty for the next cycle
|
||||
store2.removeMessagesFromGroup(1, message); // ensures that if ADD thread executed after REMOVE, the store is empty for the next cycle
|
||||
}
|
||||
assertTrue(failures.size() == 0);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user