gemmellr commented on code in PR #5327:
URL: https://github.com/apache/activemq-artemis/pull/5327#discussion_r1826115570
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -274,7 +274,8 @@ private void
checkIDSupplier(NodeStoreFactory<MessageReference> nodeStoreFactory
private final StorageManager storageManager;
- private volatile AddressSettings addressSettings;
+ // IInstead of looking up the AddressSettings every time, we cache and
monitor it through onChange
Review Comment:
```suggestion
// Instead of looking up the AddressSettings every time, we cache and
monitor it through onChange
```
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -2129,14 +2130,26 @@ public void expire(final MessageReference ref) throws
Exception {
* hence no information about delivering statistics should be
updated. */
@Override
public void expire(final MessageReference ref, final ServerConsumer
consumer, boolean delivering) throws Exception {
- if (addressSettings.getExpiryAddress() != null) {
- createExpiryResources();
+ AddressSettings settingsToUse =
getMessageAddressSettings(ref.getMessage());
Review Comment:
Yeah I wasn't suggesting to combine them as they did seem sufficiently
different on the backend to justify the two methods. Its just the initial bits
were very similar so it would have been nice for them to be set out the same
way for readability.
Combining them seems to have rearranged at least the order of some stuff for
one method, or added some stuff around bindings that other method didnt do
before...I'd need to take a much fuller look to see what I think of those
changes, but I need to finish up just now.
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
