jbonofre commented on PR #2553: URL: https://github.com/apache/activemq/pull/2553#issuecomment-5603180682
@cshannon I'm fine to do that but not sure it changes anything. I've tested the `BrokerService`: 1. `slave` defaults to `true` and is reset to `true` on every stop. 2. It only flips to `false` inside `startAllConnectors()`. 3. `adminView.setBroker(managedBroker)` (this call makes `BrokerView.broker` non-null) happens in `doStartBroker()` which runs before `startAllConnectors()`. So within a single `BrokerService` instance, `broker == null` implies `isSlave() == true` at every point at startup (there's no case where `broker` is null but `isSlave()` has already gone false. It means: ``` isStandBy() = (broker == null) && brokerService.isSlave() ``` is correct but `&& isSlave()` term can never actually be false while the first is true, given the current ordering in `BrokerService`. So: - It changes no behavior - It adds a dependency on the `slave` for a "guard" whose only real job is null-safety before calling `safeGetBroker()`. - If an user ever reorders `startAllConnectors()`/`adminView.setBroker()` in `BrokerService` (unlikely, but possible), this guard behavior would silently change without anyone touching `BrokerView`. I'm fine to add `isSlave()` check, even if I believe it's useless 😄 -- 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
