cshannon commented on code in PR #2475:
URL: https://github.com/apache/activemq/pull/2475#discussion_r3822490301
##########
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:
Shouldn't this be checking the started/disposed flags on the bridge and not
the broker?
This PR doesn't make sense with how it is currently constructed (checking if
the broker is shut down) because it is just a work around to solve the issue
with SlowConsumerStrategy but breaks normal shutdown cases.
Bridges can be stopped administratively as you point out while the broker is
still running so If someone shuts down a bridge, it _should not_ be restarted.
Arbitrarily processing an IOException causes it to get restarted. Even if it
didn't, processing an exception doesn't make sense for a normal condition.
One solution for the SlowConsumerStrategy is to just disable it for bridges,
which is already an option and I pointed out previously.
Or if you really need to use it, then figure out a way for it to cause the
exception during shut down to restart but not do it during the normal shutdown
case. During a normal bridge stop there should be flags set (which I pointed
out in the first sentence) so maybe you could check the started or disposed
flags
--
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
