jbertram commented on PR #4899: URL: https://github.com/apache/activemq-artemis/pull/4899#issuecomment-2302663039
> I am not sure I understand why should reconnect-attempts=0 be considered better in our case. It's better because it actually fits your use-case. Since persistence is disabled then _every_ time a broker restarts it will have a new identity. This is true even if you weren't on K8s. Since the broker's identity will change that means the cluster-connection will also need to be torn down and recreated so there's no reason to attempt to reconnect. Reconnecting a cluster connection only makes sense if the _same exact_ server is coming back. With `reconnect-attempts` > 0 and your fix from this PR then when a node leaves the cluster and gets recreated with a new identity using the same IP address the cluster-connection will reconnect only to then be destroyed when its discovered that the node's identity has changed. It would make more sense to simply _not_ reconnect (i.e. using `0` for `reconnect-attempts`) and allow the node to join the cluster as any other new node would. This approach follows the current design and intended function of the cluster-connection. > All these are supposed to be valid configurations, right? In your case I would say the configuration is not valid. To be clear, not all configurations are valid for all use-cases. For example, if your use-case required messages to survive a broker restart then setting `persistence-enabled` to `false` would be technically possible, but it would not be valid. > Even if our current configuration is not the "best" one by some criteria, why should this bug remain in the code? It's not that your current configuration is not the "best." It's that your current configuration is directly leading to the problem with the stale `MessageFlowRecordImpl` and that could be fixed simply by changing your configuration. When evaluating which changes should be merged into the code-base one must weigh the risk of the change (e.g. additional complexity, possibility for unintended consequences, performance impact, etc.) against the benefit. In this case I do not believe the benefits outweigh the risks given the fact that you're not using a valid configuration for your use-case. Ultimately I'm not sure I'd actually consider the current behavior a bug given that a valid configuration solves the issue. That said, there may, in fact, be another bug that you hit with the valid configuration. I expect this bug would impact other users and would be worth investigating and resolving. I empathize with you about the lost work/time regarding this PR. I know the feeling because I've lost lots of work myself when my own PRs were rejected. Ideally this is all part of the process to make the software better for everyone in the end. Of course, you are free to maintain a fork of ActiveMQ Artemis and apply your own patches (e.g. this one) before deploying in your environment. That's one of the many great benefits of open source. -- 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
