You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Rob Godfrey (JIRA)" <qp...@incubator.apache.org> on 2008/09/19 15:50:45 UTC
[jira] Commented: (QPID-1286) Priority queues can try to deliver
messages to deleted subscribers
[ https://issues.apache.org/jira/browse/QPID-1286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12632684#action_12632684 ]
Rob Godfrey commented on QPID-1286:
-----------------------------------
Review Comments:
/incubator/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/queue/AMQPriorityQueue.java
==================================================================================
rather than testing .isDeleted() on the subscription, the test should be is subnode != null in the while loop conditional:
i.e.
while(subnode != null && entry.compareTo(subnode) < 0 && !entry.isAcquired())
{
if(subscription.setLastSeenEntry(subnode,entry))
{
break;
}
else
{
subnode = subscription.getLastSeenEntry();
}
}
In general we cannot ever guarantee that another thread might not update the last seen entry to null while we are in this loop. Therefore we should always guard against this.
/incubator/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java
====================================================================================
member variable names _queue and _virtualHost have been renamed to queue and virtualHost
The former comply with our coding standards; the latter do not.
Other new member variables are also our of compliance with coding standards.
Assert on line 121 can never fail... either the constructor will succeed (in which case queue will be non null) or the constructor must have thrown an exception
Assert on line 234 can never fail
/incubator/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/queue/PriorityTest.java
==============================================================================
testOddOrdering()
I would either send the publishes as part of a transaction; close the publishing connection after the last publish; or leave a sleep between the publish and the consume. You need to ensure that the last publish has been perfromed before you create the consumer. Otherwise you have a potential race where the consumer might be created before all the publishes are processed.
> Priority queues can try to deliver messages to deleted subscribers
> ------------------------------------------------------------------
>
> Key: QPID-1286
> URL: https://issues.apache.org/jira/browse/QPID-1286
> Project: Qpid
> Issue Type: Bug
> Components: Java Broker
> Affects Versions: M4
> Reporter: Aidan Skinner
> Assignee: Rob Godfrey
> Fix For: M4
>
>
> AMQPriorityQueue.checkSubscriptionsNotAheadOfDelivery can tell deleted subscribers about messages that are being enqueued.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.