You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jbertram (via GitHub)" <gi...@apache.org> on 2023/11/30 19:13:23 UTC

[PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

jbertram opened a new pull request, #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697

   (no comment)


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram merged PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1838461087

   @erwindon 
   `the issue here was that deletion using JMX was not updated in the journal, and the PR seems to fix just that.`
   
   Yes, and I'm saying the same thing happens for diverts configured through the XML file, but with the addition that the divert is created again directly after getting removed instead of after a restart.
   
   I do see now though that the setting I mentioned above `config-delete-diverts = FORCE` might not help in your use-case after all but for diverts configured through XML it will.
   
   Regardless, I'm not arguing against the change, rather that the aforementioned config should also be removed as part of it since it will become non-functional after (as far as I can tell).


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1838020631

   Strange coincidence but I stumbled upon this issue just a few days ago as well, but from having added and then removed the divert through xml config (where the divert just remains active instead)... while I highly agree that this change is good and is how the broker should work, the current behavior is actually by design... you are supposed to set the address-setting `config-delete-diverts = FORCE` to enable removal of the diverts.
   
   This is controlled by:
   `org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl#recoverStoredDiverts()`
   
   Perhaps this setting should just be removed as well, since with this PR it will become non-functional?


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1838778315

   The JMX use-case that this PR addresses should be independent of the XML use-case you are referencing, @AntonRoskvist. Even after this change the `config-delete-diverts` setting will still be valid for changes to the XML.


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1843272830

   @AntonRoskvist, you're absolutely correct. Nice catch! I've updated the code to adjust for this.
   
   @erwindon, by the way, I think this may be related to an issue you've see with retroactive address and diverts not getting cleaned up properly.


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "erwindon (via GitHub)" <gi...@apache.org>.
erwindon commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1838263755

   @AntonRoskvist 
   > Perhaps this setting should just be removed as well, since with this PR it will become non-functional?
   the issue here was that deletion using JMX was not updated in the journal, and the PR seems to fix just that.
   for completeness... I don't have any addresses/queues/diverts in my config. all objects are auto-create(addresses/queues/retroactive) and auto-delete(addresses/queues). the few old diverts, created by retroactive, are deleted manually but had the above problem.


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4521 deleting divert using mngmnt API doesn't remove binding from journal [activemq-artemis]

Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on PR #4697:
URL: https://github.com/apache/activemq-artemis/pull/4697#issuecomment-1839413768

   It is not independent, reloading file configuration uses the same method changed with this PR, i.e `org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl#destroyDivert()` which now removes the divert from storage.
   
   After that the config reload runs: `org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl#recoverStoredDiverts()`
   which basically does: 
   
   `if (divertWithStoredConfig.isDeleted() && adressSettings.config-delete-diverts == FORCE)` -> remove stored divert config
   `else` -> recreate divert
   
   So before this change, removing a divert from xml config still left it active (with some fraction of a second in downtime), provided that `config-delete-diverts = FORCE` is not set. Default is `OFF` i.e never remove a stored divert
   
   After this change, removing a divert from xml config removes the divert, regardless what value "config-delete-diverts" has. It also removes the config from storage. 
   
   I think the change made in this PR makes a lot of sense, both for the JMX and the file configuration use case. I'm also saying that the address setting "config-delete-diverts" will no longer have any effect anywhere on anything.


-- 
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-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org