[jira] [Created] (QPID-4874) Alternate exchange unable to set from REST

2013-05-22 Thread Michal Zerola (JIRA)
Michal Zerola created QPID-4874:
---

 Summary: Alternate exchange unable to set from REST
 Key: QPID-4874
 URL: https://issues.apache.org/jira/browse/QPID-4874
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.20
Reporter: Michal Zerola
 Fix For: Future


This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the MessageProducer:

ADDR:test-exch; {create: always, node:{type: 
topic,x-declare:{alternate-exchange:'amq.fanout'}}}

The exchange 'test-exch' will then keep reference to the alternate exchange 
'amq.fanout'. However, listing the exchange using the 'curl' will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not ConfiguredObject (in this case it is 
FanoutExchange). I think the solution can be to return the alternate exchange 
name (and not an object) from the ExchangeAdapter.java as I illustrated in the 
attached patch.

Thank you,

Michal

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4874) Alternate exchange unable to set from REST

2013-05-22 Thread Michal Zerola (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michal Zerola updated QPID-4874:


Description: 
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the MessageProducer:

{{ADDR:test-exch; {create: always, node:{type: 
topic,x-declare:{alternate-exchange:'amq.fanout'}

The exchange 'test-exch' will then keep reference to the alternate exchange 
'amq.fanout'. However, listing the exchange using the 'curl' will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not ConfiguredObject (in this case it is 
FanoutExchange). I think the solution can be to return the alternate exchange 
name (and not an object) from the ExchangeAdapter.java as I illustrated in the 
attached patch.

Thank you,

Michal

  was:
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the MessageProducer:

ADDR:test-exch; {create: always, node:{type: 
topic,x-declare:{alternate-exchange:'amq.fanout'}}}

The exchange 'test-exch' will then keep reference to the alternate exchange 
'amq.fanout'. However, listing the exchange using the 'curl' will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not ConfiguredObject (in this case it is 
FanoutExchange). I think the solution can be to return the alternate exchange 
name (and not an object) from the ExchangeAdapter.java as I illustrated in the 
attached patch.

Thank you,

Michal


 Alternate exchange unable to set from REST
 --

 Key: QPID-4874
 URL: https://issues.apache.org/jira/browse/QPID-4874
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.20
Reporter: Michal Zerola
 Fix For: Future


 This is a follow-up on the discussion started on:
 http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E
 I haven't seen any JIRA covering this issue yet. Setting an alternate 
 exchange on the exchange is not possible now from the REST interface (e.g. 
 using curl / web management).
 The only way how one can set an alternate exchange is using the proper 
 address from the JMS client and passing it to the MessageProducer:
 {{ADDR:test-exch; {create: always, node:{type: 
 topic,x-declare:{alternate-exchange:'amq.fanout'}
 The exchange 'test-exch' will then keep reference to the alternate exchange 
 'amq.fanout'. However, listing the exchange using the 'curl' will fail 
 (produce endless output). I assume that the problem is caused by Json mapper, 
 having problems to write object which is not ConfiguredObject (in this case 
 it is FanoutExchange). I think the solution can be to return the alternate 
 exchange name (and not an object) from the ExchangeAdapter.java as I 
 illustrated in the attached patch.
 Thank you,
 Michal

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4874) Alternate exchange unable to set from REST

2013-05-22 Thread Michal Zerola (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michal Zerola updated QPID-4874:


Description: 
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the _MessageProducer_:

{noformat}
ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
{alternate-exchange:'amq.fanout'}}}
{noformat}

The exchange _test-exch_ will then keep reference to the alternate exchange 
_amq.fanout_. However, listing the exchange using the _curl_ will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not _ConfiguredObject_ (in this case it is 
_FanoutExchange_). I think the solution can be to return the alternate exchange 
name (and not an object) from the _ExchangeAdapter.java_ as I illustrated in 
the attached patch.

Thank you,

Michal

  was:
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the MessageProducer:

{{ADDR:test-exch; {create: always, node:{type: 
topic,x-declare:{alternate-exchange:'amq.fanout'}

The exchange 'test-exch' will then keep reference to the alternate exchange 
'amq.fanout'. However, listing the exchange using the 'curl' will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not ConfiguredObject (in this case it is 
FanoutExchange). I think the solution can be to return the alternate exchange 
name (and not an object) from the ExchangeAdapter.java as I illustrated in the 
attached patch.

Thank you,

Michal


 Alternate exchange unable to set from REST
 --

 Key: QPID-4874
 URL: https://issues.apache.org/jira/browse/QPID-4874
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.20
Reporter: Michal Zerola
 Fix For: Future


 This is a follow-up on the discussion started on:
 http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E
 I haven't seen any JIRA covering this issue yet. Setting an alternate 
 exchange on the exchange is not possible now from the REST interface (e.g. 
 using curl / web management).
 The only way how one can set an alternate exchange is using the proper 
 address from the JMS client and passing it to the _MessageProducer_:
 {noformat}
 ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
 {alternate-exchange:'amq.fanout'}}}
 {noformat}
 The exchange _test-exch_ will then keep reference to the alternate exchange 
 _amq.fanout_. However, listing the exchange using the _curl_ will fail 
 (produce endless output). I assume that the problem is caused by Json mapper, 
 having problems to write object which is not _ConfiguredObject_ (in this case 
 it is _FanoutExchange_). I think the solution can be to return the alternate 
 exchange name (and not an object) from the _ExchangeAdapter.java_ as I 
 illustrated in the attached patch.
 Thank you,
 Michal

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4874) Alternate exchange unable to set from REST

2013-05-22 Thread Michal Zerola (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michal Zerola updated QPID-4874:


Description: 
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the _MessageProducer_:

{noformat}
ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
{alternate-exchange:'amq.fanout'}}}
{noformat}

The exchange _test-exch_ will then keep reference to the alternate exchange 
_amq.fanout_. However, listing the exchange using the _curl_ will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not _ConfiguredObject_ (in this case it is 
_FanoutExchange_). This problem is reproducible by creating the exchange with 
alternate exchange reference using the address above and listing the 
exchange(s) with curl command. I think the solution can be to return the 
alternate exchange name (and not an object) from the _ExchangeAdapter.java_ as 
I illustrated in the attached patch.

Thank you,

Michal

  was:
This is a follow-up on the discussion started on:

http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E

I haven't seen any JIRA covering this issue yet. Setting an alternate exchange 
on the exchange is not possible now from the REST interface (e.g. using curl / 
web management).

The only way how one can set an alternate exchange is using the proper address 
from the JMS client and passing it to the _MessageProducer_:

{noformat}
ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
{alternate-exchange:'amq.fanout'}}}
{noformat}

The exchange _test-exch_ will then keep reference to the alternate exchange 
_amq.fanout_. However, listing the exchange using the _curl_ will fail (produce 
endless output). I assume that the problem is caused by Json mapper, having 
problems to write object which is not _ConfiguredObject_ (in this case it is 
_FanoutExchange_). I think the solution can be to return the alternate exchange 
name (and not an object) from the _ExchangeAdapter.java_ as I illustrated in 
the attached patch.

Thank you,

Michal


 Alternate exchange unable to set from REST
 --

 Key: QPID-4874
 URL: https://issues.apache.org/jira/browse/QPID-4874
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.20
Reporter: Michal Zerola
 Fix For: Future

 Attachments: alternate_exch_fix.patch


 This is a follow-up on the discussion started on:
 http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E
 I haven't seen any JIRA covering this issue yet. Setting an alternate 
 exchange on the exchange is not possible now from the REST interface (e.g. 
 using curl / web management).
 The only way how one can set an alternate exchange is using the proper 
 address from the JMS client and passing it to the _MessageProducer_:
 {noformat}
 ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
 {alternate-exchange:'amq.fanout'}}}
 {noformat}
 The exchange _test-exch_ will then keep reference to the alternate exchange 
 _amq.fanout_. However, listing the exchange using the _curl_ will fail 
 (produce endless output). I assume that the problem is caused by Json mapper, 
 having problems to write object which is not _ConfiguredObject_ (in this case 
 it is _FanoutExchange_). This problem is reproducible by creating the 
 exchange with alternate exchange reference using the address above and 
 listing the exchange(s) with curl command. I think the solution can be to 
 return the alternate exchange name (and not an object) from the 
 _ExchangeAdapter.java_ as I illustrated in the attached patch.
 Thank you,
 Michal

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4874) Alternate exchange unable to set from REST

2013-05-22 Thread Michal Zerola (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michal Zerola updated QPID-4874:


Attachment: alternate_exch_fix.patch

 Alternate exchange unable to set from REST
 --

 Key: QPID-4874
 URL: https://issues.apache.org/jira/browse/QPID-4874
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.20
Reporter: Michal Zerola
 Fix For: Future

 Attachments: alternate_exch_fix.patch


 This is a follow-up on the discussion started on:
 http://mail-archives.apache.org/mod_mbox/qpid-users/201303.mbox/%3CCAFitrpTiPo_yMhitGBM-1=QiW8xnKz3O2tqgSP3xbooDDC=y...@mail.gmail.com%3E
 I haven't seen any JIRA covering this issue yet. Setting an alternate 
 exchange on the exchange is not possible now from the REST interface (e.g. 
 using curl / web management).
 The only way how one can set an alternate exchange is using the proper 
 address from the JMS client and passing it to the _MessageProducer_:
 {noformat}
 ADDR:test-exch; {create: always, node:{type: topic,x-declare: 
 {alternate-exchange:'amq.fanout'}}}
 {noformat}
 The exchange _test-exch_ will then keep reference to the alternate exchange 
 _amq.fanout_. However, listing the exchange using the _curl_ will fail 
 (produce endless output). I assume that the problem is caused by Json mapper, 
 having problems to write object which is not _ConfiguredObject_ (in this case 
 it is _FanoutExchange_). This problem is reproducible by creating the 
 exchange with alternate exchange reference using the address above and 
 listing the exchange(s) with curl command. I think the solution can be to 
 return the alternate exchange name (and not an object) from the 
 _ExchangeAdapter.java_ as I illustrated in the attached patch.
 Thank you,
 Michal

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4875) [Java] The parsing logic for certificate subjects doesn't work properly in all cases

2013-05-22 Thread JAkub Scholz (JIRA)
JAkub Scholz created QPID-4875:
--

 Summary: [Java] The parsing logic for certificate subjects doesn't 
work properly in all cases
 Key: QPID-4875
 URL: https://issues.apache.org/jira/browse/QPID-4875
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker, Java Client, Java Common
Reporter: JAkub Scholz
Priority: Minor
 Fix For: Future


The Java code seems to contain two places where the certificate subjects are 
being parsed. One is used in the client:
{{common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLUtil.java}}
and the second is used in the broker:
{{broker/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java}}

Both are actually doing the same - extracting the CN and DC components from the 
subject and creating the username. It should be reconsidered whether we want to 
reuse the SSLUtil functionality from the common part of the code in the broker 
code as well.

However, a bigger problem is that the implementation in both places are not 
working correctly in all situations. One can very easily create a certificate 
with a subject / DN like this:
{{C=FR,O=SUNGARD,OU=CLEARVISION CN=GLKXV_GLKXVALBBDBGEN1}}
such certificate actually doesn't contain a CN. But both current 
implementations will still identify the CN as {{GLKXV_GLKXVALBBDBGEN1}} in the 
code. I would expect that this will happen only in very rare cases, but it 
should still be handled properly.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Alex Rudyy (JIRA)
Alex Rudyy created QPID-4876:


 Summary: Java Broker should not allow creation of Virtual Host 
from configuration file having no configuration for the host
 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy


It is possible to create a Virtual Host instance from configuration file having 
no configuration for the host.
Such Virtual Host has the Memory Message Store and derives all attributes from 
Broker.

It is very easy to make a mistake in a host name or to forget to include the 
host configuration into a configuration file. These mistakes should be detected 
by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4877) Consumers created using BURLs without exchanges fail with Cannot add bindings to the default exchange [error code 403: access refused]

2013-05-22 Thread Keith Wall (JIRA)
Keith Wall created QPID-4877:


 Summary: Consumers created using BURLs without exchanges fail with 
Cannot add bindings to the default exchange [error code 403: access refused]
 Key: QPID-4877
 URL: https://issues.apache.org/jira/browse/QPID-4877
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker, Java Client
Affects Versions: 0.23
Reporter: Keith Wall


Since QPID-4832, if a client, using a BURL that does not reference the 
exchange, attempts Session#createConsumer(), he will see a stack trace.

Example BURL:

destination.myqueue = BURL:direct:///myqueue/myqueue?routingkey='myqueue'

{noformat}
2013-05-22 11:27:29,255 DEBUG [IoReceiver - localhost/127.0.0.1:5672] 
[AMQConnection] exceptionReceived done by:IoReceiver - localhost/127.0.0.1:5672
org.apache.qpid.AMQChannelClosedException: Error: 
org.apache.qpid.AMQSecurityException: Cannot add bindings to the default 
exchange [error code 403: access refused] [error code 504: channel error]
at 
org.apache.qpid.client.handler.ChannelCloseMethodHandler.methodReceived(ChannelCloseMethodHandler.java:97)
at 
org.apache.qpid.client.handler.ClientMethodDispatcherImpl.dispatchChannelClose(ClientMethodDispatcherImpl.java:164)
at 
org.apache.qpid.framing.amqp_0_9.ChannelCloseBodyImpl.execute(ChannelCloseBodyImpl.java:137)
at 
org.apache.qpid.client.state.AMQStateManager.methodReceived(AMQStateManager.java:114)
at 
org.apache.qpid.client.protocol.AMQProtocolHandler.methodBodyReceived(AMQProtocolHandler.java:520)
at 
org.apache.qpid.client.protocol.AMQProtocolSession.methodFrameReceived(AMQProtocolSession.java:462)
at 
org.apache.qpid.framing.AMQMethodBodyImpl.handle(AMQMethodBodyImpl.java:97)
at 
org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:477)
at 
org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:1)
at 
org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:161)
at java.lang.Thread.run(Thread.java:662)
{noformat}

The user can work around the issue by including the exchange name within the 
BURL.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4877) Consumers created using BURLs without exchanges fail with Cannot add bindings to the default exchange [error code 403: access refused]

2013-05-22 Thread Keith Wall (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664007#comment-13664007
 ] 

Keith Wall commented on QPID-4877:
--

For 0-8..0-9-1:

Qpid client ought not to try to queue.bind to the default (nameless) exchange.

For the sake of backward compatibility with older clients, the Java Broker 
should silently ignore queue.binds where the routing key matches the queue name 
and there are no binding arguments.  Other cases should continue to cause the 
exception as this would constitute a misuse of the default exchange.

For 0-10:
(Not yet examined)




 Consumers created using BURLs without exchanges fail with Cannot add 
 bindings to the default exchange [error code 403: access refused]
 

 Key: QPID-4877
 URL: https://issues.apache.org/jira/browse/QPID-4877
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker, Java Client
Affects Versions: 0.23
Reporter: Keith Wall

 Since QPID-4832, if a client, using a BURL that does not reference the 
 exchange, attempts Session#createConsumer(), he will see a stack trace.
 Example BURL:
 destination.myqueue = BURL:direct:///myqueue/myqueue?routingkey='myqueue'
 {noformat}
 2013-05-22 11:27:29,255 DEBUG [IoReceiver - localhost/127.0.0.1:5672] 
 [AMQConnection] exceptionReceived done by:IoReceiver - 
 localhost/127.0.0.1:5672
 org.apache.qpid.AMQChannelClosedException: Error: 
 org.apache.qpid.AMQSecurityException: Cannot add bindings to the default 
 exchange [error code 403: access refused] [error code 504: channel error]
   at 
 org.apache.qpid.client.handler.ChannelCloseMethodHandler.methodReceived(ChannelCloseMethodHandler.java:97)
   at 
 org.apache.qpid.client.handler.ClientMethodDispatcherImpl.dispatchChannelClose(ClientMethodDispatcherImpl.java:164)
   at 
 org.apache.qpid.framing.amqp_0_9.ChannelCloseBodyImpl.execute(ChannelCloseBodyImpl.java:137)
   at 
 org.apache.qpid.client.state.AMQStateManager.methodReceived(AMQStateManager.java:114)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.methodBodyReceived(AMQProtocolHandler.java:520)
   at 
 org.apache.qpid.client.protocol.AMQProtocolSession.methodFrameReceived(AMQProtocolSession.java:462)
   at 
 org.apache.qpid.framing.AMQMethodBodyImpl.handle(AMQMethodBodyImpl.java:97)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:477)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:1)
   at 
 org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:161)
   at java.lang.Thread.run(Thread.java:662)
 {noformat}
 The user can work around the issue by including the exchange name within the 
 BURL.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4877) Consumers created using BURLs without named exchange fail with Cannot add bindings to the default exchange [error code 403: access refused]

2013-05-22 Thread Keith Wall (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4877?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Keith Wall updated QPID-4877:
-

Summary: Consumers created using BURLs without named exchange fail with 
Cannot add bindings to the default exchange [error code 403: access refused]  
(was: Consumers created using BURLs without exchanges fail with Cannot add 
bindings to the default exchange [error code 403: access refused])

 Consumers created using BURLs without named exchange fail with Cannot add 
 bindings to the default exchange [error code 403: access refused]
 -

 Key: QPID-4877
 URL: https://issues.apache.org/jira/browse/QPID-4877
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker, Java Client
Affects Versions: 0.23
Reporter: Keith Wall

 Since QPID-4832, if a client, using a BURL that does not reference the 
 exchange, attempts Session#createConsumer(), he will see a stack trace.
 Example BURL:
 destination.myqueue = BURL:direct:///myqueue/myqueue?routingkey='myqueue'
 {noformat}
 2013-05-22 11:27:29,255 DEBUG [IoReceiver - localhost/127.0.0.1:5672] 
 [AMQConnection] exceptionReceived done by:IoReceiver - 
 localhost/127.0.0.1:5672
 org.apache.qpid.AMQChannelClosedException: Error: 
 org.apache.qpid.AMQSecurityException: Cannot add bindings to the default 
 exchange [error code 403: access refused] [error code 504: channel error]
   at 
 org.apache.qpid.client.handler.ChannelCloseMethodHandler.methodReceived(ChannelCloseMethodHandler.java:97)
   at 
 org.apache.qpid.client.handler.ClientMethodDispatcherImpl.dispatchChannelClose(ClientMethodDispatcherImpl.java:164)
   at 
 org.apache.qpid.framing.amqp_0_9.ChannelCloseBodyImpl.execute(ChannelCloseBodyImpl.java:137)
   at 
 org.apache.qpid.client.state.AMQStateManager.methodReceived(AMQStateManager.java:114)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.methodBodyReceived(AMQProtocolHandler.java:520)
   at 
 org.apache.qpid.client.protocol.AMQProtocolSession.methodFrameReceived(AMQProtocolSession.java:462)
   at 
 org.apache.qpid.framing.AMQMethodBodyImpl.handle(AMQMethodBodyImpl.java:97)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:477)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:1)
   at 
 org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:161)
   at java.lang.Thread.run(Thread.java:662)
 {noformat}
 The user can work around the issue by including the exchange name within the 
 BURL.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4877) Consumers created using BURLs without exchanges fail with Cannot add bindings to the default exchange [error code 403: access refused]

2013-05-22 Thread Keith Wall (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664007#comment-13664007
 ] 

Keith Wall edited comment on QPID-4877 at 5/22/13 11:06 AM:



Qpid client ought not to try to queue.bind to the default (nameless) exchange.

For the sake of backward compatibility with older clients, the Java Broker 
should silently ignore queue.binds where the routing key matches the queue name 
and there are no binding arguments.  Other cases should continue to cause the 
exception as this would constitute a misuse of the default exchange.

I have not verified whether ADDRs suffer an analogous problem.



  was (Author: k-wall):
For 0-8..0-9-1:

Qpid client ought not to try to queue.bind to the default (nameless) exchange.

For the sake of backward compatibility with older clients, the Java Broker 
should silently ignore queue.binds where the routing key matches the queue name 
and there are no binding arguments.  Other cases should continue to cause the 
exception as this would constitute a misuse of the default exchange.

For 0-10:
(Not yet examined)



  
 Consumers created using BURLs without exchanges fail with Cannot add 
 bindings to the default exchange [error code 403: access refused]
 

 Key: QPID-4877
 URL: https://issues.apache.org/jira/browse/QPID-4877
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker, Java Client
Affects Versions: 0.23
Reporter: Keith Wall

 Since QPID-4832, if a client, using a BURL that does not reference the 
 exchange, attempts Session#createConsumer(), he will see a stack trace.
 Example BURL:
 destination.myqueue = BURL:direct:///myqueue/myqueue?routingkey='myqueue'
 {noformat}
 2013-05-22 11:27:29,255 DEBUG [IoReceiver - localhost/127.0.0.1:5672] 
 [AMQConnection] exceptionReceived done by:IoReceiver - 
 localhost/127.0.0.1:5672
 org.apache.qpid.AMQChannelClosedException: Error: 
 org.apache.qpid.AMQSecurityException: Cannot add bindings to the default 
 exchange [error code 403: access refused] [error code 504: channel error]
   at 
 org.apache.qpid.client.handler.ChannelCloseMethodHandler.methodReceived(ChannelCloseMethodHandler.java:97)
   at 
 org.apache.qpid.client.handler.ClientMethodDispatcherImpl.dispatchChannelClose(ClientMethodDispatcherImpl.java:164)
   at 
 org.apache.qpid.framing.amqp_0_9.ChannelCloseBodyImpl.execute(ChannelCloseBodyImpl.java:137)
   at 
 org.apache.qpid.client.state.AMQStateManager.methodReceived(AMQStateManager.java:114)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.methodBodyReceived(AMQProtocolHandler.java:520)
   at 
 org.apache.qpid.client.protocol.AMQProtocolSession.methodFrameReceived(AMQProtocolSession.java:462)
   at 
 org.apache.qpid.framing.AMQMethodBodyImpl.handle(AMQMethodBodyImpl.java:97)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:477)
   at 
 org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:1)
   at 
 org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:161)
   at java.lang.Thread.run(Thread.java:662)
 {noformat}
 The user can work around the issue by including the exchange name within the 
 BURL.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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] [Assigned] (QPID-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Alex Rudyy (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4876?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alex Rudyy reassigned QPID-4876:


Assignee: Alex Rudyy

 Java Broker should not allow creation of Virtual Host from configuration file 
 having no configuration for the host
 --

 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy
Assignee: Alex Rudyy

 It is possible to create a Virtual Host instance from configuration file 
 having no configuration for the host.
 Such Virtual Host has the Memory Message Store and derives all attributes 
 from Broker.
 It is very easy to make a mistake in a host name or to forget to include the 
 host configuration into a configuration file. These mistakes should be 
 detected by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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] [Assigned] (QPID-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rob Godfrey (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Rob Godfrey reassigned QPID-4873:
-

Assignee: Rajith Attapattu

Thanks for the patch Helen!

Rajith - you are probably the most familiar with this code - can you see any 
issues with this patch?

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties defined, but no bindings.
 Overall: Originally, each queue took up about 2760B for us; with these 
 optimizations, that goes down to 1024B, saving 63% per queue for us.
 We'd appreciate feedback on these changes and whether we are making any 
 incorrect assumptions. I've also added relevant tests to AMQDestinationTest 
 but am not sure if that's the best place. Thanks a lot!

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: 

[jira] [Closed] (QPID-3609) Refactor implementation of the default exchanges and functionality to bind queue to default exchange

2013-05-22 Thread Keith Wall (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-3609?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Keith Wall closed QPID-3609.


Resolution: Duplicate

 Refactor implementation of the default exchanges  and functionality to bind 
 queue to  default exchange
 --

 Key: QPID-3609
 URL: https://issues.apache.org/jira/browse/QPID-3609
 Project: Qpid
  Issue Type: Sub-task
  Components: Java Broker
Affects Versions: 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12, 0.13, 0.14, 
 0.15
Reporter: Alex Rudyy

 The implementation of the default exchange should be updated such that it is 
 not just a regular direct exchange (e.g. to prevent it having an MBean or at 
 least a custom MBean, since no operations should actually be able to be 
 performed on it given it is to just be a direct mapping to the available 
 queues), and its bindings should be handled directly through the Queue 
 factory to ensure they are applied uniformly across the broker.
 Additionally, the current implementation of the default exchange has a name 
 default when it should really be nameless, and it is possible to delete 
 it (and amq.* exchanges) via the JMX interface as a result which it shouldnt 
 be.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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.22 release update - proposed final RC4

2013-05-22 Thread Oleksandr Rudyy
Hi Justin,

I am wondering whether you have already cut RC5?

If not, I would like to request the inclusion of changes made in revision
http://svn.apache.org/r1485163 fixing QPID-4876. It is a one line change
which does not effect core broker functionality but it could potentially
save a lot of cursing and  user frustration when making mistakes on adding
of Virtual Host into Java Broker dynamically :)

If RC5 is already cut then it is fine  to not include the changes into 0.22.

Kind Regards,
Alex



On 20 May 2013 14:08, Robbie Gemmell robbie.gemm...@gmail.com wrote:

 Done.

 (As I'm sure is obvious from the JIRA and commit traffic, I also merged
 some other small bug fixes and improvements in keeping with the previous
 'break it you fix it' discussion)

 Robbie

 On 20 May 2013 11:24, Justin Ross justin.r...@gmail.com wrote:

  Please go ahead.  If we end up being ready, I'll produce the new RC later
  today.
 
  Justin
 
  On Sun, May 19, 2013 at 9:10 PM, Robbie Gemmell
  robbie.gemm...@gmail.com wrote:
   In case it isn't clear that was me effectively asking permission, as
 I'd
   rather not merge code changes to the branch unless they are going to be
   included.
  
   Robbie
  
   On 17 May 2013 16:56, Robbie Gemmell robbie.gemm...@gmail.com wrote:
  
   Hi Justin,
  
   In some final testing of the Java broker we came across an issue we
 feel
   warrants blocker status, and so would like to request another RC be
 cut
  on
   Monday before calling the vote in order to allow including the fix in
  the
   release.
  
   http://issues.apache.org/jira/browse/QPID-4858
   http://svn.apache.org/r1483866
  
   There are more specifics on the JIRA, but the short story is that due
 to
   changes made in this development cycle it became possible to configure
  the
   brokers HTTP management port(s) in an ambiguous way that suggested SSL
  was
   in use on the port when it was not, a situation we would obviously
 like
  to
   prevent for reasons.
  
   The change is fairly simple and involves removal of some inconsistency
  in
   configuration between the HTTP and non-HTTP ports, removing the
  ambiguity
   that lead to the issue. Aside from the obvious security-related
 benefit,
   making the change in this release would also be beneficial to avoid a
  need
   for explicit handling of the configuration change during upgrades to
  future
   releases.
  
   Robbie
  
   On 16 May 2013 22:46, Justin Ross jr...@apache.org wrote:
  
   Hi, everyone.  Here is RC4 from revision 1483543:
  
 http://people.apache.org/~jross/qpid-0.22-rc4/
 http://people.apache.org/~jross/qpid-0.22-rc4-testing.txt
  
   My testing on Fedora 16 x86-64 produced no failures.  Once again,
   thanks to all those who have tested previous RCs and reported the
   outcome.  Mick has posted to the list about some low-frequency
   failures in repeated test runs.  Those are important, but they are
 not
   at this point considered blockers for the release.
  
   My plan now is to let RC4 settle for a few days and wait for any
 signs
   of trouble.  If all goes well, I'll start the release vote on Monday
   next week and close it that Friday.
  
   This release candidate is signed.  The bits in RC4, if approved for
   release, will be the GA bits.
  
   Thanks!
   Justin
  
   ---
   0.22 release page: https://cwiki.apache.org/qpid/022-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
 
 



Re: Review Request: Address deadlock btw _messageDeliveryLock and _failoverMutex

2013-05-22 Thread Robbie Gemmell


 On May 13, 2013, 3:22 p.m., Robbie Gemmell wrote:
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer.java,
   lines 744-745
  https://reviews.apache.org/r/10738/diff/2/?file=288993#file288993line744
 
  What impact does dropping the message on the floor here have on the 
  client?
  
  Earlier points in the call hierarchy seem to make effort to do other 
  things with the message when detecting session/consumer close, so is there 
  any impact from not doing so? E.g a message getting stuck acquired for a 
  now-closed consumer?
  
  Does any particular attention need paid to the overridden 0-10 specific 
  version of this method?
 
 rajith attapattu wrote:
 The message will be dropped if a consumer (or session is closed or 
 closing).
 When a consumer is closed, any messages acquired but not acknowledged 
 should be be made available to another consumer by the broker.
 These messages will be marked redelivered.
 Is this the same situation for 0-8/0-9 ?
 
 The situation is the same as a consumer closing with a bunch of unacked 
 messages when in CLIENT_ACK mode.
 Alternatively we could reject the message (release in 0-10 terms). But I 
 don't think this is required, given that we will be closing the consumer 
 anyways.
 
 
 Earlier points in the call hierarchy seem to make effort to do other 
 things with the message when detecting session/consumer close
 By other things are you referring to a reject? In 0-10 AFIK you don't 
 need to do it. 
 The situation is the same as a consumer closing with a bunch of unacked 
 messages when in CLIENT_ACK mode.
 
 Does any particular attention need paid to the overridden 0-10 specific 
 version of this method?
 IMO adding if (!(isClosed() || isClosing())) is required to prevent the 
 0-10 specific method from issuing credit (and receiving more messages, that 
 will eventually get released).
 There is a chance where the consumer could be marked closed, but not yet 
 reached the point of sending a cancel, by the time messageFlow() is called.
 The above check should prevent it.
 
 Robbie Gemmell wrote:
 Did you mean race condition (instead of deadlock)?
 
 I really meant deadlock. One of the uses of _messageDeliveryLock being 
 removed was added as part of the fix for a deadlock 
 (https://issues.apache.org/jira/browse/QPID-3911) and so I wonder what effect 
 removing it again will have. It may be the other changes mean it isn't a 
 problem, but I think it needs to be closely considered. 
 
 The message will be dropped if a consumer (or session is closed or 
 closing).
 When a consumer is closed, any messages acquired but not acknowledged 
 should be be made available to another consumer by the broker.
 These messages will be marked redelivered.
 Is this the same situation for 0-8/0-9 ?
 
 That isn't actually the case, transferred messages are the responsibility 
 of the session and remain acquired after consumer close until such point as 
 the session explicitly does something with them. I believe this is true of 
 0-8/9/91 as well, and probably explains the origin of the reject/release code 
 I referred to earlier in the call tree than the changes would now allow the 
 message to be silently dropped.
 
 From the 0-10 spec: Canceling a subscription MUST NOT affect pending 
 transfers. A transfer made prior to canceling transfers to the destination 
 MUST be able to be accepted, released, acquired, or rejected after the 
 subscription is canceled.
 
 rajith attapattu wrote:
 Indeed, Gordon mentioned that to me and the 2nd patch (the one before I 
 did today) takes care of rejecting messages from the consumer. We don't need 
 to do the same when the session is closing, as when the session ends, the any 
 unacked messages are put back on the queue.
 
 rajith attapattu wrote:
 As for QPID-3911,
 There is a deadlock, albeit a bit different (involving the same locks) 
 from QPID-3911, that do happen in similar circumstances.
 However this deadlock appears to manifest with or without this patch, 
 which leads me to believe that _messageDeliveryLock is not the right solution 
 for QPID-3911.
 Sadly the solution for QPID-3911 made it worse as there are at least two 
 distinct cases of deadlocks involving _messageDeliveryLock.
 1. Btw _lock and _messageDeliveryLock
 2. Btw _messageDeliveryLock and _failoverMutex.
 
 We definitely need to find a solution for the deadlocks (at least 3 
 cases) btw failoverMutex and _lock (in AMQSession), which seems to be the 
 root of all evil :)
 We might have to drop one lock (most likely _lock) and see if we can 
 provide an alternative strategy to guarantee correct behaviour.

 
 rajith attapattu wrote:
 Sorry I meant dopping _failoverMutex (not _lock in AMQSession).
 It might also be an 

[jira] [Commented] (QPID-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Justin Ross (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4876?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664043#comment-13664043
 ] 

Justin Ross commented on QPID-4876:
---

Approved for 0.22.

 Java Broker should not allow creation of Virtual Host from configuration file 
 having no configuration for the host
 --

 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy
Assignee: Alex Rudyy

 It is possible to create a Virtual Host instance from configuration file 
 having no configuration for the host.
 Such Virtual Host has the Memory Message Store and derives all attributes 
 from Broker.
 It is very easy to make a mistake in a host name or to forget to include the 
 host configuration into a configuration file. These mistakes should be 
 detected by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4869) Small fix: Perl shouldn't be required to build Qpid

2013-05-22 Thread Justin Ross (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664046#comment-13664046
 ] 

Justin Ross commented on QPID-4869:
---

Approved for 0.22.

 Small fix: Perl shouldn't be required to build Qpid
 ---

 Key: QPID-4869
 URL: https://issues.apache.org/jira/browse/QPID-4869
 Project: Qpid
  Issue Type: Bug
  Components: Build Tools
Affects Versions: 0.22
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
 Fix For: 0.23


 A previous change to the CMake build files (QPID-4724) made building the Perl 
 libraries required rather than option by causing the build to fail if it 
 found Perl but not the Perl development libraries.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4869) Small fix: Perl shouldn't be required to build Qpid

2013-05-22 Thread Darryl L. Pierce (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4869?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darryl L. Pierce updated QPID-4869:
---

Fix Version/s: (was: 0.23)
   0.22

 Small fix: Perl shouldn't be required to build Qpid
 ---

 Key: QPID-4869
 URL: https://issues.apache.org/jira/browse/QPID-4869
 Project: Qpid
  Issue Type: Bug
  Components: Build Tools
Affects Versions: 0.22
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
 Fix For: 0.22


 A previous change to the CMake build files (QPID-4724) made building the Perl 
 libraries required rather than option by causing the build to fail if it 
 found Perl but not the Perl development libraries.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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.22 RFI: QPID-4869

2013-05-22 Thread Darryl L. Pierce
On Tue, May 21, 2013 at 04:01:58PM -0400, Darryl L. Pierce wrote:
 On Mon, May 20, 2013 at 11:54:40AM -0400, Darryl L. Pierce wrote:
  QPID-4869: Small fix: Perl shouldn't be required to build Qpid
  
  In QPID-4724 a change included made building Perl required instead of
  option by failing the CMake generation phase if Perl was found but not
  the Perl development files.
 
 I verified that fixing this hasn't changed any of the functionality.
 
 Looking back at when the change was introduced, it was in commit
 1cabe1b2 (20 May) where I removed the Swig 1.3.32 minimum requirement.
 Building without the Perl development files (uninstalling perl-devel on
 Fedora) Cmake doesn't error out and the build completes without the Perl
 bindings, as expected. Installing perl-devel and running the build
 produces the Perl bindings.

This is now on 0.22.

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



pgpR85HLqI3eg.pgp
Description: PGP signature


Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Gordon Sim

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/
---

Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
Ernie Allen.


Description
---

This attempts to reduce the performance impact of adding annotations (headers 
in 0-10) to a message as it passes through the broker. At present this uses a 
copy-on-write approaches that clones the header body, modifies it and then uses 
that for encode or for writing to the wire. This change avoids the full clone, 
by encoding the original header and while doing so adjusting for any additions.

Before:

$ qpid-cpp-benchmark --repeat 5 --create-option 
node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
send-tp recv-tp l-min   l-max   l-avg   total-tp
30720   30671   0.3244.03   18.28   30064
31778   31729   0.2254.36   16.77   31068
31664   31637   0.3969.50   22.11   31014
31303   31301   0.3343.97   17.32   30737
31577   31403   0.3337.73   15.59   30785

$ qpid-cpp-benchmark --repeat 5
send-tp recv-tp l-min   l-max   l-avg   total-tp
47138   47120   0.1329.02   9.3545870
46299   46156   0.1325.24   8.3344803
45910   45793   0.1353.53   10.81   45300
46607   46388   0.1629.70   9.2145245
46905   46766   0.1631.43   9.6245776

After:

$ qpid-cpp-benchmark --repeat 5  --create-option 
node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
send-tp recv-tp l-min   l-max   l-avg   total-tp
44418   44376   0.1532.29   10.24   43191
44127   44045   0.1432.39   11.71   42762
44115   43882   0.2037.27   13.42   42633
43444   43431   0.1536.99   12.00   42147
44573   44265   0.1734.87   11.81   42898

$ qpid-cpp-benchmark --repeat 5 
send-tp recv-tp l-min   l-max   l-avg   total-tp
47178   47061   0.1627.23   8.4645644
46291   46291   0.1425.92   9.5144812
47704   47525   0.1732.19   9.0645998
47771   47745   0.1721.34   8.6046271
43950   43931   0.1352.13   10.60   43118

So, there is still some impact, but I believe it has been reduced. There are 
some further things to try (caching encoded header as originally received to 
avoid re-encoding it).


Diffs
-

  /trunk/qpid/cpp/src/CMakeLists.txt 1485001 
  /trunk/qpid/cpp/src/Makefile.am 1485001 
  /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 
  /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1485001 
  /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1485001 
  /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.h 1485001 
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/framing/AMQFrame.h 1485001 
  /trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp 1485001 
  /trunk/qpid/cpp/src/qpid/framing/FrameSet.h 1485001 
  /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/11329/diff/


Testing
---

make check/test passes both on autotools and cmake builds


Thanks,

Gordon Sim



[jira] [Commented] (QPID-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Alex Rudyy (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4876?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664134#comment-13664134
 ] 

Alex Rudyy commented on QPID-4876:
--

The fix (r1485163) was merged into 0.22 branch in revision 
http://svn.apache.org/r1485215

 Java Broker should not allow creation of Virtual Host from configuration file 
 having no configuration for the host
 --

 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy
Assignee: Alex Rudyy

 It is possible to create a Virtual Host instance from configuration file 
 having no configuration for the host.
 Such Virtual Host has the Memory Message Store and derives all attributes 
 from Broker.
 It is very easy to make a mistake in a host name or to forget to include the 
 host configuration into a configuration file. These mistakes should be 
 detected by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Alex Rudyy (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4876?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alex Rudyy updated QPID-4876:
-

Fix Version/s: 0.22

 Java Broker should not allow creation of Virtual Host from configuration file 
 having no configuration for the host
 --

 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy
Assignee: Alex Rudyy
 Fix For: 0.22


 It is possible to create a Virtual Host instance from configuration file 
 having no configuration for the host.
 Such Virtual Host has the Memory Message Store and derives all attributes 
 from Broker.
 It is very easy to make a mistake in a host name or to forget to include the 
 host configuration into a configuration file. These mistakes should be 
 detected by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4876) Java Broker should not allow creation of Virtual Host from configuration file having no configuration for the host

2013-05-22 Thread Alex Rudyy (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4876?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alex Rudyy resolved QPID-4876.
--

Resolution: Fixed

 Java Broker should not allow creation of Virtual Host from configuration file 
 having no configuration for the host
 --

 Key: QPID-4876
 URL: https://issues.apache.org/jira/browse/QPID-4876
 Project: Qpid
  Issue Type: Bug
  Components: Java Broker
Affects Versions: 0.22
Reporter: Alex Rudyy
Assignee: Alex Rudyy
 Fix For: 0.22


 It is possible to create a Virtual Host instance from configuration file 
 having no configuration for the host.
 Such Virtual Host has the Memory Message Store and derives all attributes 
 from Broker.
 It is very easy to make a mistake in a host name or to forget to include the 
 host configuration into a configuration file. These mistakes should be 
 detected by the Broker and corresponding Exception have to be thrown.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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.22 release update - proposed final RC4

2013-05-22 Thread Oleksandr Rudyy
Justin,

Thanks for approval.
The fix (r1485163) was merged into 0.22 branch.

Kind Regards,
Alex


On 22 May 2013 13:09, Oleksandr Rudyy oru...@gmail.com wrote:

 Hi Justin,

 I am wondering whether you have already cut RC5?

 If not, I would like to request the inclusion of changes made in revision
 http://svn.apache.org/r1485163 fixing QPID-4876. It is a one line change
 which does not effect core broker functionality but it could potentially
 save a lot of cursing and  user frustration when making mistakes on adding
 of Virtual Host into Java Broker dynamically :)

 If RC5 is already cut then it is fine  to not include the changes into
 0.22.

 Kind Regards,
 Alex



 On 20 May 2013 14:08, Robbie Gemmell robbie.gemm...@gmail.com wrote:

 Done.

 (As I'm sure is obvious from the JIRA and commit traffic, I also merged
 some other small bug fixes and improvements in keeping with the previous
 'break it you fix it' discussion)

 Robbie

 On 20 May 2013 11:24, Justin Ross justin.r...@gmail.com wrote:

  Please go ahead.  If we end up being ready, I'll produce the new RC
 later
  today.
 
  Justin
 
  On Sun, May 19, 2013 at 9:10 PM, Robbie Gemmell
  robbie.gemm...@gmail.com wrote:
   In case it isn't clear that was me effectively asking permission, as
 I'd
   rather not merge code changes to the branch unless they are going to
 be
   included.
  
   Robbie
  
   On 17 May 2013 16:56, Robbie Gemmell robbie.gemm...@gmail.com
 wrote:
  
   Hi Justin,
  
   In some final testing of the Java broker we came across an issue we
 feel
   warrants blocker status, and so would like to request another RC be
 cut
  on
   Monday before calling the vote in order to allow including the fix in
  the
   release.
  
   http://issues.apache.org/jira/browse/QPID-4858
   http://svn.apache.org/r1483866
  
   There are more specifics on the JIRA, but the short story is that
 due to
   changes made in this development cycle it became possible to
 configure
  the
   brokers HTTP management port(s) in an ambiguous way that suggested
 SSL
  was
   in use on the port when it was not, a situation we would obviously
 like
  to
   prevent for reasons.
  
   The change is fairly simple and involves removal of some
 inconsistency
  in
   configuration between the HTTP and non-HTTP ports, removing the
  ambiguity
   that lead to the issue. Aside from the obvious security-related
 benefit,
   making the change in this release would also be beneficial to avoid a
  need
   for explicit handling of the configuration change during upgrades to
  future
   releases.
  
   Robbie
  
   On 16 May 2013 22:46, Justin Ross jr...@apache.org wrote:
  
   Hi, everyone.  Here is RC4 from revision 1483543:
  
 http://people.apache.org/~jross/qpid-0.22-rc4/
 http://people.apache.org/~jross/qpid-0.22-rc4-testing.txt
  
   My testing on Fedora 16 x86-64 produced no failures.  Once again,
   thanks to all those who have tested previous RCs and reported the
   outcome.  Mick has posted to the list about some low-frequency
   failures in repeated test runs.  Those are important, but they are
 not
   at this point considered blockers for the release.
  
   My plan now is to let RC4 settle for a few days and wait for any
 signs
   of trouble.  If all goes well, I'll start the release vote on Monday
   next week and close it that Friday.
  
   This release candidate is signed.  The bits in RC4, if approved for
   release, will be the GA bits.
  
   Thanks!
   Justin
  
   ---
   0.22 release page: https://cwiki.apache.org/qpid/022-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
 
 





Re: Review Request: Python client: use same arguments when re-trying SSL socket read and write

2013-05-22 Thread Alan Conway

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11319/#review20895
---

Ship it!


Logic looks ok, comments are style only. I can't think of a simpler solution.


/trunk/qpid/python/qpid/messaging/transports.py
https://reviews.apache.org/r/11319/#comment43022

I found the logic hard to follow. It might be clearer if you put these 2 
lines in the try: block so the non-error path is easier to see.



/trunk/qpid/python/qpid/messaging/transports.py
https://reviews.apache.org/r/11319/#comment43023

Same comment as as for send


- Alan Conway


On May 21, 2013, 8:40 p.m., Kenneth Giusti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11319/
 ---
 
 (Updated May 21, 2013, 8:40 p.m.)
 
 
 Review request for qpid, Alan Conway and Rafael Schloming.
 
 
 Description
 ---
 
 This is a hack (yes, a _HACK_) to work around the behavior of python's SSL 
 implementation.
 
 When the SSL socket cannot complete a read() or write() operation without 
 performing it's opposite (a write() or read() operation), it will throw an 
 SSL_ERROR_WANT_READ/WRITE exception.  To resume the failed operation, the 
 code must re-invoke the read() or write() with _exactly_ the same parameters 
 as were passed when the call threw the exception.
 
 This is due to the underlying OpenSSL implementation requirements.
 
 Originally, when writing we passed the string buffer that holds output data.  
 This string buffer can be appended with new write data between the successive 
 calls to write(), which will cause the underlying pointer to change.  This 
 fix tracks the original buffer passed to the failing call, and retries the 
 call using that.
 
 Originally, each call to recv() would return a new buffer.  This patch 
 introduces a bytearray-based buffer that can be preserved across a read retry.
 
 Not 100% enamored by this patch, if anyone has a better approach I'm all 
 ears
 
 
 This addresses bug qpid-4872.
 https://issues.apache.org/jira/browse/qpid-4872
 
 
 Diffs
 -
 
   /trunk/qpid/python/qpid/messaging/transports.py 1484491 
 
 Diff: https://reviews.apache.org/r/11319/diff/
 
 
 Testing
 ---
 
 Reproduced the failure both with and without the patch.  No exceptions thrown 
 with the patch in place.
 
 
 Thanks,
 
 Kenneth Giusti
 




Re: Review Request: Python client: use same arguments when re-trying SSL socket read and write

2013-05-22 Thread Rafael Schloming

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11319/#review20898
---

Ship it!


Ship It!

- Rafael Schloming


On May 21, 2013, 8:40 p.m., Kenneth Giusti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11319/
 ---
 
 (Updated May 21, 2013, 8:40 p.m.)
 
 
 Review request for qpid, Alan Conway and Rafael Schloming.
 
 
 Description
 ---
 
 This is a hack (yes, a _HACK_) to work around the behavior of python's SSL 
 implementation.
 
 When the SSL socket cannot complete a read() or write() operation without 
 performing it's opposite (a write() or read() operation), it will throw an 
 SSL_ERROR_WANT_READ/WRITE exception.  To resume the failed operation, the 
 code must re-invoke the read() or write() with _exactly_ the same parameters 
 as were passed when the call threw the exception.
 
 This is due to the underlying OpenSSL implementation requirements.
 
 Originally, when writing we passed the string buffer that holds output data.  
 This string buffer can be appended with new write data between the successive 
 calls to write(), which will cause the underlying pointer to change.  This 
 fix tracks the original buffer passed to the failing call, and retries the 
 call using that.
 
 Originally, each call to recv() would return a new buffer.  This patch 
 introduces a bytearray-based buffer that can be preserved across a read retry.
 
 Not 100% enamored by this patch, if anyone has a better approach I'm all 
 ears
 
 
 This addresses bug qpid-4872.
 https://issues.apache.org/jira/browse/qpid-4872
 
 
 Diffs
 -
 
   /trunk/qpid/python/qpid/messaging/transports.py 1484491 
 
 Diff: https://reviews.apache.org/r/11319/diff/
 
 
 Testing
 ---
 
 Reproduced the failure both with and without the patch.  No exceptions thrown 
 with the patch in place.
 
 
 Thanks,
 
 Kenneth Giusti
 




Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Alan Conway

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20896
---


It probably makes little difference but why did you use 
qpid.queue_msg_sequence':1 before and qpid.queue_msg_sequence':'seqno' after in 
your before-after comparison?

I think MessageAnnotations should be a class in its own right to encapsulate 
the encoding logic.

There is duplication of framing and encoding logic, could existing encoding 
logic be refactored rather than duplicated?


/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43024

typo: buy - by



/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43025

Manual step is error prone, can you do it automatically when the persistent 
context is requested?



/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43026

Presumably you split the annotations map into string/int because Variant 
overhead was a performance problem. Should comment that here so somebody 
doesn't later go hey, I could just use a VariantMap here



/trunk/qpid/cpp/src/qpid/broker/Message.cpp
https://reviews.apache.org/r/11329/#comment43027

What's a CharSequence?



/trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp
https://reviews.apache.org/r/11329/#comment43029

This is a lot of repetitive code, but I'm not sure how to do it more neatly.



/trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp
https://reviews.apache.org/r/11329/#comment43032

There is a lot of gritty encoding detail in Message for handling 
annotations. I suggest adding a MessageAnnotations class to hold this kind of 
detail to keep Message clean.



/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
https://reviews.apache.org/r/11329/#comment43033

Could this be encapsulated in a MessageAnnotations class?



/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
https://reviews.apache.org/r/11329/#comment43034

Why do we have to create all this encoding detail? Don't we already have 
coded that encodes properties etc?


- Alan Conway


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.1432.39   11.71   42762
 44115 43882   0.2037.27   13.42   42633
 43444 43431   0.1536.99   12.00   42147
 44573 44265   0.1734.87   11.81   42898
 
 $ qpid-cpp-benchmark --repeat 5 
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47178 47061   0.1627.23   8.4645644
 46291 46291   0.1425.92   9.5144812
 47704 47525   0.1732.19   9.0645998
 47771 47745   0.1721.34   8.6046271
 43950 43931   0.1352.13   10.60   43118
 
 So, there is still some impact, but I believe it has been reduced. There are 
 some further things to try (caching encoded header as originally received to 
 avoid re-encoding it).
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/CMakeLists.txt 1485001 
   /trunk/qpid/cpp/src/Makefile.am 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 

Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Gordon Sim


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/Message.h, line 109
  https://reviews.apache.org/r/11329/diff/1/?file=295341#file295341line109
 
  Manual step is error prone, can you do it automatically when the 
  persistent context is requested?

The reason I didn't do that is that requesting the persistent context is 
currently defined as a const method. If I do actually modify the internal state 
of the message then I need to explicitly make the relevant members mutable and 
that complicates the concurrency picture a little. I'm not delighted with the 
explicit pack however...


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/Message.h, line 158
  https://reviews.apache.org/r/11329/diff/1/?file=295341#file295341line158
 
  Presumably you split the annotations map into string/int because 
  Variant overhead was a performance problem. Should comment that here so 
  somebody doesn't later go hey, I could just use a VariantMap here

Yes indeed that was the reason and I will add a comment.


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/Message.cpp, line 231
  https://reviews.apache.org/r/11329/diff/1/?file=295342#file295342line231
 
  What's a CharSequence?

It avoids converting data into a string, i.e. it allows you to identify 
particular parts of a larger block of memory to avoid unpacking it and building 
up some other structure. All it is is a pointer and a limit.


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp, line 422
  https://reviews.apache.org/r/11329/diff/1/?file=295349#file295349line422
 
  There is a lot of gritty encoding detail in Message for handling 
  annotations. I suggest adding a MessageAnnotations class to hold this kind 
  of detail to keep Message clean.

Note that this Message class is for the AMQP 1.0 encoding (i.e. it is different 
from qpid::broker::Message). Does that change your opinion?


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp, line 147
  https://reviews.apache.org/r/11329/diff/1/?file=295351#file295351line147
 
  Could this be encapsulated in a MessageAnnotations class?

I guess I could split it out, yes. (There would need to be two different 
annotations classes for 0-10 and 1.0 - currently the code is in the classes 
encapsulating the encoding of those formats).


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp, line 246
  https://reviews.apache.org/r/11329/diff/1/?file=295351#file295351line246
 
  Why do we have to create all this encoding detail? Don't we already 
  have coded that encodes properties etc?

Yes, but the problem is that it is very tied to complete structures and what I 
want to avoid is copying and modifying those structures.


- Gordon


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20896
---


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.14

Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Andrew Stitcher

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20897
---



/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43030

Generally it'll be faster to use an unordered_map than a map (obviously no 
good if you actually need the sorted property of maps) - #include 
qpid/sys/unordered_map.h and use unordered_mapT instead of mapT.



/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43028

sp. buy - by



/trunk/qpid/cpp/src/qpid/broker/Message.h
https://reviews.apache.org/r/11329/#comment43031

Does having 2 annotation stores give much of a speed up compared to just 
using 1 with a Variant? Which seems more natural (to me at least)

If you are going to have a separate integer store possibly it should be of 
int64_t/uint64_t so it can hold any size int.



/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
https://reviews.apache.org/r/11329/#comment43041

Much of the following code seems like it isn't specific to MessageTransfer 
but is rather another implementation of the FieldTable encoder, doing a very 
similar function (encoding a mapstring,Variant). The only real difference is 
the use of CharSequence instead of string.

There must be a less duplicative solution, possibly making the FieldTable 
encoder use this functionality.

In any case I'd advise putting this code in it's own file as something like 
qpid/framing/MapEncoder.(h|cpp) where it might be used elsewhere.


- Andrew Stitcher


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.1432.39   11.71   42762
 44115 43882   0.2037.27   13.42   42633
 43444 43431   0.1536.99   12.00   42147
 44573 44265   0.1734.87   11.81   42898
 
 $ qpid-cpp-benchmark --repeat 5 
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47178 47061   0.1627.23   8.4645644
 46291 46291   0.1425.92   9.5144812
 47704 47525   0.1732.19   9.0645998
 47771 47745   0.1721.34   8.6046271
 43950 43931   0.1352.13   10.60   43118
 
 So, there is still some impact, but I believe it has been reduced. There are 
 some further things to try (caching encoded header as originally received to 
 avoid re-encoding it).
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/CMakeLists.txt 1485001 
   /trunk/qpid/cpp/src/Makefile.am 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp 1485001 
   

Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Alan Conway


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp, line 422
  https://reviews.apache.org/r/11329/diff/1/?file=295349#file295349line422
 
  There is a lot of gritty encoding detail in Message for handling 
  annotations. I suggest adding a MessageAnnotations class to hold this kind 
  of detail to keep Message clean.
 
 Gordon Sim wrote:
 Note that this Message class is for the AMQP 1.0 encoding (i.e. it is 
 different from qpid::broker::Message). Does that change your opinion?

Yes, that makes more sense. I still think that MessageAnnotations deserves to 
be a class in its own right - handling the 2  maps as one and doing the 
encoding is complex enough to be encapsulated. Is your optimization 1.0 only? 
Are your before/after numbers for 1.0 or 0.10?


- Alan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20896
---


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.1432.39   11.71   42762
 44115 43882   0.2037.27   13.42   42633
 43444 43431   0.1536.99   12.00   42147
 44573 44265   0.1734.87   11.81   42898
 
 $ qpid-cpp-benchmark --repeat 5 
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47178 47061   0.1627.23   8.4645644
 46291 46291   0.1425.92   9.5144812
 47704 47525   0.1732.19   9.0645998
 47771 47745   0.1721.34   8.6046271
 43950 43931   0.1352.13   10.60   43118
 
 So, there is still some impact, but I believe it has been reduced. There are 
 some further things to try (caching encoded header as originally received to 
 avoid re-encoding it).
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/CMakeLists.txt 1485001 
   /trunk/qpid/cpp/src/Makefile.am 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/framing/AMQFrame.h 1485001 
   /trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/framing/FrameSet.h 1485001 
   /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.h PRE-CREATION 
   /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/11329/diff/
 
 
 Testing
 ---
 
 make check/test passes both on autotools and cmake builds
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Gordon Sim


 On May 22, 2013, 3:35 p.m., Andrew Stitcher wrote:
  /trunk/qpid/cpp/src/qpid/broker/Message.h, line 31
  https://reviews.apache.org/r/11329/diff/1/?file=295341#file295341line31
 
  Generally it'll be faster to use an unordered_map than a map (obviously 
  no good if you actually need the sorted property of maps) - #include 
  qpid/sys/unordered_map.h and use unordered_mapT instead of mapT.

Thanks, I'll try that out.


 On May 22, 2013, 3:35 p.m., Andrew Stitcher wrote:
  /trunk/qpid/cpp/src/qpid/broker/Message.h, line 158
  https://reviews.apache.org/r/11329/diff/1/?file=295341#file295341line158
 
  Does having 2 annotation stores give much of a speed up compared to 
  just using 1 with a Variant? Which seems more natural (to me at least)
  
  If you are going to have a separate integer store possibly it should be 
  of int64_t/uint64_t so it can hold any size int.

I agree the Variant approach was 'nicer', the speed-up was observable however 
so I included it. I think the problem with Variant is that the PIMPL idiom 
which is really only needed on the client side results in extra allocations. A 
more efficient equivalent would also be possible.


 On May 22, 2013, 3:35 p.m., Andrew Stitcher wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp, line 147
  https://reviews.apache.org/r/11329/diff/1/?file=295351#file295351line147
 
  Much of the following code seems like it isn't specific to 
  MessageTransfer but is rather another implementation of the FieldTable 
  encoder, doing a very similar function (encoding a mapstring,Variant). 
  The only real difference is the use of CharSequence instead of string.
  
  There must be a less duplicative solution, possibly making the 
  FieldTable encoder use this functionality.
  
  In any case I'd advise putting this code in it's own file as something 
  like qpid/framing/MapEncoder.(h|cpp) where it might be used elsewhere.

Yes, a large part of this patch adds code that is very similar to existing 
encoding logic. The key difference (apart from the use of CharSequence) is that 
it does 'partial' encoding of structures which existing logic can only encode 
as a whole. This is the heart of the approach (i.e. don't clone, simply merge 
in changes while encoding). A this point I would consider 0-10 encoding logic 
something of a legacy concern. I'm personally not convinced updating the 
FieldTable encoding logic to include this is really justified (there is already 
code duplication between that and qpid::amqp_0_10::Codecs). A more complete 
solution would be to avoid using FieldTable entirely on the broker side. As 
always the question is where to draw the line in concrete improvement and ever 
deeper entanglement in the consequences of changes.


- Gordon


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20897
---


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.1432.39   11.71   42762
 44115 43882   0.2037.27   13.42   42633
 43444 43431   0.1536.99   12.00   42147
 44573 44265   0.1734.87   11.81   

Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Gordon Sim


 On May 22, 2013, 3:19 p.m., Alan Conway wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp, line 422
  https://reviews.apache.org/r/11329/diff/1/?file=295349#file295349line422
 
  There is a lot of gritty encoding detail in Message for handling 
  annotations. I suggest adding a MessageAnnotations class to hold this kind 
  of detail to keep Message clean.
 
 Gordon Sim wrote:
 Note that this Message class is for the AMQP 1.0 encoding (i.e. it is 
 different from qpid::broker::Message). Does that change your opinion?
 
 Alan Conway wrote:
 Yes, that makes more sense. I still think that MessageAnnotations 
 deserves to be a class in its own right - handling the 2  maps as one and 
 doing the encoding is complex enough to be encapsulated. Is your optimization 
 1.0 only? Are your before/after numbers for 1.0 or 0.10?

No the optimisation covers 0-10 as well, which is what I used for the numbers 
reported. I'll separate out the code.


- Gordon


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20896
---


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47138 47120   0.1329.02   9.3545870
 46299 46156   0.1325.24   8.3344803
 45910 45793   0.1353.53   10.81   45300
 46607 46388   0.1629.70   9.2145245
 46905 46766   0.1631.43   9.6245776
 
 After:
 
 $ qpid-cpp-benchmark --repeat 5  --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 44418 44376   0.1532.29   10.24   43191
 44127 44045   0.1432.39   11.71   42762
 44115 43882   0.2037.27   13.42   42633
 43444 43431   0.1536.99   12.00   42147
 44573 44265   0.1734.87   11.81   42898
 
 $ qpid-cpp-benchmark --repeat 5 
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 47178 47061   0.1627.23   8.4645644
 46291 46291   0.1425.92   9.5144812
 47704 47525   0.1732.19   9.0645998
 47771 47745   0.1721.34   8.6046271
 43950 43931   0.1352.13   10.60   43118
 
 So, there is still some impact, but I believe it has been reduced. There are 
 some further things to try (caching encoded header as originally received to 
 avoid re-encoding it).
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/CMakeLists.txt 1485001 
   /trunk/qpid/cpp/src/Makefile.am 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.h 1485001 
   /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/framing/AMQFrame.h 1485001 
   /trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp 1485001 
   /trunk/qpid/cpp/src/qpid/framing/FrameSet.h 1485001 
   /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.h PRE-CREATION 
   /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/11329/diff/
 
 
 Testing
 ---
 
 make check/test passes both on autotools and cmake builds
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Performance improvements to Message::addAnnotations()

2013-05-22 Thread Gordon Sim


 On May 22, 2013, 3:35 p.m., Andrew Stitcher wrote:
  /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp, line 147
  https://reviews.apache.org/r/11329/diff/1/?file=295351#file295351line147
 
  Much of the following code seems like it isn't specific to 
  MessageTransfer but is rather another implementation of the FieldTable 
  encoder, doing a very similar function (encoding a mapstring,Variant). 
  The only real difference is the use of CharSequence instead of string.
  
  There must be a less duplicative solution, possibly making the 
  FieldTable encoder use this functionality.
  
  In any case I'd advise putting this code in it's own file as something 
  like qpid/framing/MapEncoder.(h|cpp) where it might be used elsewhere.
 
 Gordon Sim wrote:
 Yes, a large part of this patch adds code that is very similar to 
 existing encoding logic. The key difference (apart from the use of 
 CharSequence) is that it does 'partial' encoding of structures which existing 
 logic can only encode as a whole. This is the heart of the approach (i.e. 
 don't clone, simply merge in changes while encoding). A this point I would 
 consider 0-10 encoding logic something of a legacy concern. I'm personally 
 not convinced updating the FieldTable encoding logic to include this is 
 really justified (there is already code duplication between that and 
 qpid::amqp_0_10::Codecs). A more complete solution would be to avoid using 
 FieldTable entirely on the broker side. As always the question is where to 
 draw the line in concrete improvement and ever deeper entanglement in the 
 consequences of changes.
 
 Andrew Stitcher wrote:
 Yes you are correct about the duplicated code in Codecs, probably that 
 code could use this as well.
 
 Although it may be a legacy concern, all this extra duplication makes it 
 continually harder to understand the code base and hence maintain it. So 
 unless we're not going to support 0.10 in the near future in which case we 
 can remove the code and be done with it, we will need to maintain the code.
 
 But of course, it does always come down to ROI at some level.

Exactly, its a question of ROI. Though the same function is being done in three 
different places (Codecs, here and FieldTable) they all have different source 
structures. In Codecs it is a Variant::Map, in FieldTable it is the map of 
FieldValue pointers and here it is a more abstracted source that hides how the 
actual values are stored (also as above, here we split the encoding of values 
in the map from the encoding of the map as a whole.

Could Codecs and FieldTable be modified to pump their structures through a 
MapHandler (which would be relocated to the common package) and thus share the 
small amount of common code (put type code, put typed value)? yes, of course. 
That wouldn't improve the readability/maintainability of FieldTable however. It 
would still be confusing and annoying to have the different apopraches. The 
only solution that IMHO gives tangible maintenance benefits would be wholesale 
removal of FieldTable from (at least) the broker codebase. That is something I 
would dearly love to do. It isn't a small change though.

As and when I have time I'll return to this problem and see whether by altering 
the brokers handling of 0-10 header frames more radically I can make a 
significant simpification overall (as well as possibly some further performance 
gains).


- Gordon


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11329/#review20897
---


On May 22, 2013, 1:58 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/11329/
 ---
 
 (Updated May 22, 2013, 1:58 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and 
 Ernie Allen.
 
 
 Description
 ---
 
 This attempts to reduce the performance impact of adding annotations (headers 
 in 0-10) to a message as it passes through the broker. At present this uses a 
 copy-on-write approaches that clones the header body, modifies it and then 
 uses that for encode or for writing to the wire. This change avoids the full 
 clone, by encoding the original header and while doing so adjusting for any 
 additions.
 
 Before:
 
 $ qpid-cpp-benchmark --repeat 5 --create-option 
 node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}
 send-tp   recv-tp l-min   l-max   l-avg   total-tp
 30720 30671   0.3244.03   18.28   30064
 31778 31729   0.2254.36   16.77   31068
 31664 31637   0.3969.50   22.11   31014
 31303 31301   0.3343.97   17.32   30737
 31577 31403   0.3337.73   15.59   30785
 
 $ qpid-cpp-benchmark --repeat 5
 send-tp  

Re: RC3: three rare failures

2013-05-22 Thread Alan Conway

On 05/13/2013 09:31 AM, Michael Goulish wrote:

I ran autotools make check 100 times.
In addition to the queue_flow_limit failures that we already know about, I saw 
three very-low-frequency failures.


   1  cli_tests.CliTests.test_queue_params
   1  ha_tests.ReplicationTests.test_priority_ring


There is a fix for this, its QPID-4745. The issue is a test bug, not a qpid bug 
so I don't know if it is worth porting to trunk at this stage of the release.


-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



Re: Website update 4

2013-05-22 Thread Alan Conway

On 05/10/2013 12:39 PM, Justin Ross wrote:

Update:
 http://people.apache.org/~jross/transom/2013-05-10/


Perhaps a Documentation link under Resources on the front page, leading to a 
list of all the docs?


C++ Broker page: High Availability links to C++ broker book, suggest a link to 
the HA chapter.
It's not immediately obvious what section you should read if you're just brought 
to the book.





Previous versions:
 http://people.apache.org/~jross/transom/2013-04-16/
 http://people.apache.org/~jross/transom/2013-04-03/
 http://people.apache.org/~jross/transom/2013-03-26/
 http://people.apache.org/~jross/transom/2013-03-13/
Head version, for following things as they change:
 http://people.apache.org/~jross/transom/head/
The source code and README:
 https://github.com/ssorj/transom

Some bigger changes:

   - Added scripts to generate release notes
   - Added link to EAP 6 instructions in JCA page
   - Added QMF api doc
   - Add pdfs to book-generation scripts
   - Updated the messenger doc snapshot
   - Added messenger and protocol engine API doc in C, python, and java
   - Added messenger examples in many languages
   - Added make target and instructions for publishing content
   - Improved the developer center
   - Added 0.22 release content
   - Overhauled the scripts for generating release content
   - Added jms examples

To better show the proposed launch state, I've made 0.22 the current
release in the preview site.  As a result, some of the links won't
work because they point to things (such as a release tag) that don't
exist yet.

I'd like to hold a vote in about a week on replacing our current
website.  I'll have time for another update in the meantime, so please
let me know about any changes you'd like to see.  In particular, take
a pass through the component page for the parts of Qpid you are
familiar with.  I'd love to get some detailed corrections and
refinements.

Thanks!
Justin

-
To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org
For additional commands, e-mail: users-h...@qpid.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



0.22 release update - proposed final RC5

2013-05-22 Thread Justin Ross
Hi, everyone.  RC5 from revision 1485302:

  http://people.apache.org/~jross/qpid-0.22-rc5/
  http://people.apache.org/~jross/qpid-0.22-rc5-testing.txt

This release candidate is signed.  The bits in RC5, if approved for
release, will be the GA bits.

Vote thread coming up.

Justin

---
0.22 release page: https://cwiki.apache.org/qpid/022-release.html

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Resolved] (QPID-4857) Perl TypeError on message release/reject

2013-05-22 Thread Darryl L. Pierce (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4857?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darryl L. Pierce resolved QPID-4857.


Resolution: Fixed

 Perl TypeError on message release/reject 
 -

 Key: QPID-4857
 URL: https://issues.apache.org/jira/browse/QPID-4857
 Project: Qpid
  Issue Type: Bug
  Components: Perl Client
Affects Versions: 0.22
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
 Fix For: 0.23


 When user tries to release/reject the acquired message, following error 
 occurs:
 TypeError in method 'Session_release', argument 2 of type 
 'qpid::messaging::Message '
 Version-Release number of selected component (if applicable):
 perl-qpid-0.22-4
 How reproducible:
 100%
 Steps to Reproduce:
 1. see additional info 
   
 Actual results:
 Unable to release/reject message
 Expected results:
 Acquired message may be released/rejected
 Additional info:
 Reproducer:
 1. release message
 #!/usr/bin/env perl
 use qpid;
 my $message  = new qpid::messaging::Message();
 my $address  = q;{create:sender,delete:receiver};
 my $connection = new qpid::messaging::Connection(127.0.0.1);
 $connection-open();
 my $session  = $connection-create_session();
 my $sender = $session-create_sender($address);
 my $receiver = $session-create_receiver($address);
 $sender-send($message);
 eval { $message = $receiver-fetch(0); };
 $session-release($message);
 $session-reject($message);
 $receiver-close();
 $session-close();
 $connection-close();
 2. reject message
 same as above with following modification:
 - $session-release($message);
 + $session-reject($message);

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rajith Attapattu (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664379#comment-13664379
 ] 

Rajith Attapattu commented on QPID-4873:


I'm happy with optimization #2 and #3.

A bit concerned with optimization #1. 
While it works well if you reuse a limited number of option Strings, it might 
become an issue when there is a large number of distinct entries.
Even with a moderate number of queues (with distinct option strings), we might 
be keeping those maps in memory for longer than necessary, with little or no 
gain.

I would prefer to make this behaviour configurable and leave the current 
behaviour as the default.
Having a large number of queues with the same option String is not really a 
common use case, therefore it makes sense to make this optimization to be 
turned on only when needed.

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link 

[jira] [Resolved] (QPID-4872) python client occasionally fails with [Errno 1] _ssl.c:1217: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry error

2013-05-22 Thread Ken Giusti (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4872?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ken Giusti resolved QPID-4872.
--

Resolution: Fixed

Fix:

http://svn.apache.org/viewvc?view=revisionrevision=1485331

 python client occasionally fails with [Errno 1] _ssl.c:1217: 
 error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry error
 ---

 Key: QPID-4872
 URL: https://issues.apache.org/jira/browse/QPID-4872
 Project: Qpid
  Issue Type: Bug
  Components: Python Client
Affects Versions: 0.20
Reporter: Ken Giusti
Assignee: Ken Giusti
 Fix For: 0.23

 Attachments: bz950501.py


 Originally reported in JIRA QPID-3175.
 See comment 
 https://issues.apache.org/jira/browse/QPID-3175?focusedCommentId=13276773page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13276773

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4790) CI test failure: queue_flow_limit_tests.QueueFlowLimitTests.test_blocked_queue_delete ....fail

2013-05-22 Thread Ken Giusti (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664415#comment-13664415
 ] 

Ken Giusti commented on QPID-4790:
--

Root cause of this issue is actually a bug in the RHEL6 version I've been using:

https://bugzilla.redhat.com/show_bug.cgi?id=741897

The bug affects RHEL6.4 running on Xenon hardware.  It can be worked-around by 
blacklisting the ioatdma kernel module in /etc/modprobe.d/blacklist.conf

Given that the bug can cause rare but unpredictable corruption of data read 
from a socket, I'd recommend implementing the work-around if you are using the 
target hardware and software.

Resolving this as it is not a bug in the qpid code.


 CI test failure: 
 queue_flow_limit_tests.QueueFlowLimitTests.test_blocked_queue_delete fail
 --

 Key: QPID-4790
 URL: https://issues.apache.org/jira/browse/QPID-4790
 Project: Qpid
  Issue Type: Bug
  Components: C++ Broker, Python Client
Affects Versions: 0.22
Reporter: Ken Giusti
Assignee: Ken Giusti

 Occasional failure seen for above test.   Hard to reproduce.
 Additional details:
 1) Failure occurs when test attempts to delete a queue that has messages on 
 it, and a producer that is blocked by flow control.
 2) The broker log trace shows that the queue.delete message is received.  The 
 queue is deleted and all messages released.  The delete command is completed 
 and the session.completed response is sent.
 3) The python client library (used by the test) receives the 
 session.completed frame.  However, when the test fails the frame appears to 
 be corrupted - it is completely zeroed.
 I've determined (3) by dumping the data as it is read from the socket by the 
 python client library.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4790) CI test failure: queue_flow_limit_tests.QueueFlowLimitTests.test_blocked_queue_delete ....fail

2013-05-22 Thread Ken Giusti (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ken Giusti resolved QPID-4790.
--

Resolution: Not A Problem

 CI test failure: 
 queue_flow_limit_tests.QueueFlowLimitTests.test_blocked_queue_delete fail
 --

 Key: QPID-4790
 URL: https://issues.apache.org/jira/browse/QPID-4790
 Project: Qpid
  Issue Type: Bug
  Components: C++ Broker, Python Client
Affects Versions: 0.22
Reporter: Ken Giusti
Assignee: Ken Giusti

 Occasional failure seen for above test.   Hard to reproduce.
 Additional details:
 1) Failure occurs when test attempts to delete a queue that has messages on 
 it, and a producer that is blocked by flow control.
 2) The broker log trace shows that the queue.delete message is received.  The 
 queue is deleted and all messages released.  The delete command is completed 
 and the session.completed response is sent.
 3) The python client library (used by the test) receives the 
 session.completed frame.  However, when the test fails the frame appears to 
 be corrupted - it is completely zeroed.
 I've determined (3) by dumping the data as it is read from the socket by the 
 python client library.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rob Godfrey (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664438#comment-13664438
 ] 

Rob Godfrey commented on QPID-4873:
---

Rather than having an on/off configuration, how about having a (configurable) 
fixed size LRU cache? Replace the ConcurrentHashMap in the patch with a 
subclass of LinkedHashMap with modification to keep a fixed size (by overriding 
removeEldestEntry as described in the javadoc) overriding get(Object key) as 

{code}
public V get(Object key)
{ 
V value = remove(key); 
if(value != null) 
{
   put((K) key, value);
}
}
{code}

then wrapping the map using Collections.synchronizedMap() to maintain safety.

That way we'd have an upper bound on what we cache, but still benefit from 
reuse in the case where most of the options are the same.



 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties 

Re: [VOTE] Release 0.22

2013-05-22 Thread Darryl L. Pierce
On Wed, May 22, 2013 at 01:54:50PM -0400, Justin Ross wrote:
 RC5 contains the proposed final bits for Qpid 0.22.
 
 If you favor making the RC5 bits into our official release, vote +1.
 If you have reason to believe RC5 is not ready for release, vote -1.
 
 I will close this vote on Monday, 22 May.  I know that's a little
 brief, but I have some vacation time coming up soon, and I'm hoping to
 finish the release before I leave.

The patches for 4344 were applied in reverse on this branch and, with
the last cherry-pick, the bindings/CMakeLists.txt broke the CMake
changes. The conditional check for Swig  1.3.32 is still in the file
and needs to be deleted to fix CMake.

This is totally my fault.

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



pgplHrstRn2hT.pgp
Description: PGP signature


[jira] [Created] (QPID-4879) Patches applied out of order breaks CMake

2013-05-22 Thread Darryl L. Pierce (JIRA)
Darryl L. Pierce created QPID-4879:
--

 Summary: Patches applied out of order breaks CMake
 Key: QPID-4879
 URL: https://issues.apache.org/jira/browse/QPID-4879
 Project: Qpid
  Issue Type: Bug
  Components: Build Tools
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
Priority: Blocker


QPID-4344 patches were applied out of order and caused a conditional to be 
added to the bindings Cmake file. This didn't show up until the last patch 
pulled in removed the end() directive for it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4879) Patches applied out of order breaks CMake

2013-05-22 Thread Darryl L. Pierce (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4879?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darryl L. Pierce updated QPID-4879:
---

Attachment: 0001-QPID-4879-Remove-the-swig-minimum-version-check.patch

 Patches applied out of order breaks CMake
 -

 Key: QPID-4879
 URL: https://issues.apache.org/jira/browse/QPID-4879
 Project: Qpid
  Issue Type: Bug
  Components: Build Tools
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
Priority: Blocker
 Attachments: 
 0001-QPID-4879-Remove-the-swig-minimum-version-check.patch


 QPID-4344 patches were applied out of order and caused a conditional to be 
 added to the bindings Cmake file. This didn't show up until the last patch 
 pulled in removed the end() directive for it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4879) Patches applied out of order breaks CMake

2013-05-22 Thread Darryl L. Pierce (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664481#comment-13664481
 ] 

Darryl L. Pierce commented on QPID-4879:


The attached patch fixes the issue on the 0.22 branch.

 Patches applied out of order breaks CMake
 -

 Key: QPID-4879
 URL: https://issues.apache.org/jira/browse/QPID-4879
 Project: Qpid
  Issue Type: Bug
  Components: Build Tools
Reporter: Darryl L. Pierce
Assignee: Darryl L. Pierce
Priority: Blocker
 Attachments: 
 0001-QPID-4879-Remove-the-swig-minimum-version-check.patch


 QPID-4344 patches were applied out of order and caused a conditional to be 
 added to the bindings Cmake file. This didn't show up until the last patch 
 pulled in removed the end() directive for it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rajith Attapattu (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664487#comment-13664487
 ] 

Rajith Attapattu commented on QPID-4873:


+1 Seems a good solution!

Rajith

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties defined, but no bindings.
 Overall: Originally, each queue took up about 2760B for us; with these 
 optimizations, that goes down to 1024B, saving 63% per queue for us.
 We'd appreciate feedback on these changes and whether we are making any 
 incorrect assumptions. I've also added relevant tests to AMQDestinationTest 
 but am not sure if that's the best place. Thanks a lot!

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


Re: [VOTE] Release 0.22

2013-05-22 Thread Justin Ross
Okay, we'll fix it tomorrow.
On May 22, 2013 4:09 PM, Darryl L. Pierce dpie...@redhat.com wrote:

 On Wed, May 22, 2013 at 01:54:50PM -0400, Justin Ross wrote:
  RC5 contains the proposed final bits for Qpid 0.22.
 
  If you favor making the RC5 bits into our official release, vote +1.
  If you have reason to believe RC5 is not ready for release, vote -1.
 
  I will close this vote on Monday, 22 May.  I know that's a little
  brief, but I have some vacation time coming up soon, and I'm hoping to
  finish the release before I leave.

 The patches for 4344 were applied in reverse on this branch and, with
 the last cherry-pick, the bindings/CMakeLists.txt broke the CMake
 changes. The conditional check for Swig  1.3.32 is still in the file
 and needs to be deleted to fix CMake.

 This is totally my fault.

 --
 Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
 Delivering value year after year.
 Red Hat ranks #1 in value among software vendors.
 http://www.redhat.com/promo/vendor/




[jira] [Commented] (QPID-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Helen Kwong (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664604#comment-13664604
 ] 

Helen Kwong commented on QPID-4873:
---

Thanks for looking at this and the feedback. Having a fixed size LRU cache 
sounds good -- Rob / Rajith, should I attach another patch with the changes, or 
do you want to make those changes directly? If you'd like an updated patch, can 
you please suggest an appropriate default cache size, and where that config 
should be?

Also, according to the javadoc, there is a LinkedHashMap constructor that lets 
you specify to use access order instead of insertion order -- if that is used 
it seems like get(Object key) doesn't need to be overridden.

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties defined, but no bindings.
 Overall: Originally, each queue took up about 2760B for us; with these 
 optimizations, that 

[jira] [Resolved] (QPID-4878) [AMQP 1.0] SASL layer doesn't work for interlinks

2013-05-22 Thread Gordon Sim (JIRA)

 [ 
https://issues.apache.org/jira/browse/QPID-4878?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gordon Sim resolved QPID-4878.
--

Resolution: Fixed

Fixed by http://svn.apache.org/viewvc?view=revisionrevision=r1485467

 [AMQP 1.0] SASL layer doesn't work for interlinks
 -

 Key: QPID-4878
 URL: https://issues.apache.org/jira/browse/QPID-4878
 Project: Qpid
  Issue Type: Bug
  Components: C++ Broker
Affects Versions: 0.20, 0.22
Reporter: Gordon Sim
Assignee: Gordon Sim
 Fix For: 0.23




--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
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-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rob Godfrey (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664646#comment-13664646
 ] 

Rob Godfrey commented on QPID-4873:
---

Hi Helen,

good spot - I'd forgotten about that extra constructor (been a while since I 
messed with LinkedHashMaps :-) ).
 
I think Rajith is planning on making the change in the morning so there's no 
need for you to make another patch.

Thanks again for all your efforts

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties defined, but no bindings.
 Overall: Originally, each queue took up about 2760B for us; with these 
 optimizations, that goes down to 1024B, saving 63% per queue for us.
 We'd appreciate feedback on these changes and whether we are making any 
 incorrect assumptions. I've also added relevant tests to AMQDestinationTest 
 but am not sure if that's the best place. Thanks a lot!

--
This 

[jira] [Commented] (QPID-4873) Optimizations in Java client to reduce queue memory footprint

2013-05-22 Thread Rajith Attapattu (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-4873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13664819#comment-13664819
 ] 

Rajith Attapattu commented on QPID-4873:


Hi Helen,

I will take care of it first thing tomorrow morning.
Another big thank you from me for all your effort on this.


Rajith

 Optimizations in Java client to reduce queue memory footprint
 -

 Key: QPID-4873
 URL: https://issues.apache.org/jira/browse/QPID-4873
 Project: Qpid
  Issue Type: Improvement
  Components: Java Client, Java Common
Affects Versions: 0.23
Reporter: Helen Kwong
Assignee: Rajith Attapattu
Priority: Minor
 Fix For: 0.23

 Attachments: ClientQueueMemoryOptimizations.patch


 My team is using the Java broker and Java client, version 0.16, and looking 
 to lower the client's memory footprint on our servers. We did some heap 
 analysis and found that the consumption is coming mostly from 
 AMQAnyDestination instances, each having a retained size close to ~3KB, and 
 since we have 6000 queues on each of our 2 brokers, this amounts to about 
 ~33MB, which is valuable real estate for us. In our analysis we found a few 
 possible optimizations in the Qpid code that would reduce the per-queue heap 
 consumption and which don't seem high risk, and would like to propose the 
 following changes (will attach a patch file). 
 (I had originally emailed the users list 2 weeks ago, and Rob Godfrey asked 
 me to raise a JIRA with the changes in a patch file -- 
 http://mail-archives.apache.org/mod_mbox/qpid-users/201305.mbox/%3CCACsaS94F0MQeyAKTN3yoU=j-MPc6oFWZgtCtj68GAwOcN=5...@mail.gmail.com%3E)
 The changes I attach here are with the trunk code and I've redone the numbers 
 / analysis running with the latest client.
 1. In Address / AddressParser, cacheing / reusing the options Maps for queues 
 created with the same options string. (This optimization gives us the most 
 significant savings.)
 All our queues are created with the same options string, which means each 
 corresponding AMQDestination has an Address that has an _options Map that is 
 the same for all queues, i.e., 12K copies of the same map. As far as we can 
 tell, the _options map is effectively immutable, i.e., there is no code path 
 by which an Address’s _options map can be modified. (Is this correct?) So a 
 possible improvement is that in org.apache.qpid.messaging.util.AddressParser, 
 we cache the options map for each options string that we've already 
 encountered, and if the options string passed in has already been seen, we 
 use the stored options map for that Address. This way, for queues having the 
 same options, their Address options will reference the same Map. (For our 
 queues, each Address _options Map currently takes up 1416 B.)
 2. AMQDestination's _link field -- 
 org.apache.qpid.client.messaging.address.Link
 Optimization A: org.apache.qpid.client.messaging.address.Link$Subscription's 
 args field is by default a new HashMap with default capacity 16. In our use 
 case it remains empty for all queues. A possible optimization is to set the 
 default value as Collections.emptyMap() instead. As far was we can tell, 
 Subscription.getArgs() is not used to get the map and then modify it. For us 
 this saves 128B per queue.
 Optimization B: Similarly, Link has a _bindings List that is by default a new 
 ArrayList with a default capacity of 10. In our use case it remains empty for 
 all queues, and as far as we can tell this list is not modified after it is 
 set. If we make the default value Collections.emptyList() instead, it will 
 save us 80B per queue.
 3. AMQDestination's _node field -- 
 org.apache.qpid.client.messaging.address.Node
 Node has a _bindings List that is by default a new ArrayList with the default 
 capacity. In our use case _bindings remains empty for all queues, and I don't 
 see getBindings() being used to get the list and then modify it. I also don't 
 see addBindings() being called anywhere in the client. So a possible 
 optimization is to set the default value as Collections.emptyList() instead. 
 For us this saves 80B per queue.
 The changes in AddressHelper.getBindings() are for the case when there are 
 node or link properties defined, but no bindings.
 Overall: Originally, each queue took up about 2760B for us; with these 
 optimizations, that goes down to 1024B, saving 63% per queue for us.
 We'd appreciate feedback on these changes and whether we are making any 
 incorrect assumptions. I've also added relevant tests to AMQDestinationTest 
 but am not sure if that's the best place. Thanks a lot!

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more