You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "kenliao94 (via GitHub)" <gi...@apache.org> on 2024/11/14 20:46:49 UTC
[PR] [WIP] Add option to clean scheduled messages on startup [activemq]
kenliao94 opened a new pull request, #1352:
URL: https://github.com/apache/activemq/pull/1352
Working in progress: add an option to delete all scheduled message on startup.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #1352:
URL: https://github.com/apache/activemq/pull/1352#discussion_r2386755518
##########
activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java:
##########
@@ -184,6 +184,7 @@ public class BrokerService implements Service {
private JmsConnector[] jmsBridgeConnectors; // these are Jms to Jms bridges
// to other jms messaging systems
private boolean deleteAllMessagesOnStartup;
+ private boolean deleteAllScheduledMessagesOnStartup = false;
Review Comment:
I'm still in favor on a dedicated flag here, because the semantic is different so the behavior is.
To reduce the "side effect" for existing users on ActiveMQ 6.x, I'm in favor of `deleteAllScheduledMessagesOnStartup`. We can revisit this for 7.x.
@mattrpav @kenliao94 thoughts ?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2504621193
Thanks for the feedback. I will add a unit test.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on code in PR #1352:
URL: https://github.com/apache/activemq/pull/1352#discussion_r1865257087
##########
activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java:
##########
@@ -184,6 +184,7 @@ public class BrokerService implements Service {
private JmsConnector[] jmsBridgeConnectors; // these are Jms to Jms bridges
// to other jms messaging systems
private boolean deleteAllMessagesOnStartup;
+ private boolean deleteAllScheduledMessagesOnStartup = false;
Review Comment:
I see where you are coming from. Initially I would expect deleteAllMessagesOnStartup will include scheduled messages. I even opened a Jira ticket on it. https://issues.apache.org/jira/projects/AMQ/issues/AMQ-9572?filter=allissues from @jbonofre feedback we should have a separated flag for scheduled messages.
Regarding the proposal in v7.x I think if we have the consensus that deleteAllMessagesOnStartup should include scheduled messages then it makes sense to me.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1352:
URL: https://github.com/apache/activemq/pull/1352#discussion_r1864047656
##########
activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java:
##########
@@ -184,6 +184,7 @@ public class BrokerService implements Service {
private JmsConnector[] jmsBridgeConnectors; // these are Jms to Jms bridges
// to other jms messaging systems
private boolean deleteAllMessagesOnStartup;
+ private boolean deleteAllScheduledMessagesOnStartup = false;
Review Comment:
Scheduled message support is part of the JMS 3.1/2.0 spec. I'm not loving the separate config flag. We don't have a 'deleteAllDurableTopicMessagesOnStartup' flag. It just 'deleteAllmessagesOnStartup'.
I'm thinking it would be better to have a flag to have the scheduler _ignore_ the deleteAllMessagesOnStartup.
Possible path forward:
v6.x - Scheduler ignores deleteAllMessagesOnStartup by default (to maintain existing behavior)
v7.x - Scheduler honors deleteAllMessagesOnStartup by default
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2502015949
The PR is now ready for review.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-3937940585
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [WIP] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2477392936
Hey @mattrpav and @jbonofre I am curious on what you think about the approach here and want to clarify a few things:
1. I was debating between adding this logic to the job scheduler or the persistence layer (following the approach of deleteAllMessages). However, I noticed that `SchedulerBroker` doesn't expose their internal scheduler, and it has public methods that deal directly with `Message` (initially I thought SchedulerBroker is not "message aware" but only "job aware". So I decided to add the logic in SB. What do you think?
2. I wasn't able to find any other non-message jobs (even job in `ByteSequence` type is assumed to be unmarshal into `Message`) scheduled on "JMS" scheduler so I assume that remove all jobs are safe. However, do you know any pitfull for simply remove all jobs?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2576945413
Hey @mattrpav @cshannon and @jbonofre what do you think about this PR? Should we merge it for 6.1.5 release?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2577551268
> Hey @mattrpav @cshannon and @jbonofre what do you think about this PR? Should we merge it for 6.1.5 release?
>
> I recently added a unit test as well.
It should be 6.2.0 just like #1288 , see https://github.com/apache/activemq/pull/1288#issuecomment-2577524572 and https://github.com/apache/activemq/pull/1288#issuecomment-2577528031
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2502884101
@cshannon yeah I was doing that as my first approach. But then I realize I need to somehow expose (by changing the visibility of the field) the name of the scheduler ("JMS") used by `SchedulerBroker`. I didn't like that approach because it creates some sort of coupling. So I decided to handle it where no visibility needed to change.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Add option to clean scheduled messages on startup [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2503661475
Yeah that's fine, the most important thing is it's consistent for the config (which it is)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1352:
URL: https://github.com/apache/activemq/pull/1352#discussion_r1864047656
##########
activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java:
##########
@@ -184,6 +184,7 @@ public class BrokerService implements Service {
private JmsConnector[] jmsBridgeConnectors; // these are Jms to Jms bridges
// to other jms messaging systems
private boolean deleteAllMessagesOnStartup;
+ private boolean deleteAllScheduledMessagesOnStartup = false;
Review Comment:
Scheduled message support is part of the JMS 3.1/2.0 spec. I'm not loving the separate config flag. We don't have a 'deleteAllDurableTopicMessagesOnStartup' flag. It is 'all' messages on startup.
I'm thinking it would be better to have a flag to have the scheduler _ignore_ the deleteAllMessagesOnStartup.
Possible path forward:
v6.x - Scheduler ignores deleteAllMessagesOnStartup by default (to maintain existing behavior)
v7.x - Scheduler honors deleteAllMessagesOnStartup by default
Thoughts?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1352:
URL: https://github.com/apache/activemq/pull/1352#discussion_r1864047656
##########
activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java:
##########
@@ -184,6 +184,7 @@ public class BrokerService implements Service {
private JmsConnector[] jmsBridgeConnectors; // these are Jms to Jms bridges
// to other jms messaging systems
private boolean deleteAllMessagesOnStartup;
+ private boolean deleteAllScheduledMessagesOnStartup = false;
Review Comment:
Scheduled message support is part of the JMS 3.1/2.0 spec. I'm not loving the separate config flag. We don't have a 'deleteAllDurableTopicMessagesOnStartup' flag. It is 'all' messages on startup.
I'm thinking it would be better to have a flag to have the scheduler _ignore_ the deleteAllMessagesOnStartup.
Possible path forward:
v6.x - Scheduler ignores deleteAllMessagesOnStartup by default (to maintain existing behavior)
v7.x - Scheduler honors deleteAllMessagesOnStartup by default
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Add option to clean scheduled messages on startup [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-2502143585
It might be be better to move the cleanup to the persistence layer just because that's how KahaDB handles clearing messages (as was pointed out). But I don't really care too much either way because the flag on the BrokerService is consistent with the other flag so for users it's still a consistent way to configure it. I'll let @mattrpav and @jbonofre comment to see if they have an opinion.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "kenliao94 (via GitHub)" <gi...@apache.org>.
kenliao94 commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-3392328937
hey @jbonofre sounds good to me! When you said "dedicated tag" do you mean a attribute on the XML as it is today?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9593] Add option to clean scheduled messages on startup [activemq]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #1352:
URL: https://github.com/apache/activemq/pull/1352#issuecomment-3345055709
This PR looks good to me for 6.2.0, with the use of a dedicated tag. @mattrpav @cshannon @kenliao94 thoughts ?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact