ARTEMIS-3719 DLA and expiry incorrect w/temp-queue-namespace #3984
Conversation
When using a temporary queue with a `temporary-queue-namespace` the `AddressSettings` lookup wasn't correct. This commit fixes that and refactors `QueueImpl` a bit so that it holds a copy of its `AddressSettings` rather than looking them up all the time. If any relevant `AddressSettings` changes the `HierarchicalRepositoryChangeListener` implementation will still refresh the `QueueImpl` appropriately. The `QueueControlImpl` was likewise changed to get the dead-letter address and expiry address directly from the `QueueImpl` rather than looking them up in the `AddressSettings` repository. I modified some code that came from ARTEMIS-734, but I ran the test that was associated with that Jira (i.e. `o.a.a.a.t.i.c.d.ExpireWhileLoadBalanceTest`) and it passed so I think that should be fine. There actually was no test included with the original commit. One was added later so it's hard to say for sure it exactly captures the original issue.
| } | ||
|
|
||
| if (messageExpiryAddress != null) { | ||
| if (addressSettings.getExpiryAddress() != null) { |
There was a problem hiding this comment.
We used to get the address from the message because of original queue versus current queue.
I believe we would have tests validating that.. (if no testa re failing, there's still a semantic change here I guess we need to be careful).
There was a problem hiding this comment.
I looked at the commit for that change and it wasn't clear what it was for since there was no test associated with it. You added a test for it later (i.e. o.a.a.a.t.i.c.d.ExpireWhileLoadBalanceTest) which I ran after my changes and it succeeded. It looked like a semantic change, but I couldn't verify it with any existing test. It's still not clear to me what the desired behavior would be for messages that expire in a SnF queue. Are they supposed to go to the expiry address configured for where they were originally sent? If so, it's not clear to me why that's better than going to the expiry address configured for the SnF.
There was a problem hiding this comment.
ok.. thanks.. I just wanted to make sure you looked into that.
I'm running the whole test suite to make sure.
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
Show resolved
Hide resolved
When using a temporary queue with a
temporary-queue-namespacetheAddressSettingslookup wasn't correct. This commit fixes that andrefactors
QueueImpla bit so that it holds a copy of itsAddressSettingsrather than looking them up all the time. If anyrelevant
AddressSettingschanges theHierarchicalRepositoryChangeListenerimplementation will stillrefresh the
QueueImplappropriately.The
QueueControlImplwas likewise changed to get the dead-letteraddress and expiry address directly from the
QueueImplrather thanlooking them up in the
AddressSettingsrepository.I modified some code that came from ARTEMIS-734, but I ran the test that
was associated with that Jira (i.e.
o.a.a.a.t.i.c.d.ExpireWhileLoadBalanceTest) and it passed so I thinkthat should be fine. There actually was no test included with the
original commit. One was added later so it's hard to say for sure it
exactly captures the original issue.