cshannon commented on code in PR #2475:
URL: https://github.com/apache/activemq/pull/2475#discussion_r3822769690
##########
activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java:
##########
@@ -1328,8 +1328,13 @@ public void onCompletion(FutureResponse future) {
} else if (command.isBrokerInfo()) {
futureLocalBrokerInfo.set((BrokerInfo) command);
} else if (command.isShutdownInfo()) {
- LOG.info("{} Shutting down {}",
configuration.getBrokerName(), configuration.getName());
- stop();
+ if (brokerService.isStopping() ||
brokerService.isStopped()) {
Review Comment:
I took a closer look and a couple of things:
1. It looks like serviceLocalCommand()
[doesn't](https://github.com/mattrpav/activemq/blob/3a420863659ac00d6435e23e446586a5d76fe2ca/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java#L1252)
process commands if the bridge is stopped and marked as disposed, so checking
for that flag isn't necessary. This is also good as my main concern here is
triggering a restart of the bridge during proper shut down and this wouldn't
get executed if stopped. However, it's still interesting that the method will
handle ShutdownInfo and do a stop so I am wondering what other legitimate cases
it could be used so I am still looking and not sure it's a good idea to do
this. The bridge stuff is confusing and there are 2 or 3 transports (if duplex)
running.
2. The second thing I noticed is that the abort slow consumer strategy on
connection close should trigger an IOException on the connection which I would
think should cause the bridge to detect the error and reconnect. It is done
[here](https://github.com/apache/activemq/blob/e0dca51c19d9c39e5e63915ff8ceb5ad1a2277cf/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java#L165).
I'm assuming that doesn't do that? I'm wondering if we could improve this
somehow.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
