From 6c384c9b79aeb16c48ce98bb04cd99b2939e64d0 Mon Sep 17 00:00:00 2001 From: gtully Date: Fri, 3 Apr 2015 14:18:55 +0100 Subject: [PATCH] https://issues.apache.org/jira/browse/AMQ-5674 - revisit change - use single redelivey delay attribute for both initial and subsequent delays - resolves regression in BrokerRedeliveryTest --- .../activemq/broker/util/RedeliveryPlugin.java | 14 ++++---------- .../java/org/apache/activemq/RedeliveryPolicy.java | 5 +---- .../apache/activemq/ra/ActiveMQActivationSpec.java | 1 + .../org/apache/activemq/RedeliveryPolicyTest.java | 6 +++--- .../activemq/broker/BrokerRedeliveryTest.java | 2 +- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java b/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java index 5a9075301a..f270744d16 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java @@ -140,9 +140,10 @@ public class RedeliveryPlugin extends BrokerPluginSupport { int redeliveryCount = messageReference.getRedeliveryCounter(); if (RedeliveryPolicy.NO_MAXIMUM_REDELIVERIES == maximumRedeliveries || redeliveryCount < maximumRedeliveries) { - long delay = ( redeliveryCount == 0 ? - redeliveryPolicy.getInitialRedeliveryDelay() : - redeliveryPolicy.getNextRedeliveryDelay(getExistingDelay(messageReference))); + long delay = redeliveryPolicy.getInitialRedeliveryDelay(); + for (int i = 0; i < redeliveryCount; i++) { + delay = redeliveryPolicy.getNextRedeliveryDelay(delay); + } scheduleRedelivery(context, messageReference, delay, ++redeliveryCount); } else if (isSendToDlqIfMaxRetriesExceeded()) { @@ -199,11 +200,4 @@ public class RedeliveryPlugin extends BrokerPluginSupport { } } - private int getExistingDelay(MessageReference messageReference) throws IOException { - Object val = messageReference.getMessage().getProperty(REDELIVERY_DELAY); - if (val instanceof Long) { - return ((Long)val).intValue(); - } - return 0; - } } diff --git a/activemq-client/src/main/java/org/apache/activemq/RedeliveryPolicy.java b/activemq-client/src/main/java/org/apache/activemq/RedeliveryPolicy.java index e0a8f333d1..91f2b71923 100644 --- a/activemq-client/src/main/java/org/apache/activemq/RedeliveryPolicy.java +++ b/activemq-client/src/main/java/org/apache/activemq/RedeliveryPolicy.java @@ -98,10 +98,7 @@ public class RedeliveryPolicy extends DestinationMapEntry implements Cloneable, } public long getNextRedeliveryDelay(long previousDelay) { - long nextDelay = initialRedeliveryDelay; - if (nextDelay == 0) { - nextDelay = redeliveryDelay; - } + long nextDelay = redeliveryDelay; if (previousDelay > 0 && useExponentialBackOff && backOffMultiplier > 1) { nextDelay = (long) (previousDelay * backOffMultiplier); diff --git a/activemq-ra/src/main/java/org/apache/activemq/ra/ActiveMQActivationSpec.java b/activemq-ra/src/main/java/org/apache/activemq/ra/ActiveMQActivationSpec.java index c17f7f0f71..0c1440b9b4 100644 --- a/activemq-ra/src/main/java/org/apache/activemq/ra/ActiveMQActivationSpec.java +++ b/activemq-ra/src/main/java/org/apache/activemq/ra/ActiveMQActivationSpec.java @@ -639,6 +639,7 @@ public class ActiveMQActivationSpec implements MessageActivationSpec, Serializab */ public void setInitialRedeliveryDelay(long initialRedeliveryDelay) { lazyCreateRedeliveryPolicy().setInitialRedeliveryDelay(initialRedeliveryDelay); + redeliveryPolicy.setRedeliveryDelay(initialRedeliveryDelay); } /** diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/RedeliveryPolicyTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/RedeliveryPolicyTest.java index 1f8f687e77..b7a870aa45 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/RedeliveryPolicyTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/RedeliveryPolicyTest.java @@ -75,11 +75,11 @@ public class RedeliveryPolicyTest extends JmsTestSupport { policy.setInitialRedeliveryDelay(500); long delay = policy.getNextRedeliveryDelay(500); - assertEquals(500, delay); + assertEquals(1000, delay); delay = policy.getNextRedeliveryDelay(delay); - assertEquals(500, delay); + assertEquals(1000, delay); delay = policy.getNextRedeliveryDelay(delay); - assertEquals(500, delay); + assertEquals(1000, delay); } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRedeliveryTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRedeliveryTest.java index 1fc7a6aeb2..4320ade1ae 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRedeliveryTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerRedeliveryTest.java @@ -81,7 +81,7 @@ public class BrokerRedeliveryTest extends org.apache.activemq.TestSupport { LOG.info("got: " + brokerRedeliveryMessage); assertNotNull("got message via broker redelivery after delay", brokerRedeliveryMessage); assertEquals("message matches", message.getStringProperty("data"), brokerRedeliveryMessage.getStringProperty("data")); - assertEquals("has expiryDelay specified", i == 0 ? initialRedeliveryDelayMillis : redeliveryDelayMillis, brokerRedeliveryMessage.getLongProperty(RedeliveryPlugin.REDELIVERY_DELAY)); + assertEquals("has expiryDelay specified - iteration:" + i, i == 0 ? initialRedeliveryDelayMillis : redeliveryDelayMillis, brokerRedeliveryMessage.getLongProperty(RedeliveryPlugin.REDELIVERY_DELAY)); consumerSession.rollback(); }