search

stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

Errors sent from ActiveMQ are not exposed in any way to the client #73

Closed rtyler closed 10 years ago

rtyler commented 11 years ago

When performing some load testing, we configured really low memory/disk usage for ActiveMQ, e.g.:

          <systemUsage>                                                             
            <systemUsage sendFailIfNoSpace="true">                                  
                <memoryUsage>                                                       
                    <memoryUsage limit="1 mb"/>                                    
                </memoryUsage>                                                      
                <storeUsage>                                                        
                    <storeUsage limit="1 mb"/>                                    
                </storeUsage>                                                       
                <tempUsage>                                                         
                    <tempUsage limit="1 mb"/>                                      
                </tempUsage>                                                        
            </systemUsage>                                                          
        </systemUsage>

Note the sendFailIfNoSpace.

What ends up happening if the broker cannot receive messages is that the Stomp::Client#publish call continues to "succeed" (not block at least) and eventually the following is printed to $stderr:

hreceive failed: invalid command: "\nERROR\ncontent-type:text/plain\nmessage:Temp Store is Full (0% of 5242880). Stopping producer (ID:activemq-test1-57156-1378923899188-8:1:-1:1) to prevent flooding queue://consumer.testcase.virtual.stahp. See http://activemq.apache.org/producer-flow-control.html for more info\n\njavax.jms.ResourceAllocationException: Temp Store is Full (0% of 5242880). Stopping producer (ID:activemq-test1-57156-1378923899188-8:1:-1:1) to prevent flooding queue://consumer.testcase.virtual.stahp. See http://activemq.apache.org/producer-flow-control.html for more info\n\tat org.apache.activemq.broker.region.BaseDestination.waitForSpace(BaseDestination.java:616)\n\tat org.apache.activemq.broker.region.BaseDestination.waitForSpace(BaseDestination.java:610)\n\tat org.apache.activemq.broker.region.Queue.checkUsage(Queue.java:825)\n\tat org.apache.activemq.broker.region.Queue.doMessageSend(Queue.java:742)\n\tat org.apache.activemq.broker.region.Queue.send(Queue.java:721)\n\tat org.apache.activemq.broker.region.DestinationFilter.send(DestinationFilter.java:142)\n\tat org.apache.activemq.broker.region.virtual.VirtualTopicInterceptor.send(VirtualTopicInterceptor.java:50)\n\tat org.apache.activemq.broker.region.AbstractRegion.send(AbstractRegion.java:406)\n\tat org.apache.activemq.broker.region.RegionBroker.send(RegionBroker.java:392)\n\tat org.apache.activemq.broker.jmx.ManagedRegionBroker.send(ManagedRegionBroker.java:282)\n\tat org.apache.activemq.broker.BrokerFilter.send(BrokerFilter.java:129)\n\tat org.apache.activemq.broker.CompositeDestinationBroker.send(CompositeDestinationBroker.java:96)\n\tat org.apache.activemq.broker.TransactionBroker.send(TransactionBroker.java:317)\n\tat org.apache.activemq.broker.BrokerFilter.send(BrokerFilter.java:129)\n\tat org.apache.activemq.broker.BrokerFilter.send(BrokerFilter.java:129)\n\tat org.apache.activemq.security.AuthorizationBroker.send(AuthorizationBroker.java:202)\n\tat org.apache.activemq.broker.MutableBrokerFilter.send(MutableBrokerFilter.java:135)\n\tat org.apache.activemq.broker.TransportConnection.processMessage(TransportConnection.java:499)\n\tat org.apache.activemq.command.ActiveMQMessage.visit(ActiveMQMessage.java:749)\n\tat org.apache.activemq.broker.TransportConnection.service(TransportConnection.java:329)\n\tat org.apache.activemq.broker.TransportConnection$1.onCommand(TransportConnection.java:184)\n\tat org.apache.activemq.transport.MutexTransport.onCommand(MutexTransport.java:45)\n\tat org.apache.activemq.transport.AbstractInactivityMonitor.onCommand(AbstractInactivityMonitor.java:288)\n\tat org.apache.activemq.transport.stomp.StompTransportFilter.sendToActiveMQ(StompTransportFilter.java:84)\n\tat org.apache.activemq.transport.stomp.ProtocolConverter.sendToActiveMQ(ProtocolConverter.java:195)\n\tat org.apache.activemq.transport.stomp.ProtocolConverter.onStompSend(ProtocolConverter.java:321)\n\tat org.apache.activemq.transport.stomp.ProtocolConverter.onStompCommand(ProtocolConverter.java:233)\n\tat org.apache.activemq.transport.stomp.StompTransportFilter.onCommand(StompTransportFilter.java:73)\n\tat org.apache.activemq.transport.TransportSupport.doConsume(TransportSupport.java:83)\n\tat org.apache.activemq.transport.tcp.TcpTransport.doRun(TcpTransport.java:214)\n\tat org.apache.activemq.transport.tcp.TcpTransport.run(TcpTransport.java:196)\n\tat java.lang.Thread.run(Thread.java:722)\n\u0

I think the Stomp::Client should have some means of expressing an error from the server, but I'm not sure how it should be expressed

PaulGale commented 11 years ago

Yep - noticed the same thing. The latest version of the gem seems to rely on the presence of a Slogger to get much information to bubble out to the caller. I don't like that approach.

I have a number of local patches to the latest gem that I can share with you if you're interested.

One in particular includes handling ERROR frames, parsing their content and turning them into higher level exceptions that are invoked on the caller's thread, etc. I've trolled the entirety of the ActiveMQ codebase to come up with the list of all ResourceAllocationException's that can make their way back to the client. If you're using a different broker you can just replace the regexs with ones of your choosing.

Let me know.

rtyler commented 11 years ago

@ppaul Any particular reason you've not submitted pull requests for those patches you've got backlogged? I'd like to see them in that form, or at least in a forked repo

PaulGale commented 11 years ago

Any particular reason you've not submitted pull requests for those patches you've got backlogged?

Mainly because I no longer have an appetite for dealing with the current maintainers with whom one has to battle tirelessly in order to get something accepted.

My patches are just a stop gap until I either write a stomp gem myself (which I don't really have time for) or start using another protocol.

ismith commented 11 years ago

@ppaul I'd also be interested in seeing those patches. (@rtyler and I work together, so I'll see 'em anyway if you send them to him.) And I have recently acquired bits on this repo, so if the problem is just lack of maintainer time/responsiveness, uh ... reiterates mention of a pull request

gmallard commented 11 years ago

The inability of a Stomp::Client to receive async. ERROR frames from the broker is indeed a flaw. Been there since day 1 it seems.

A fix is on the todo list.

PaulGale commented 11 years ago

As requested, here is a Gist containing my local changes. The large comment block at the top details the changes, their rationale, as well as guidance to our developers regarding their use.

They were tested against ActiveMQ 5.8.0 only, using my local client tester, on both JRuby 1.7.3 and rubyee_1.8.7.

YMMV. Bug reports welcome.

Thanks, Paul

ismith commented 10 years ago

@ppaul The error logging parts of that gist are in https://github.com/ismith/stomp/tree/error_logging, they just lack unit tests. If you have some to share, please do, otherwise, I'm going to work on writing some.

PaulGale commented 10 years ago

Go for it

On Tue, Sep 24, 2013 at 4:33 PM, Ian Smith notifications@github.com wrote:

@ppaul https://github.com/ppaul The error logging parts of that gist are in https://github.com/ismith/stomp/tree/error_logging, they just lack unit tests. If you have some to share, please do, otherwise, I'm going to work on writing some.

— Reply to this email directly or view it on GitHubhttps://github.com/stompgem/stomp/issues/73#issuecomment-25039917 .

ismith commented 10 years ago

Resolved as pull req #81, will be releasing shortly with this change.