borgstrom opened a new pull request #359: AMQ-7211 STOMP: Do not clear pending ACKs on COMMIT URL: https://github.com/apache/activemq/pull/359 This PR first adds a test that proves the behavior is broken: c3729b0 When you run this test against master it will fail: ``` ------------------------------------------------------- T E S T S ------------------------------------------------------- Running org.apache.activemq.transport.stomp.Stomp12Test Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 62.016 sec <<< FAILURE! - in org.apache.activemq.transport.stomp.Stomp12Test testClientIndividualAckPrefetchTransaction(org.apache.activemq.transport.stomp.Stomp12Test) Time elapsed: 61.822 sec <<< FAILURE! java.lang.AssertionError: null at org.junit.Assert.fail(Assert.java:86) at org.junit.Assert.assertTrue(Assert.java:41) at org.junit.Assert.assertTrue(Assert.java:52) at org.apache.activemq.transport.stomp.Stomp12Test.testClientIndividualAckPrefetchTransaction(Stomp12Test.java:413) Results : Failed tests: Stomp12Test.testClientIndividualAckPrefetchTransaction:413 null Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` The test output shows: ``` 2019-05-20 21:54:48,711 [0.1:55337@55324] - WARN ProtocolConverter - Exception occurred processing: ACK -> org.apache.activemq.transport.stomp.ProtocolException: Unexpected ACK received for message-id [null] 2019-05-20 21:54:48,712 [0.1:55337@55324] - DEBUG ProtocolConverter - Exception detail org.apache.activemq.transport.stomp.ProtocolException: Unexpected ACK received for message-id [null] at org.apache.activemq.transport.stomp.ProtocolConverter.onStompAck(ProtocolConverter.java:476) at org.apache.activemq.transport.stomp.ProtocolConverter.onStompCommand(ProtocolConverter.java:251) at org.apache.activemq.transport.stomp.StompTransportFilter.onCommand(StompTransportFilter.java:85) at org.apache.activemq.transport.TransportSupport.doConsume(TransportSupport.java:83) at org.apache.activemq.transport.tcp.TcpTransport.doRun(TcpTransport.java:233) at org.apache.activemq.transport.tcp.TcpTransport.run(TcpTransport.java:215) at java.lang.Thread.run(Thread.java:748) ``` Which is the exception I reported in https://issues.apache.org/jira/browse/AMQ-7211 The fix for this (be19dcaa3) is simply to NOT clear pending ACKs on transaction COMMIT & ABORT. Looking at the ticket referenced in the commit that added the `pedingAcks.clear()` lines (52d95ee01 / https://issues.apache.org/jira/browse/AMQ-5423) I believe that changing the `this.pedingAcks.get` to `this.pedingAcks.remove` is the real solution to the memory leak that prompted the change and that clearing the acks at the end of a transaction is not needed and incorrect behavior. CC @tabish121 -- since you commited 52d95ee01 and worked on AMQ-5423
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
