ActiveMQ .Net
  1. ActiveMQ .Net
  2. AMQNET-377

Sending to non-existent temp queue causes consumer to shutdown

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.5.3, 1.5.4
    • Fix Version/s: 1.5.5, 1.6.0
    • Component/s: ActiveMQ
    • Labels:
      None
    • Environment:

      .NET client on Windows Server 2008 R2

      Description

      It appears as though attempting to send a message to a non-existent temp queue is causing the consumer to shutdown. The behavior manifests itself by either the consumer ceasing to consume messages or, if a request timeout is set, a RequestTimedOutException being thrown. I have an NUnit project attached that I was able to reproduce the issue with. I've also attached logs.

      1. 2012-04-09.log
        9 kB
        Chris Robison
      2. activemq.xml
        4 kB
        Chris Robison
      3. AMQ-NMS.zip
        1.41 MB
        Chris Robison
      4. NonExistentTempQueueSendTest.cs
        7 kB
        Timothy Bish
      5. putty.log
        46 kB
        Chris Robison

        Activity

        Hide
        Timothy Bish added a comment -

        Fixed on trunk and in the 1.5.x branch. On the older 1.5 branch the exception will arrive via onException event as an NMSConnectionException.

        Show
        Timothy Bish added a comment - Fixed on trunk and in the 1.5.x branch. On the older 1.5 branch the exception will arrive via onException event as an NMSConnectionException.
        Hide
        Timothy Bish added a comment -

        Nice work on the tests Jim, thanks for cleaning those up. This should be all fixed now on trunk, not sure about merging back into 1.5.x will have to look at that when time permits.

        Show
        Timothy Bish added a comment - Nice work on the tests Jim, thanks for cleaning those up. This should be all fixed now on trunk, not sure about merging back into 1.5.x will have to look at that when time permits.
        Hide
        Jim Gomes added a comment -

        I made an improvement to the test to check for the deletion of the temp queue so that it can run as fast as possible, and to be more reliable. This was only checked into the trunk version to correspond with Tim's changes.

        Show
        Jim Gomes added a comment - I made an improvement to the test to check for the deletion of the temp queue so that it can run as fast as possible, and to be more reliable. This was only checked into the trunk version to correspond with Tim's changes.
        Hide
        Timothy Bish added a comment -

        Added what should be a fix for this in trunk, the OnCommand handling of ConnectionError commands was over aggressive in calling OnException, should call OnAsyncException instead and allow the transport layer to deal with any forced closure of the connection by the broker.

        Show
        Timothy Bish added a comment - Added what should be a fix for this in trunk, the OnCommand handling of ConnectionError commands was over aggressive in calling OnException, should call OnAsyncException instead and allow the transport layer to deal with any forced closure of the connection by the broker.
        Hide
        Timothy Bish added a comment -

        Yes, I think there's still an issue here with that, but for now a simple workaround is to use the AlwaysSyncSend option. When I have some spare time I will look into this a bit more. If anyone wants to construct a simpler test for this to add to the TempDestinationTest test set feel free, should be easy enough to reproduce by creating a temp dest and deleting it and then trying to send to it ensuring that AlwaysSyncSend is true, and WatchTopicAdvisories is false.

        Show
        Timothy Bish added a comment - Yes, I think there's still an issue here with that, but for now a simple workaround is to use the AlwaysSyncSend option. When I have some spare time I will look into this a bit more. If anyone wants to construct a simpler test for this to add to the TempDestinationTest test set feel free, should be easy enough to reproduce by creating a temp dest and deleting it and then trying to send to it ensuring that AlwaysSyncSend is true, and WatchTopicAdvisories is false.
        Hide
        Chris Robison added a comment -

        It seems a little strange to me that a BrokerException can take the whole things down without any way for the user to handle it. Is that by design?

        Show
        Chris Robison added a comment - It seems a little strange to me that a BrokerException can take the whole things down without any way for the user to handle it. Is that by design?
        Hide
        Timothy Bish added a comment -

        You need the AlwaysSyncSend because that's the only way you get an exception response back for the send otherwise the error comes back as a broker error. The sleep is to give the broker time to deal with the removal of the connection and its associated resources, which is needed in the test because we want the next send to fail deterministically. If you didn't sleep its entirely possible that the reply producer send which is using a different connection could sneak in the send before the other connection's transport connection finishes processing the remove of the connection, this is somewhat dependent though on your brokers configuration on whether its using dedicated task runners or not.

        Show
        Timothy Bish added a comment - You need the AlwaysSyncSend because that's the only way you get an exception response back for the send otherwise the error comes back as a broker error. The sleep is to give the broker time to deal with the removal of the connection and its associated resources, which is needed in the test because we want the next send to fail deterministically. If you didn't sleep its entirely possible that the reply producer send which is using a different connection could sneak in the send before the other connection's transport connection finishes processing the remove of the connection, this is somewhat dependent though on your brokers configuration on whether its using dedicated task runners or not.
        Hide
        Jim Gomes added a comment -

        The queue purge was a good addition to the test. As I play around with it a bit, I can get it to fail if I comment out only the Thread.Sleep() call. It failed on about the 20th loop. I'm still trying to understand all of the interactions here. Specifically, why is the AlwaysSyncSend option necessary, and why is the sleep delay necessary? I've never used the AlwaysSyncSend option before, and I don't think we can count on a delay in a real production scenario that is trying to run at top speed. Thoughts?

        Show
        Jim Gomes added a comment - The queue purge was a good addition to the test. As I play around with it a bit, I can get it to fail if I comment out only the Thread.Sleep() call. It failed on about the 20th loop. I'm still trying to understand all of the interactions here. Specifically, why is the AlwaysSyncSend option necessary, and why is the sleep delay necessary? I've never used the AlwaysSyncSend option before, and I don't think we can count on a delay in a real production scenario that is trying to run at top speed. Thoughts?
        Hide
        Timothy Bish added a comment -

        I was wondering if that might be the problem, the test was failing previously because there was no purge of the queue at the start which meant that on a future run it could receive messages left over from previous runs. I have not seen it fails since my changes, I ran it several times successively.

        Show
        Timothy Bish added a comment - I was wondering if that might be the problem, the test was failing previously because there was no purge of the queue at the start which meant that on a future run it could receive messages left over from previous runs. I have not seen it fails since my changes, I ran it several times successively.
        Hide
        Jim Gomes added a comment - - edited

        It's the TestConsumeAfterPublishFailsForDestroyedTempDestination test. On the second pass, it will usually fail randomly between the 15th and 25th loop of the test.

        Scratch that. Looks like I may have had a merge error with some of my local changes. I've reverted out my local changes, and the test seems to succeed consistently now.

        Show
        Jim Gomes added a comment - - edited It's the TestConsumeAfterPublishFailsForDestroyedTempDestination test. On the second pass, it will usually fail randomly between the 15th and 25th loop of the test. Scratch that. Looks like I may have had a merge error with some of my local changes. I've reverted out my local changes, and the test seems to succeed consistently now.
        Hide
        Timothy Bish added a comment -

        Which test are you referring to? If its the TempDestinationTest one it works for me every time, I ran it several times in a row.

        Show
        Timothy Bish added a comment - Which test are you referring to? If its the TempDestinationTest one it works for me every time, I ran it several times in a row.
        Hide
        Jim Gomes added a comment -

        Tim, I can still get the test to fail even after the change you put in to close the replyConsumer. I usually have to run it more than once, because the first time through, it will work. Try running the test more than once, one after another. I'm still trying to narrow it down, but it would be good if you could confirm that it either works all the time for you, or whether subsequent runs will fail.

        Show
        Jim Gomes added a comment - Tim, I can still get the test to fail even after the change you put in to close the replyConsumer. I usually have to run it more than once, because the first time through, it will work. Try running the test more than once, one after another. I'm still trying to narrow it down, but it would be good if you could confirm that it either works all the time for you, or whether subsequent runs will fail.
        Hide
        Chris Robison added a comment -

        So, upon further investigation, I found that I had WatchTopicAdvisory=false because of an apparent bug and getting the exception "Channel open for too long" or something like that. Once I took that away, everything worked like the test did with AlwaysSyncSend=true.

        Show
        Chris Robison added a comment - So, upon further investigation, I found that I had WatchTopicAdvisory=false because of an apparent bug and getting the exception "Channel open for too long" or something like that. Once I took that away, everything worked like the test did with AlwaysSyncSend=true.
        Hide
        Timothy Bish added a comment -

        You need to ensure that you set the ActiveMQHost environment var or set the IP address manually in the test.

        Show
        Timothy Bish added a comment - You need to ensure that you set the ActiveMQHost environment var or set the IP address manually in the test.
        Hide
        Chris Robison added a comment -

        How long is it taking for that test case to run for you? I copied it into my environment the way that you have it and it looks like the AsyncTask method thread is crashing bring the whole thing down but reporting that it passed. For me it is showing that it is completing in 2.5 seconds, but that is impossible considering there are sleeps through the whole test.

        Show
        Chris Robison added a comment - How long is it taking for that test case to run for you? I copied it into my environment the way that you have it and it looks like the AsyncTask method thread is crashing bring the whole thing down but reporting that it passed. For me it is showing that it is completing in 2.5 seconds, but that is impossible considering there are sleeps through the whole test.
        Hide
        Timothy Bish added a comment -

        Attached is a variation of your test that seems to work fine with trunk and the 1.5.x fixes branch code. Tests included in NMS.ActiveMQ need to have license granted to Apache and build with .NET v2.0.

        Show
        Timothy Bish added a comment - Attached is a variation of your test that seems to work fine with trunk and the 1.5.x fixes branch code. Tests included in NMS.ActiveMQ need to have license granted to Apache and build with .NET v2.0.
        Hide
        Timothy Bish added a comment -

        Create a test more in line with the NMS unit tests (works with .NET v2.0 no Tasks) and added the AlwaysSyncSend option as well as catching the InvalidDestinationException that is sometimes expected. Test passes.

        Show
        Timothy Bish added a comment - Create a test more in line with the NMS unit tests (works with .NET v2.0 no Tasks) and added the AlwaysSyncSend option as well as catching the InvalidDestinationException that is sometimes expected. Test passes.
        Hide
        Chris Robison added a comment -

        Yes, but that isn't where the failing is happening. The consumers shutdown if I attempt to send to a non-existent temp queue.

        Show
        Chris Robison added a comment - Yes, but that isn't where the failing is happening. The consumers shutdown if I attempt to send to a non-existent temp queue.
        Hide
        Timothy Bish added a comment -

        Have you tried setting the option AlwaysSyncSend on the connection, this will let the sender wait for a response from the broker which will either indicate success or throw an exception if the send fails. With an Async send the broker will send back a BrokerError that is probably why your connection is getting shut down.

        Show
        Timothy Bish added a comment - Have you tried setting the option AlwaysSyncSend on the connection, this will let the sender wait for a response from the broker which will either indicate success or throw an exception if the send fails. With an Async send the broker will send back a BrokerError that is probably why your connection is getting shut down.
        Hide
        Jim Gomes added a comment - - edited

        I don't think there is a bug currently filed against this issue, so this one will do. I actually spent some time yesterday working on this unit test before I saw your bug entry. I tried refactoring the test to see if it would fail even if different Sessions were used, and it still does. Once it fails, all of the consumers on that Connection are closed, regardless of Session.

        Show
        Jim Gomes added a comment - - edited I don't think there is a bug currently filed against this issue, so this one will do. I actually spent some time yesterday working on this unit test before I saw your bug entry. I tried refactoring the test to see if it would fail even if different Sessions were used, and it still does. Once it fails, all of the consumers on that Connection are closed, regardless of Session.
        Hide
        Chris Robison added a comment -

        Yeah, it's pretty much the same concept. Is there another bug already filed against it?

        Show
        Chris Robison added a comment - Yeah, it's pretty much the same concept. Is there another bug already filed against it?
        Hide
        Jim Gomes added a comment - - edited

        I think this problem may already be exposed by the existing unit test TestConsumeAfterPublishFailsForDestroyedTempDestination in the TempDestinationTest.cs file. Can you confirm if this is the same problem or not? (Currently, this unit test fails.)

        Show
        Jim Gomes added a comment - - edited I think this problem may already be exposed by the existing unit test TestConsumeAfterPublishFailsForDestroyedTempDestination in the TempDestinationTest.cs file. Can you confirm if this is the same problem or not? (Currently, this unit test fails.)
        Hide
        Chris Robison added a comment -

        The basic AMQ configuration I'm using.

        Show
        Chris Robison added a comment - The basic AMQ configuration I'm using.
        Hide
        Chris Robison added a comment -

        NUnit test case to show the problem, client and server logs.

        Show
        Chris Robison added a comment - NUnit test case to show the problem, client and server logs.

          People

          • Assignee:
            Timothy Bish
            Reporter:
            Chris Robison
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development