You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2023/01/10 20:04:53 UTC
[GitHub] [activemq] NikitaShupletsov opened a new pull request, #953: [AMQ-8354] Add helper methods needed for replica plugin.
NikitaShupletsov opened a new pull request, #953:
URL: https://github.com/apache/activemq/pull/953
AMQ-8354
extracted internal changes from https://github.com/apache/activemq/pull/848 to make it easier to review.
Added some new methods that will be used only in replica plugin.
Updated `processDispatchNotification` and related methods to work correctly(it's not been used since pure master-slave feature was deleted). it will also be used in the plugin
--
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
[GitHub] [activemq] NikitaShupletsov commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
NikitaShupletsov commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1068483608
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java:
##########
@@ -2387,8 +2495,8 @@ private QueueMessageReference getMatchingMessage(MessageDispatchNotification mes
messages.reset();
while (messages.hasNext()) {
MessageReference node = messages.next();
- messages.remove();
Review Comment:
The idea of this change is to delete only the message that we are looking for. Because deleting all messages up to the one we need prevents reading them in future. Basically if we delete them here, we will not be able to get them anymore.
--
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
[GitHub] [activemq] NikitaShupletsov commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
NikitaShupletsov commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1068481508
##########
activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java:
##########
@@ -54,7 +54,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class SchedulerBroker extends BrokerFilter implements JobListener {
+public class SchedulerBroker extends MutableBrokerFilter implements JobListener {
Review Comment:
I wasn't able to find any information about possible `AtomicReference` overhead. I believe if there is some, it should be almost not noticeable
--
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
[GitHub] [activemq] michaelandrepearce commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1069058454
##########
activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java:
##########
@@ -54,7 +54,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class SchedulerBroker extends BrokerFilter implements JobListener {
+public class SchedulerBroker extends MutableBrokerFilter implements JobListener {
Review Comment:
Is sync and see docs of the class you're changing too.
--
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
[GitHub] [activemq] michaelandrepearce commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1066683378
##########
activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java:
##########
@@ -54,7 +54,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class SchedulerBroker extends BrokerFilter implements JobListener {
+public class SchedulerBroker extends MutableBrokerFilter implements JobListener {
Review Comment:
Wont this be overhead now for use cases not needing this? due to the sync in mutable thats needed.
--
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
[GitHub] [activemq] michaelandrepearce commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1066685307
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java:
##########
@@ -2387,8 +2495,8 @@ private QueueMessageReference getMatchingMessage(MessageDispatchNotification mes
messages.reset();
while (messages.hasNext()) {
MessageReference node = messages.next();
- messages.remove();
Review Comment:
Why move this?
--
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
[GitHub] [activemq] michaelandrepearce commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1069058950
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java:
##########
@@ -2387,8 +2495,8 @@ private QueueMessageReference getMatchingMessage(MessageDispatchNotification mes
messages.reset();
while (messages.hasNext()) {
MessageReference node = messages.next();
- messages.remove();
Review Comment:
And? That's now unneeded change in behaviour
--
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] [AMQ-8354] Add helper methods needed for replica plugin. [activemq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #953:
URL: https://github.com/apache/activemq/pull/953#issuecomment-3988183153
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] [AMQ-8354] Add helper methods needed for replica plugin. [activemq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
URL: https://github.com/apache/activemq/pull/953
--
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
[GitHub] [activemq] NikitaShupletsov commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
NikitaShupletsov commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1066330795
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java:
##########
@@ -1296,9 +1297,36 @@ public QueueMessageReference getMessage(String id) {
return null;
}
+ public List<MessageId> getAllMessageIds() throws Exception {
Review Comment:
here out of memory error is possible: https://github.com/apache/activemq/pull/848#discussion_r1062161446
will address it
--
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
[GitHub] [activemq] michaelandrepearce commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1066683378
##########
activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java:
##########
@@ -54,7 +54,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class SchedulerBroker extends BrokerFilter implements JobListener {
+public class SchedulerBroker extends MutableBrokerFilter implements JobListener {
Review Comment:
Wont this be overhead now for use cases not needing this? due to the sync in mutable thats needed. e.g. this will introduce a perf drop for existing users
--
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
[GitHub] [activemq] NikitaShupletsov commented on a diff in pull request #953: [AMQ-8354] Add helper methods needed for replica plugin.
Posted by GitBox <gi...@apache.org>.
NikitaShupletsov commented on code in PR #953:
URL: https://github.com/apache/activemq/pull/953#discussion_r1071640842
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java:
##########
@@ -2387,8 +2495,8 @@ private QueueMessageReference getMatchingMessage(MessageDispatchNotification mes
messages.reset();
while (messages.hasNext()) {
MessageReference node = messages.next();
- messages.remove();
Review Comment:
I am not sure why. this method is used to "deliver" a specific message by id to a specific consumer. we still want to have the access to all other messages. in case we want to "deliver" them too, or in case there is a real consumer
--
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