diff options
author | Keith Wall <kwall@apache.org> | 2012-03-19 14:00:37 +0000 |
---|---|---|
committer | Keith Wall <kwall@apache.org> | 2012-03-19 14:00:37 +0000 |
commit | e9a23033bb075f50b0a46c9366012e30538a4e54 (patch) | |
tree | ca952b4b7109cea8ff7ca7896f708c525ec3410a | |
parent | 617810d8db1359d81688be587908cebe77d3f559 (diff) | |
download | qpid-python-e9a23033bb075f50b0a46c9366012e30538a4e54.tar.gz |
QPID-3895: Remove blocked channel/session from the list of blocked channels on channel/session close
This patch adds the fllowing:
- fixes AMQChannel to stop sending flow commands if channel is closing
- fixes AMQChannel#compareTo ServerSession#compareTo
- removes AMQSessionModel#getID() method from AMQChannel and Server session in order to avoid confusions
Applied patch from Oleksandr Rudyy <orudyy@gmail.com>.
git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1302455 13f79535-47bb-0310-9956-ffa450edef68
7 files changed, 163 insertions, 19 deletions
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java index 588b2079f2..22f9544b0c 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java @@ -1123,11 +1123,6 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm } - public Object getID() - { - return _channelId; - } - public AMQConnectionModel getConnectionModel() { return _session; @@ -1377,7 +1372,7 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm { if(_blockingQueues.remove(queue)) { - if(_blocking.compareAndSet(true,false)) + if(_blocking.compareAndSet(true,false) && !isClosing()) { _actor.message(_logSubject, ChannelMessages.FLOW_REMOVED()); @@ -1627,6 +1622,6 @@ public class AMQChannel implements SessionConfig, AMQSessionModel, AsyncAutoComm public int compareTo(AMQSessionModel session) { - return getId().toString().compareTo(session.getID().toString()); + return getId().compareTo(session.getId()); } } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java index f6980be525..1a055240b9 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java @@ -1315,7 +1315,8 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr public void closeSession(AMQSessionModel session, AMQConstant cause, String message) throws AMQException { - closeChannel((Integer)session.getID()); + int channelId = ((AMQChannel)session).getChannelId(); + closeChannel(channelId); MethodRegistry methodRegistry = getMethodRegistry(); ChannelCloseBody responseBody = @@ -1324,7 +1325,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr new AMQShortString(message), 0,0); - writeFrame(responseBody.generateFrame((Integer)session.getID())); + writeFrame(responseBody.generateFrame(channelId)); } public void close(AMQConstant cause, String message) throws AMQException diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java index a69f2a74ee..fa171815ca 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java @@ -20,6 +20,7 @@ */ package org.apache.qpid.server.protocol; +import java.util.UUID; import java.util.concurrent.ConcurrentSkipListSet; import org.apache.qpid.AMQException; @@ -35,7 +36,8 @@ import org.apache.qpid.server.queue.SimpleAMQQueue; */ public interface AMQSessionModel extends Comparable<AMQSessionModel> { - public Object getID(); + /** Unique session ID across entire broker*/ + public UUID getId(); public AMQConnectionModel getConnectionModel(); diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java index 9f7b8c53b8..462e880e5f 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java @@ -720,11 +720,6 @@ public class ServerSession extends Session close(); } - public Object getID() - { - return getName(); - } - public AMQConnectionModel getConnectionModel() { return getConnection(); @@ -854,7 +849,6 @@ public class ServerSession extends Session // unregister subscriptions in order to prevent sending of new messages // to subscriptions with closing session unregisterSubscriptions(); - super.close(); } @@ -1025,6 +1019,7 @@ public class ServerSession extends Session public int compareTo(AMQSessionModel session) { - return getId().toString().compareTo(session.getID().toString()); + return getId().compareTo(session.getId()); } + } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java new file mode 100644 index 0000000000..fc6cbcb248 --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/AMQChannelTest.java @@ -0,0 +1,52 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.qpid.server; + +import org.apache.qpid.server.protocol.AMQProtocolSession; +import org.apache.qpid.server.protocol.InternalTestProtocolSession; +import org.apache.qpid.server.registry.ApplicationRegistry; +import org.apache.qpid.server.util.InternalBrokerBaseCase; +import org.apache.qpid.server.virtualhost.VirtualHost; + +public class AMQChannelTest extends InternalBrokerBaseCase +{ + private VirtualHost _virtualHost; + private AMQProtocolSession _protocolSession; + + @Override + public void setUp() throws Exception + { + super.setUp(); + _virtualHost = ApplicationRegistry.getInstance().getVirtualHostRegistry().getVirtualHosts().iterator().next(); + _protocolSession = new InternalTestProtocolSession(_virtualHost); + } + + public void testCompareTo() throws Exception + { + AMQChannel channel1 = new AMQChannel(_protocolSession, 1, _virtualHost.getMessageStore()); + + // create a channel with the same channelId but on a different session + AMQChannel channel2 = new AMQChannel(new InternalTestProtocolSession(_virtualHost), 1, _virtualHost.getMessageStore()); + assertFalse("Unexpected compare result", channel1.compareTo(channel2) == 0); + assertEquals("Unexpected compare result", 0, channel1.compareTo(channel1)); + } + +} diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java new file mode 100644 index 0000000000..d775b0f2f8 --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/transport/ServerSessionTest.java @@ -0,0 +1,67 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.qpid.server.transport; + +import java.util.UUID; + +import org.apache.qpid.server.configuration.MockConnectionConfig; +import org.apache.qpid.server.registry.ApplicationRegistry; +import org.apache.qpid.server.util.InternalBrokerBaseCase; +import org.apache.qpid.server.virtualhost.VirtualHost; +import org.apache.qpid.transport.Binary; + +public class ServerSessionTest extends InternalBrokerBaseCase +{ + + private VirtualHost _virtualHost; + + @Override + public void setUp() throws Exception + { + super.setUp(); + _virtualHost = ApplicationRegistry.getInstance().getVirtualHostRegistry().getVirtualHosts().iterator().next(); + } + + public void testCompareTo() throws Exception + { + ServerConnection connection = new ServerConnection(1); + connection.setConnectionConfig(createConnectionConfig()); + ServerSession session1 = new ServerSession(connection, new ServerSessionDelegate(), + new Binary(getName().getBytes()), 0 , connection.getConfig()); + + // create a session with the same name but on a different connection + ServerConnection connection2 = new ServerConnection(2); + connection2.setConnectionConfig(createConnectionConfig()); + ServerSession session2 = new ServerSession(connection2, new ServerSessionDelegate(), + new Binary(getName().getBytes()), 0 , connection2.getConfig()); + + assertFalse("Unexpected compare result", session1.compareTo(session2) == 0); + assertEquals("Unexpected compare result", 0, session1.compareTo(session1)); + } + + private MockConnectionConfig createConnectionConfig() + { + return new MockConnectionConfig(UUID.randomUUID(), null, null, + false, 1, _virtualHost, "address", Boolean.TRUE, Boolean.TRUE, Boolean.TRUE, + "authid", "remoteProcessName", new Integer(1967), new Integer(1970), _virtualHost.getConfigStore(), Boolean.FALSE); + } + +} diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java index ad8c856a74..13053d02df 100644 --- a/qpid/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java +++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java @@ -357,13 +357,45 @@ public class ProducerFlowControlTest extends AbstractTestLogging consumer.receive(); } + public void testQueueDeleteWithBlockedFlow() throws Exception + { + String queueName = getTestQueueName(); + createAndBindQueueWithFlowControlEnabled(producerSession, queueName, 1000, 800, true, false); + + producer = producerSession.createProducer(queue); + + // try to send 5 messages (should block after 4) + sendMessagesAsync(producer, producerSession, 5, 50L); + + Thread.sleep(5000); + + assertEquals("Incorrect number of message sent before blocking", 4, _sentMessages.get()); + + // close blocked producer session and connection + producerConnection.close(); + + // delete queue with a consumer session + ((AMQSession<?,?>) consumerSession).sendQueueDelete(new AMQShortString(queueName)); + + consumer = consumerSession.createConsumer(queue); + consumerConnection.start(); + + Message message = consumer.receive(1000l); + assertNull("Unexpected message", message); + } + private void createAndBindQueueWithFlowControlEnabled(Session session, String queueName, int capacity, int resumeCapacity) throws Exception { + createAndBindQueueWithFlowControlEnabled(session, queueName, capacity, resumeCapacity, false, true); + } + + private void createAndBindQueueWithFlowControlEnabled(Session session, String queueName, int capacity, int resumeCapacity, boolean durable, boolean autoDelete) throws Exception + { final Map<String,Object> arguments = new HashMap<String, Object>(); arguments.put("x-qpid-capacity",capacity); arguments.put("x-qpid-flow-resume-capacity",resumeCapacity); - ((AMQSession<?,?>) session).createQueue(new AMQShortString(queueName), true, false, false, arguments); - queue = session.createQueue("direct://amq.direct/"+queueName+"/"+queueName+"?durable='false'&autodelete='true'"); + ((AMQSession<?,?>) session).createQueue(new AMQShortString(queueName), autoDelete, durable, false, arguments); + queue = session.createQueue("direct://amq.direct/"+queueName+"/"+queueName+"?durable='" + durable + "'&autodelete='" + autoDelete + "'"); ((AMQSession<?,?>) session).declareAndBind((AMQDestination)queue); } |