Re: Review Request: QPID-4118 HA does not work with auhentication and authorization.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5894/#review9053 --- Ship it! Ship It! - Gordon Sim On July 10, 2012, 8:26 p.m., Alan Conway wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5894/ --- (Updated July 10, 2012, 8:26 p.m.) Review request for qpid and Gordon Sim. Description --- If authentication and ACL are enabled on brokers in a HA cluster, backup brokers are unable to replicate even if --ha-username etc. are set to a user with all priviledges. Need to correct this, and update the documentation explaining what permissions are required to run a secure HA cluster. Diffs - /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1359232 /trunk/qpid/cpp/src/qpid/broker/Link.h 1359232 /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1359232 /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1359232 /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1359232 /trunk/qpid/cpp/src/tests/ha_tests.py 1359232 /trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml 1359232 /trunk/qpid/tools/src/py/qpid-ha 1359232 Diff: https://reviews.apache.org/r/5894/diff/ Testing --- make check, new unit test Thanks, Alan Conway
Re: Review Request: QPID-4128 HA should not use amq.failover for replication links
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5895/#review9054 --- Ship it! Ship It! - Gordon Sim On July 10, 2012, 7:53 p.m., Alan Conway wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5895/ --- (Updated July 10, 2012, 7:53 p.m.) Review request for qpid. Description --- HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. Diffs - /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1359232 /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1359232 Diff: https://reviews.apache.org/r/5895/diff/ Testing --- Thanks, Alan Conway
[jira] [Commented] (QPID-3999) [Java Broker] Implement Web based management tool for the Java Broker
[ https://issues.apache.org/jira/browse/QPID-3999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411402#comment-13411402 ] Robbie Gemmell commented on QPID-3999: -- 0001-QPID-3999-Java-Broker-Get-the-user-name-from-HttpSer.patch (9th July) and 0001-QPID-3998-QPID-3999-System-tests-for-Rest-API-small-.patch (10 July) applied to trunk, after the branch for 0.18 was created. [Java Broker] Implement Web based management tool for the Java Broker - Key: QPID-3999 URL: https://issues.apache.org/jira/browse/QPID-3999 Project: Qpid Issue Type: Improvement Components: Java Broker Reporter: Rob Godfrey Fix For: 0.17 Attachments: 0001-QPID-3998-QPID-3999-System-tests-for-Rest-API-small-.patch, 0001-QPID-3999-Add-authorization-checks-for-message-copy-.patch, 0001-QPID-3999-Add-default-constructors-to-servlets-make-.patch, 0001-QPID-3999-Add-java-broker-web-module-to-package-brok.patch, 0001-QPID-3999-Add-servlet-filter-to-set-request-credenti.patch, 0001-QPID-3999-Allow-invocation-of-functionality-original.patch, 0001-QPID-3999-Java-Broker-Get-the-user-name-from-HttpSer.patch, 0001-QPID-3999-System-tests-for-Rest-API.patch, 0001-QPID-3999-Use-anonymous-subject-for-non-authenticate.patch Implement a web front end which uses the RESTful HTTP management API -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Comment Edited] (QPID-3999) [Java Broker] Implement Web based management tool for the Java Broker
[ https://issues.apache.org/jira/browse/QPID-3999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411402#comment-13411402 ] Robbie Gemmell edited comment on QPID-3999 at 7/11/12 11:25 AM: 0001\-QPID\-3999\-Java\-Broker\-Get\-the\-user\-name\-from\-HttpSer.patch (9th July) and 0001\-QPID\-3998\-QPID\-3999\-System\-tests\-for\-Rest\-API\-small\-.patch (10 July) applied to trunk, after the branch for 0.18 was created. was (Author: gemmellr): 0001-QPID-3999-Java-Broker-Get-the-user-name-from-HttpSer.patch (9th July) and 0001-QPID-3998-QPID-3999-System-tests-for-Rest-API-small-.patch (10 July) applied to trunk, after the branch for 0.18 was created. [Java Broker] Implement Web based management tool for the Java Broker - Key: QPID-3999 URL: https://issues.apache.org/jira/browse/QPID-3999 Project: Qpid Issue Type: Improvement Components: Java Broker Reporter: Rob Godfrey Fix For: 0.17 Attachments: 0001-QPID-3998-QPID-3999-System-tests-for-Rest-API-small-.patch, 0001-QPID-3999-Add-authorization-checks-for-message-copy-.patch, 0001-QPID-3999-Add-default-constructors-to-servlets-make-.patch, 0001-QPID-3999-Add-java-broker-web-module-to-package-brok.patch, 0001-QPID-3999-Add-servlet-filter-to-set-request-credenti.patch, 0001-QPID-3999-Allow-invocation-of-functionality-original.patch, 0001-QPID-3999-Java-Broker-Get-the-user-name-from-HttpSer.patch, 0001-QPID-3999-System-tests-for-Rest-API.patch, 0001-QPID-3999-Use-anonymous-subject-for-non-authenticate.patch Implement a web front end which uses the RESTful HTTP management API -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Comment Edited] (QPID-3999) [Java Broker] Implement Web based management tool for the Java Broker
[ https://issues.apache.org/jira/browse/QPID-3999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411402#comment-13411402 ] Robbie Gemmell edited comment on QPID-3999 at 7/11/12 11:35 AM: 0001\-QPID\-3999\-Java\-Broker\-Get\-the\-user\-name\-from\-HttpSer.patch (9th July) and 0001\-QPID\-3998\-QPID\-3999\-System\-tests\-for\-Rest\-API\-small\-.patch (10 July) applied to trunk, after the branch for 0.18 was created. Commits were http://svn.apache.org/viewvc?view=revisionrevision=1360120 and http://svn.apache.org/viewvc?view=revisionrevision=1360121 respectively. was (Author: gemmellr): 0001\-QPID\-3999\-Java\-Broker\-Get\-the\-user\-name\-from\-HttpSer.patch (9th July) and 0001\-QPID\-3998\-QPID\-3999\-System\-tests\-for\-Rest\-API\-small\-.patch (10 July) applied to trunk, after the branch for 0.18 was created. [Java Broker] Implement Web based management tool for the Java Broker - Key: QPID-3999 URL: https://issues.apache.org/jira/browse/QPID-3999 Project: Qpid Issue Type: Improvement Components: Java Broker Reporter: Rob Godfrey Fix For: 0.17 Attachments: 0001-QPID-3998-QPID-3999-System-tests-for-Rest-API-small-.patch, 0001-QPID-3999-Add-authorization-checks-for-message-copy-.patch, 0001-QPID-3999-Add-default-constructors-to-servlets-make-.patch, 0001-QPID-3999-Add-java-broker-web-module-to-package-brok.patch, 0001-QPID-3999-Add-servlet-filter-to-set-request-credenti.patch, 0001-QPID-3999-Allow-invocation-of-functionality-original.patch, 0001-QPID-3999-Java-Broker-Get-the-user-name-from-HttpSer.patch, 0001-QPID-3999-System-tests-for-Rest-API.patch, 0001-QPID-3999-Use-anonymous-subject-for-non-authenticate.patch Implement a web front end which uses the RESTful HTTP management API -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Fix for QPID-3575
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9055 --- Hi Rajith, The suggested code changes looks reasonable for me. I could not find any issue with the suggested solution. The only thing I would like to ask is to add some tests to cover the changes. For example, I used the following tests to test your changes with Java Broker public class ExceptionHandlingTest extends QpidBrokerTestCase implements ExceptionListener { private AMQConnection _connection; private AMQSession_0_10 _exceptionSession; private AMQSession_0_10 _session; private JMSException _exception; public void setUp() throws Exception { super.setUp(); _connection = (AMQConnection) getConnection(); _connection.setExceptionListener(this); _session = (AMQSession_0_10) _connection.createSession(false, Session.AUTO_ACKNOWLEDGE); _exceptionSession = (AMQSession_0_10) _connection.createSession(false, Session.AUTO_ACKNOWLEDGE); _exception = null; } public void testDeclareStandardEchange() throws Exception { Exception caughtException = null; try { _exceptionSession.declareExchange(new AMQShortString(qpid.management), new AMQShortString(direct), false); } catch (Exception e) { e.printStackTrace(); caughtException = e; } assertState(); assertTrue(Unexpected exception, caughtException == null || caughtException instanceof AMQException); } public void testDeclareQueuePassiveForNonExistingQueue() throws Exception { Exception caughtException = null; try { _exceptionSession.send0_10QueueDeclare((AMQDestination) getTestQueue(), _connection.getProtocolHandler(), false, false, true); } catch (Exception e) { e.printStackTrace(); caughtException = e; } assertState(); assertTrue(Unexpected exception, caughtException == null || caughtException instanceof AMQException); } @Override public void onException(JMSException exception) { _exception = exception; } private void assertState() throws Exception { assertFalse(Connection has been closed unexpectedly, _connection.isClosed()); assertFalse(Not affected session has been closed unexpectedly, _session.isClosed()); assertNotNull(Exception is not received, _exception); assertTrue(Exception session has not been closed, _exceptionSession.isClosed()); // assert that JMS operations are performed OK on non-affected session Destination queue = _session.createQueue(getTestQueueName()); MessageConsumer consumer = _session.createConsumer(queue); MessageProducer producer = _session.createProducer(queue); producer.send(_session.createTextMessage(Test)); _connection.start(); Message m = consumer.receive(1000l); assertNotNull(Test message has not been received, m); consumer.close(); producer.close(); } } I did not try to run these tests with CPP broker. The failing test SelectorTest#testRuntimeSelectorError can be fixed by replacing the assert checking if connection is closed assertTrue(Connection should be closed, _connection.isClosed()); with assert checking if session is closed assertTrue(Session should be closed, (AMQSession?,?)session.isClosed()); Apart from adding the tests, I do not have any other commentaries about the suggested changes. - Oleksandr Rudyy On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the following. 1. When a session exception is received, it closes the Session (and the consumers/producers associated with it) to prevent further use of this session. 2. It also notifies the connection, however the exception being passed, is marked as a soft error, thereby preventing the connection from being marked
0.18 inclusion request
Hi Justin, Could you please consider inclusions of trunk commits http://svn.apache.org/viewvc?view=revisionrevision=1360120 and http://svn.apache.org/viewvc?view=revisionrevision=1360121 into 0.18 ? These 2 commits are low risk bug fixes and improvements for recently added into Java Broker REST API and management web console. The second commit also adds 40 system tests for the Java Broker REST API. The functional changes are covered by these test and do not interfere with broker core functionality Kind Regards, Alex Rudyy - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-3914) SSL Client Authentication support for the Windows C++ client
[ https://issues.apache.org/jira/browse/QPID-3914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411430#comment-13411430 ] Michal Zerola commented on QPID-3914: - Hi, what was the decision taken on the above comments? Should I modify the patch? SSL Client Authentication support for the Windows C++ client Key: QPID-3914 URL: https://issues.apache.org/jira/browse/QPID-3914 Project: Qpid Issue Type: New Feature Components: C++ Client Affects Versions: 0.14, 0.16 Environment: Windows (all versions) Reporter: JAkub Scholz Attachments: ssl-client-auth-filecert.patch, ssl-client-authentication.patch The Windows C++ client has been missing support for the SSL Client Authentication - authentication using SSL certificates on the client side. The patch attached to this JIRA implements this feature. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-3914) SSL Client Authentication support for the Windows C++ client
[ https://issues.apache.org/jira/browse/QPID-3914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411500#comment-13411500 ] Steve Huston commented on QPID-3914: Hi Michal... apparently noone has had free time to address the issues raised and fully investigate the options. SSL Client Authentication support for the Windows C++ client Key: QPID-3914 URL: https://issues.apache.org/jira/browse/QPID-3914 Project: Qpid Issue Type: New Feature Components: C++ Client Affects Versions: 0.14, 0.16 Environment: Windows (all versions) Reporter: JAkub Scholz Attachments: ssl-client-auth-filecert.patch, ssl-client-authentication.patch The Windows C++ client has been missing support for the SSL Client Authentication - authentication using SSL certificates on the client side. The patch attached to this JIRA implements this feature. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Acl publish exchange fastpath lookup uses TopicExchange binding string match rules
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5836/#review9057 --- trunk/qpid/cpp/src/qpid/acl/AclData.h https://reviews.apache.org/r/5836/#comment19204 Type names should be capitalized. I realize the names weren't introduced by this change but would be nice to clean it up. trunk/qpid/cpp/src/qpid/acl/AclData.h https://reviews.apache.org/r/5836/#comment19205 Why so many public data members? trunk/qpid/cpp/src/qpid/acl/AclData.cpp https://reviews.apache.org/r/5836/#comment19206 I'm not familiar with the logic here so can't really comment on correcness. - Alan Conway On July 10, 2012, 3:56 p.m., Chug Rolke wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5836/ --- (Updated July 10, 2012, 3:56 p.m.) Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross. Description --- Acl match rules with single '*' at end of a string are inadequate for expressing topic exchange key matches. This patch uses the actual topic exchange match logic for matching the Acl rule against a user's run-time publish request. Note that each Acl rule contains a topic exchange node tree with only one key in it. The topic exchange match then returns match or no-match for a lookup on that rule. Although the topic exchange node trees may have many nodes in them, Acl logic uses one tree-with-one-key per rule so that allow and deny rules and broad and narrow key specifications may be intermixed in the Acl rule file and still produce correct matches. This patch also improves a run-time issue by parsing the 'publish exchange' rules' property list once at rule-creation time. The required routing key and exchange name are pulled out of the property list and placed as members of the rule. When run-time publish authorizations are performed the n the lookup code uses these members directly. This addresses bug QPID-3892. https://issues.apache.org/jira/browse/QPID-3892 Diffs - trunk/qpid/cpp/src/qpid/acl/AclData.h 1359193 trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1359193 trunk/qpid/cpp/src/qpid/acl/AclReader.h 1359193 trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1359193 trunk/qpid/cpp/src/qpid/acl/AclTopicMatch.h PRE-CREATION trunk/qpid/cpp/src/tests/acl.py 1359193 Diff: https://reviews.apache.org/r/5836/diff/ Testing --- The topic key node request self test patterns are replicated through Acl files and the same tests are run. New tests are added to check that Acl user '*' works with the same results as named users. Acl tests do not include multiple-node-per-tree tests since that's not how Acl code uses the node trees. Thanks, Chug Rolke
[jira] [Created] (QPID-4129) C++ Broker failed client authentication in multi-node cluster causes connection leak
Chuck Rolke created QPID-4129: - Summary: C++ Broker failed client authentication in multi-node cluster causes connection leak Key: QPID-4129 URL: https://issues.apache.org/jira/browse/QPID-4129 Project: Qpid Issue Type: Bug Components: C++ Broker Affects Versions: 0.16 Reporter: Chuck Rolke Assignee: Chuck Rolke When an HA cluster has at least one connected peer cluster member and a client connection fails authentication then the broker::Connection is leaked. The issue does not affect standalone brokers nor does it affect a clustered broker with no connected peers. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Created] (QPID-4130) Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions.
Alan Conway created QPID-4130: - Summary: Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions. Key: QPID-4130 URL: https://issues.apache.org/jira/browse/QPID-4130 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Fix for QPID-3575
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9058 --- I don't see any obvious issues with the patch, though I must admit to still being reluctant to change exception handling in the client this late into 0.18 development given it has presumably been this way since time began. As the behavior is ultimately controlled by a boolean, it might be an idea to put a system property backdoor in to allow changing the behaviour back later should the need arise. - Robbie Gemmell On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the following. 1. When a session exception is received, it closes the Session (and the consumers/producers associated with it) to prevent further use of this session. 2. It also notifies the connection, however the exception being passed, is marked as a soft error, thereby preventing the connection from being marked as closed. It also prevents the rest of the sessions being closed. 3. The exception will however be notified via the exception listener. This is important in certain cases, as it might be the only case where an exception can be notified. What is the impact to applications. 1. If an application closes the connection when it receives an exception, and then recreates the connection and it's associated objects (sessions/consumers/producers) then this change will have no impact. 2. If an application just recreates everything from scratch, without resorting to closing the connection (assuming that all exceptions received via the listener will automatically close the connection), there will be tcp connection leaks. However the objects associated with the connection will eventualy be garbage collected. We should probably close the underlying TCP connection in the finalizer for the Connection object as a safety measure. With this change, the recommended approach would be, for application to check the connection status in deciding what it should do. For example, if (conn.isClosed()) { // re-create connection. } else { // session exception // this means the rest of the sessions and the connection is still open. } This addresses bug QPID-3575. https://issues.apache.org/jira/browse/QPID-3575 Diffs - http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/AMQException.java 1357981 Diff: https://reviews.apache.org/r/5794/diff/ Testing --- Tested with the default java profile and the cpp test profile. SelectorTest#testRuntimeSelectorError is failing as it expects the connection to be closed. I need to investigate further to determine if the test needs to be changed or not. Thanks, rajith attapattu
[jira] [Commented] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411606#comment-13411606 ] Alan Conway commented on QPID-4128: --- Fixed on trunk r1360218 | aconway | 2012-07-11 11:02:36 -0400 (Wed, 11 Jul 2012) | 2 lines QPID-4128: Remove use of intrusive_ptr::reset, not available in older boost versions. Requesting review for 0.18 branch. HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411614#comment-13411614 ] Alan Conway commented on QPID-4128: --- Mixed 2 issues by mistake on same JIRA, but both are trivial 1 line fixes: - HA replication should not use amq.failover exchange: reviewed by gsim https://reviews.apache.org/r/5069/ - Remove use of intrusive_ptr::reset from ha/Primary.cpp - raised by Steve Huston HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Fix for QPID-3575
On July 11, 2012, 11:42 a.m., Oleksandr Rudyy wrote: Hi Rajith, The suggested code changes looks reasonable for me. I could not find any issue with the suggested solution. The only thing I would like to ask is to add some tests to cover the changes. For example, I used the following tests to test your changes with Java Broker public class ExceptionHandlingTest extends QpidBrokerTestCase implements ExceptionListener { private AMQConnection _connection; private AMQSession_0_10 _exceptionSession; private AMQSession_0_10 _session; private JMSException _exception; public void setUp() throws Exception { super.setUp(); _connection = (AMQConnection) getConnection(); _connection.setExceptionListener(this); _session = (AMQSession_0_10) _connection.createSession(false, Session.AUTO_ACKNOWLEDGE); _exceptionSession = (AMQSession_0_10) _connection.createSession(false, Session.AUTO_ACKNOWLEDGE); _exception = null; } public void testDeclareStandardEchange() throws Exception { Exception caughtException = null; try { _exceptionSession.declareExchange(new AMQShortString(qpid.management), new AMQShortString(direct), false); } catch (Exception e) { e.printStackTrace(); caughtException = e; } assertState(); assertTrue(Unexpected exception, caughtException == null || caughtException instanceof AMQException); } public void testDeclareQueuePassiveForNonExistingQueue() throws Exception { Exception caughtException = null; try { _exceptionSession.send0_10QueueDeclare((AMQDestination) getTestQueue(), _connection.getProtocolHandler(), false, false, true); } catch (Exception e) { e.printStackTrace(); caughtException = e; } assertState(); assertTrue(Unexpected exception, caughtException == null || caughtException instanceof AMQException); } @Override public void onException(JMSException exception) { _exception = exception; } private void assertState() throws Exception { assertFalse(Connection has been closed unexpectedly, _connection.isClosed()); assertFalse(Not affected session has been closed unexpectedly, _session.isClosed()); assertNotNull(Exception is not received, _exception); assertTrue(Exception session has not been closed, _exceptionSession.isClosed()); // assert that JMS operations are performed OK on non-affected session Destination queue = _session.createQueue(getTestQueueName()); MessageConsumer consumer = _session.createConsumer(queue); MessageProducer producer = _session.createProducer(queue); producer.send(_session.createTextMessage(Test)); _connection.start(); Message m = consumer.receive(1000l); assertNotNull(Test message has not been received, m); consumer.close(); producer.close(); } } I did not try to run these tests with CPP broker. The failing test SelectorTest#testRuntimeSelectorError can be fixed by replacing the assert checking if connection is closed assertTrue(Connection should be closed, _connection.isClosed()); with assert checking if session is closed assertTrue(Session should be closed, (AMQSession?,?)session.isClosed()); Apart from adding the tests, I do not have any other commentaries about the suggested changes. Thanks Alex! I will add the tests before I commit. - rajith --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9055 --- On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the
Re: Review Request: Fix for QPID-3575
On July 11, 2012, 3 p.m., Robbie Gemmell wrote: I don't see any obvious issues with the patch, though I must admit to still being reluctant to change exception handling in the client this late into 0.18 development given it has presumably been this way since time began. As the behavior is ultimately controlled by a boolean, it might be an idea to put a system property backdoor in to allow changing the behaviour back later should the need arise. Robbie, thanks for the review. I appreciate it. I think the system property might be a good idea. But that would take us back to the connection leak issue when someone decides to go the old route. So if we add the system property, then we might have to add code to kill the underlying connection, close all the sessions if the Session Exception is considered 'hard' (due to the system prop) But closing the connection was difficult as it created a deadlock :( -- My initial attempt was to just not change the exception behavior and to close everything. - rajith --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9058 --- On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the following. 1. When a session exception is received, it closes the Session (and the consumers/producers associated with it) to prevent further use of this session. 2. It also notifies the connection, however the exception being passed, is marked as a soft error, thereby preventing the connection from being marked as closed. It also prevents the rest of the sessions being closed. 3. The exception will however be notified via the exception listener. This is important in certain cases, as it might be the only case where an exception can be notified. What is the impact to applications. 1. If an application closes the connection when it receives an exception, and then recreates the connection and it's associated objects (sessions/consumers/producers) then this change will have no impact. 2. If an application just recreates everything from scratch, without resorting to closing the connection (assuming that all exceptions received via the listener will automatically close the connection), there will be tcp connection leaks. However the objects associated with the connection will eventualy be garbage collected. We should probably close the underlying TCP connection in the finalizer for the Connection object as a safety measure. With this change, the recommended approach would be, for application to check the connection status in deciding what it should do. For example, if (conn.isClosed()) { // re-create connection. } else { // session exception // this means the rest of the sessions and the connection is still open. } This addresses bug QPID-3575. https://issues.apache.org/jira/browse/QPID-3575 Diffs - http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/AMQException.java 1357981 Diff: https://reviews.apache.org/r/5794/diff/ Testing --- Tested with the default java profile and the cpp test profile. SelectorTest#testRuntimeSelectorError is failing as it expects the connection to be closed. I need to investigate further to determine if the test needs to be changed or not. Thanks, rajith attapattu
Re: 0.18 release update - release branch and beta
Request for inclusion into 0.18: QPID-4129 C++ Broker failed client authentication in multi-node cluster causes connection leak Fix is http://svn.apache.org/viewvc?view=revisionrevision=1360214 - Original Message - From: Justin Ross jr...@redhat.com To: dev@qpid.apache.org Sent: Monday, July 9, 2012 1:37:15 PM Subject: 0.18 release update - release branch and beta Hi, everyone. I've branched trunk for release at revision 1359193: https://svn.apache.org/repos/asf/qpid/branches/0.18/ That means trunk is now open for development toward our next release, 0.20. Commits to the release branch require prior review and approval, recorded in a jira. The new beta distribution is also available, from revision 1359232 of the release branch: http://people.apache.org/~jross/qpid-0.18-beta/ Take it for a spin, and please report back your results. RC1 is set for 18 July, in nine days. Thanks, Justin --- 0.18 release page: https://cwiki.apache.org/qpid/018-release.html - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Resolved] (QPID-4118) HA does not work with auhentication and authorization.
[ https://issues.apache.org/jira/browse/QPID-4118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alan Conway resolved QPID-4118. --- Resolution: Fixed r1360228 | aconway | 2012-07-11 11:26:36 -0400 (Wed, 11 Jul 2012) | 9 lines QPID-4128: HA should not use amq.failover for replication links HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. HA does not work with auhentication and authorization. -- Key: QPID-4118 URL: https://issues.apache.org/jira/browse/QPID-4118 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway If authentication and ACL are enabled on brokers in a HA cluster, backup brokers are unable to replicate even if --ha-username etc. are set to a user with all priviledges. Need to correct this, and update the documentation explaining what permissions are required to run a secure HA cluster. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4118) HA does not work with auhentication and authorization.
[ https://issues.apache.org/jira/browse/QPID-4118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411649#comment-13411649 ] Alan Conway commented on QPID-4118: --- Reviewd by gsim https://reviews.apache.org/r/5894/ HA does not work with auhentication and authorization. -- Key: QPID-4118 URL: https://issues.apache.org/jira/browse/QPID-4118 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway If authentication and ACL are enabled on brokers in a HA cluster, backup brokers are unable to replicate even if --ha-username etc. are set to a user with all priviledges. Need to correct this, and update the documentation explaining what permissions are required to run a secure HA cluster. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Comment Edited] (QPID-4118) HA does not work with auhentication and authorization.
[ https://issues.apache.org/jira/browse/QPID-4118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411650#comment-13411650 ] Alan Conway edited comment on QPID-4118 at 7/11/12 3:57 PM: Merged to 0.18 r1360227 | aconway | 2012-07-11 11:26:28 -0400 (Wed, 11 Jul 2012) | 7 lines QPID-4118: HA does not work with authentication and authorization. - Updated test framework to use credentials - Updated BrokerReplicator to use HA identity to create configuration - Updated documentation with a HA security section. - Updated qpid-ha to take --sasl-mechanism was (Author: aconway): r1360228 | aconway | 2012-07-11 11:26:36 -0400 (Wed, 11 Jul 2012) | 9 lines QPID-4128: HA should not use amq.failover for replication links HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. HA does not work with auhentication and authorization. -- Key: QPID-4118 URL: https://issues.apache.org/jira/browse/QPID-4118 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway If authentication and ACL are enabled on brokers in a HA cluster, backup brokers are unable to replicate even if --ha-username etc. are set to a user with all priviledges. Need to correct this, and update the documentation explaining what permissions are required to run a secure HA cluster. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411658#comment-13411658 ] Alan Conway commented on QPID-4128: --- Merged to 0.18 r1360228 | aconway | 2012-07-11 11:26:36 -0400 (Wed, 11 Jul 2012) | 9 lines QPID-4128: HA should not use amq.failover for replication links HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Updated] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alan Conway updated QPID-4128: -- Comment: was deleted (was: Mixed 2 issues by mistake on same JIRA, but both are trivial 1 line fixes: - HA replication should not use amq.failover exchange: reviewed by gsim https://reviews.apache.org/r/5069/ - Remove use of intrusive_ptr::reset from ha/Primary.cpp - raised by Steve Huston ) HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Updated] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alan Conway updated QPID-4128: -- Comment: was deleted (was: Fixed on trunk r1360218 | aconway | 2012-07-11 11:02:36 -0400 (Wed, 11 Jul 2012) | 2 lines QPID-4128: Remove use of intrusive_ptr::reset, not available in older boost versions. Requesting review for 0.18 branch.) HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411660#comment-13411660 ] Alan Conway commented on QPID-4128: --- Reviewed by gsim https://reviews.apache.org/r/5069/ HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Resolved] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alan Conway resolved QPID-4128. --- Resolution: Fixed HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Fix for QPID-3575
On July 11, 2012, 3 p.m., Robbie Gemmell wrote: I don't see any obvious issues with the patch, though I must admit to still being reluctant to change exception handling in the client this late into 0.18 development given it has presumably been this way since time began. As the behavior is ultimately controlled by a boolean, it might be an idea to put a system property backdoor in to allow changing the behaviour back later should the need arise. rajith attapattu wrote: Robbie, thanks for the review. I appreciate it. I think the system property might be a good idea. But that would take us back to the connection leak issue when someone decides to go the old route. So if we add the system property, then we might have to add code to kill the underlying connection, close all the sessions if the Session Exception is considered 'hard' (due to the system prop) But closing the connection was difficult as it created a deadlock :( -- My initial attempt was to just not change the exception behavior and to close everything. I'm suggesting we put a backdoor in to make it behave exactly like it does now /has for probably 4 years, not to enable some other form of trying to fix the underlying problem but simply in case the sensitive'ish change late in the release process raises unforseen issues later (or anyone wants it to behave the way it does now and isn't particularly bothered by an occasional leak currently). - Robbie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9058 --- On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the following. 1. When a session exception is received, it closes the Session (and the consumers/producers associated with it) to prevent further use of this session. 2. It also notifies the connection, however the exception being passed, is marked as a soft error, thereby preventing the connection from being marked as closed. It also prevents the rest of the sessions being closed. 3. The exception will however be notified via the exception listener. This is important in certain cases, as it might be the only case where an exception can be notified. What is the impact to applications. 1. If an application closes the connection when it receives an exception, and then recreates the connection and it's associated objects (sessions/consumers/producers) then this change will have no impact. 2. If an application just recreates everything from scratch, without resorting to closing the connection (assuming that all exceptions received via the listener will automatically close the connection), there will be tcp connection leaks. However the objects associated with the connection will eventualy be garbage collected. We should probably close the underlying TCP connection in the finalizer for the Connection object as a safety measure. With this change, the recommended approach would be, for application to check the connection status in deciding what it should do. For example, if (conn.isClosed()) { // re-create connection. } else { // session exception // this means the rest of the sessions and the connection is still open. } This addresses bug QPID-3575. https://issues.apache.org/jira/browse/QPID-3575 Diffs - http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/AMQException.java 1357981 Diff: https://reviews.apache.org/r/5794/diff/ Testing --- Tested with the default java profile and the cpp test profile. SelectorTest#testRuntimeSelectorError is failing as it expects the connection to be closed. I
Re: Review Request: HA primary times out expected backups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5890/#review9062 --- Ship it! Not terribly familar with all the logic here yet, but the changes all seem reasonable to me. - Gordon Sim On July 10, 2012, 7:10 p.m., Alan Conway wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5890/ --- (Updated July 10, 2012, 7:10 p.m.) Review request for qpid and Gordon Sim. Description --- After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. Diffs - /trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp 1359673 /trunk/qpid/cpp/src/qpid/ha/Primary.h 1359673 /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1359673 /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1359673 /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1359673 /trunk/qpid/cpp/src/qpid/ha/Settings.h 1359673 /trunk/qpid/cpp/src/tests/ha_tests.py 1359673 Diff: https://reviews.apache.org/r/5890/diff/ Testing --- Thanks, Alan Conway
Re: Review Request: Fix for QPID-3575
On July 11, 2012, 3 p.m., Robbie Gemmell wrote: I don't see any obvious issues with the patch, though I must admit to still being reluctant to change exception handling in the client this late into 0.18 development given it has presumably been this way since time began. As the behavior is ultimately controlled by a boolean, it might be an idea to put a system property backdoor in to allow changing the behaviour back later should the need arise. rajith attapattu wrote: Robbie, thanks for the review. I appreciate it. I think the system property might be a good idea. But that would take us back to the connection leak issue when someone decides to go the old route. So if we add the system property, then we might have to add code to kill the underlying connection, close all the sessions if the Session Exception is considered 'hard' (due to the system prop) But closing the connection was difficult as it created a deadlock :( -- My initial attempt was to just not change the exception behavior and to close everything. Robbie Gemmell wrote: I'm suggesting we put a backdoor in to make it behave exactly like it does now /has for probably 4 years, not to enable some other form of trying to fix the underlying problem but simply in case the sensitive'ish change late in the release process raises unforseen issues later (or anyone wants it to behave the way it does now and isn't particularly bothered by an occasional leak currently). Gotcha. I will add it before I commit. - rajith --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/#review9058 --- On July 6, 2012, 12:39 a.m., rajith attapattu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5794/ --- (Updated July 6, 2012, 12:39 a.m.) Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey. Description --- When a session exception is received, we notify the connection via the exceptionReceived method. The connection will close all sessions for this connection, but does not close the underlying Protocol connection. Further calls (from application code) to close the connection, does not close the underlying protocol-connection as it skips this part, due to the connection already being marked as closed. This results in leaked connections. The proposed patch does the following. 1. When a session exception is received, it closes the Session (and the consumers/producers associated with it) to prevent further use of this session. 2. It also notifies the connection, however the exception being passed, is marked as a soft error, thereby preventing the connection from being marked as closed. It also prevents the rest of the sessions being closed. 3. The exception will however be notified via the exception listener. This is important in certain cases, as it might be the only case where an exception can be notified. What is the impact to applications. 1. If an application closes the connection when it receives an exception, and then recreates the connection and it's associated objects (sessions/consumers/producers) then this change will have no impact. 2. If an application just recreates everything from scratch, without resorting to closing the connection (assuming that all exceptions received via the listener will automatically close the connection), there will be tcp connection leaks. However the objects associated with the connection will eventualy be garbage collected. We should probably close the underlying TCP connection in the finalizer for the Connection object as a safety measure. With this change, the recommended approach would be, for application to check the connection status in deciding what it should do. For example, if (conn.isClosed()) { // re-create connection. } else { // session exception // this means the rest of the sessions and the connection is still open. } This addresses bug QPID-3575. https://issues.apache.org/jira/browse/QPID-3575 Diffs - http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1357981 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/AMQException.java 1357981 Diff: https://reviews.apache.org/r/5794/diff/ Testing --- Tested with the default java profile and the cpp test profile.
Re: [jira] [Commented] (QPID-4120) Qpid-0.16: session not closed
On 07/11/2012 05:16 AM, Paul Colby wrote: I don't want to hijack the original topic, but since it //might// be relevant, the messages I'm seeing are like: qpidd[32238]: 2012-07-08 23:02:43 error Channel exception: session-busy: Session already attached: cu...@qpid.host.30010.3(qpid/broker/SessionManager.cpp:55) qpidd[3435]: 2012-07-08 23:02:43 error Channel exception: session-busy: Session already attached: cu...@qpid.host.30010.3(qpid/broker/SessionManager.cpp:55) qpidd[20157]: 2012-07-08 23:02:44 error Channel exception: session-busy: Session already attached: cu...@qpid.host.30010.3(qpid/broker/SessionManager.cpp:55) qpidd[32238]: 2012-07-08 23:02:43 error Channel exception: not-attached: Channel 1 is not attached (qpid/amqp_0_10/SessionHandler.cpp:39) qpidd[3435]: 2012-07-08 23:02:44 error Channel exception: not-attached: Channel 1 is not attached (qpid/amqp_0_10/SessionHandler.cpp:39) qpidd[20157]: 2012-07-08 23:02:44 error Channel exception: not-attached: Channel 1 is not attached (qpid/amqp_0_10/SessionHandler.cpp:39) Note, the messages are repeated thrice (each time for a different qpidd PID), because this is a cluster with three brokers. That looks like a different issue, an apparent session name clash. I believe the session name here is made up of the client's host and pid, along with a counter that is supposed to ensure uniqueness. I don't know enough about the old QMF console code to suggest the cause, but it does seem to me to be a bug in that QMF library (or perhaps cumins use of it). Its probably not terribly harmful; perhaps just a race in a reconnect or something. - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4126) HA primary times out expected backups.
[ https://issues.apache.org/jira/browse/QPID-4126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411856#comment-13411856 ] Alan Conway commented on QPID-4126: --- Reviewed by gsim https://reviews.apache.org/r/5890/#review9062 Fixed on 0.18: r1360348 | aconway | 2012-07-11 14:55:17 -0400 (Wed, 11 Jul 2012) | 10 lines QPID-4126: HA primary times out expected backups. After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. HA primary times out expected backups. -- Key: QPID-4126 URL: https://issues.apache.org/jira/browse/QPID-4126 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4126) HA primary times out expected backups.
[ https://issues.apache.org/jira/browse/QPID-4126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411858#comment-13411858 ] Alan Conway commented on QPID-4126: --- Fixed on trunk r1359872 HA primary times out expected backups. -- Key: QPID-4126 URL: https://issues.apache.org/jira/browse/QPID-4126 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Resolved] (QPID-4126) HA primary times out expected backups.
[ https://issues.apache.org/jira/browse/QPID-4126?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alan Conway resolved QPID-4126. --- Resolution: Fixed Fix Version/s: 0.18 HA primary times out expected backups. -- Key: QPID-4126 URL: https://issues.apache.org/jira/browse/QPID-4126 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: qpidd refactor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5833/#review9080 --- Ship it! Ship It! /trunk/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/5833/#comment19230 Why pass msg by copy instead of reference? /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp https://reviews.apache.org/r/5833/#comment19251 +1 - Kenneth Giusti On July 9, 2012, 12:31 p.m., Gordon Sim wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5833/ --- (Updated July 9, 2012, 12:31 p.m.) Review request for qpid, Alan Conway and Kenneth Giusti. Description --- == Background == I've been looking at what would be required to get AMQP 1.0 support in the qpidd broker (using proton-c). In that context I felt there was a need to refactor the broker code, particularly that part that would be shared between different protocol versions. Part of the motivation was clearer separation of 0-10 specific logic, so that 1.0 logic could be introduced as an alternative. However part of it was also simply the recognition of some long-standing problems that we have never stopped to address. So, here is a patch representing my ideas on what is needed. This is already a little stale (patch was generated against r1331342) and needs a rebase. However I think the basic ideas involved are clear enough that it would be worth getting some early feedback. == Key Changes == qpid::broker::Message This is now supposed to be a protocol neutral representation of a message. It no longer exposes qpid::framing::FrameSet. It can be based on data received in different encodings (this patch only includes the existing 0-10 encoding). The immutable, sharable state is separated from the mutable queue-specific state. Messages themselves are no longer held through a shared pointer but are passed by reference or copied if needed. The immutable state (essentially the data as received) *is* still shared and referenced internally through an intrusive pointer. There is no longer a message level lock. A message instance is 'owned' by someother entity (usually the queue it is on) which controls concurrent access/modification if necessary. The persistence context is a separate part of the message also. Currently that can be shared between two message instances if desired. qpid::broker::Messages Switched from using qpid::broker::QueuedMessage (which relied on shared pointer to message itself and made sequence number the explicit - and only - way to refer to a specific message) to using modified Message class directly and a new qpid::broker::QueueCursor. The cursor is opaque outside the Messages implementation to which it relates. It provides a way to refer to a specific message (without directly using sequence number, though at present that is what is used 'under the covers') and/or to track progress through a sequence of messages (for consumers or other iterating entities). I.e. its an iterator to a Message within its containing Messages instance that is not invalidated by changes to that container. A Messages instance *owns* the Message instances within it. Other classes access this through a reference or (raw) pointer, or if needed copy it (the immutable part can be - and is - safely shared). The codepath for browse/consume is a lot more unified now. You use a cursor and call Messages::next() in each case. This also lays the foundation for selectors. The simplified Messages interface led to a simplied MessageDistributor. There is still a little more to do to clarify these separate roles (or indeed perhaps unify them?) but more on that later. qpid::broker::amqp_0_10::MessageTransfer This represents the familiar 0-10 encoding of a message. This class is broadly similar to the old Message class, based on a FrameSet. However it represents the shared and essentially immutable state. The sendHeader() method now explicitly takes a copy of the original headers and adds to it or otherwise modifies it if needed (e.g. for redelivered flag, ttl, annotations etc). [Ideally I'd like to move more of the 0-10 specific classes out of qpid::broker and into qpid::broker::amqp_0_10, but that has no functional relevance so I've left existing classes alone for now.] qpid::broker::Consumer The deliver() method now takes a QueueCursor (representing a 'handle' to this message for use in subsequent operations such as accept, relese etc) and a *constant reference* to the Message itself (i.e. consumers can't alter the state of the message on the queue directly, but only through operations on the queue itself).
[jira] [Commented] (QPID-4126) HA primary times out expected backups.
[ https://issues.apache.org/jira/browse/QPID-4126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411926#comment-13411926 ] Justin Ross commented on QPID-4126: --- Reviewed by Gordon. Approved for 0.18. HA primary times out expected backups. -- Key: QPID-4126 URL: https://issues.apache.org/jira/browse/QPID-4126 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 After a failure, the newly-promoted primary broker guards messages for each of the backups that were connected at the time of the failure. It waits for them to reconnect to the new primary before becoming active. This patch introduces a time-out so that if an expected backup fails to fail-over within the time limit, the primary will cancel the guards for that backup and carry on. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4130) Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions.
[ https://issues.apache.org/jira/browse/QPID-4130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411932#comment-13411932 ] Steve Huston commented on QPID-4130: I have verified that with the fix, ha now builds clean on RHEL 5 w/Boost 1.33 Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions. Key: QPID-4130 URL: https://issues.apache.org/jira/browse/QPID-4130 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 Remove use of intrusive_ptr::reset in ha/Primary.cpp, not supported by older boost versions. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-4128) HA should not use amq.failover for replication links
[ https://issues.apache.org/jira/browse/QPID-4128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13411953#comment-13411953 ] Justin Ross commented on QPID-4128: --- Reviewed by Gordon. Approved for 0.18. HA should not use amq.failover for replication links Key: QPID-4128 URL: https://issues.apache.org/jira/browse/QPID-4128 Project: Qpid Issue Type: Bug Components: C++ Clustering Affects Versions: 0.17 Reporter: Alan Conway Assignee: Alan Conway Fix For: 0.18 HA replication links should not use the amq.failover exchange to get updates reconnect targets. amq.failover provides the client failover list, HA manages a separate failover list for brokers. Replication links should be using the broker list, and not allow it to be overwritten by amq.failover updates. Review requested for 0.18, 2 line fix. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: 0.18 inclusion request
Hi, Alex. Approved for 0.18. Justin - Original Message - From: Oleksandr Rudyy oru...@gmail.com To: dev@qpid.apache.org Sent: Wednesday, July 11, 2012 8:12:31 AM Subject: 0.18 inclusion request Hi Justin, Could you please consider inclusions of trunk commits http://svn.apache.org/viewvc?view=revisionrevision=1360120 and http://svn.apache.org/viewvc?view=revisionrevision=1360121 into 0.18 ? These 2 commits are low risk bug fixes and improvements for recently added into Java Broker REST API and management web console. The second commit also adds 40 system tests for the Java Broker REST API. The functional changes are covered by these test and do not interfere with broker core functionality Kind Regards, Alex Rudyy - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
Re: Review Request: Add log entries for correlatable broker object life cycles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5616/ --- (Updated July 12, 2012, 2:38 a.m.) Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross. Changes --- This uses the new strategy described in QPID-4079: 1. It adds a new log category [Model] 2. INFO level logs for create/delete objects in the same user-specified names as management Events. 3. DEBUG level logs for create/delete objects using management keys. The #2 info logs are created by using QPID_LOG_CAT() statements near existing raiseEvent() calls. The #3 debug logs are new functions: * creates are logged by a new member function and is called near where management objects are created. * deletes are logged in resourceDestroy() with a common logging framework and a virtual call into the managed object to get the statistics. Much of the complication of previous reviews came from trying to correlate the user-specified names with management keys at inappropriate places in the source code. This diff is much simpler yet provides more correlation info. Note this prints debug info for creation/deletion of Session objects but sessions are not logged with raiseEvent. Description --- This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format: [Configuration] object event, where object is one of Connection, Session, Queue, Subscription, Exchange or Binding, and event is one of created or closed with connection also getting a setUser. The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log. This addresses bug QPID-4079. https://issues.apache.org/jira/browse/QPID-4079 Diffs (updated) - trunk/qpid/cpp/include/qpid/log/Statement.h 1360512 trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1360512 trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1360512 trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Link.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1360512 trunk/qpid/cpp/src/qpid/broker/System.cpp 1360512 trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1360512 trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1360512 trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512 trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1360512 Diff: https://reviews.apache.org/r/5616/diff/ Testing --- Passes cmake test and automake make check. Thanks, Chug Rolke