You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "tabish121 (via GitHub)" <gi...@apache.org> on 2025/02/10 23:51:56 UTC
[PR] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
tabish121 opened a new pull request, #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493
When demand is removed from a federation consumer and the AMQP link is drained allow the consumer to idle for a configurable time period before closing the link. If demand returns before the idled link is closed we can recover the link by simply granting more credit.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "tabish121 (via GitHub)" <gi...@apache.org>.
tabish121 commented on PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#issuecomment-2659538116
I've squashed the changes and rebased with main.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "tabish121 (via GitHub)" <gi...@apache.org>.
tabish121 commented on code in PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#discussion_r1955342002
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/federation/AMQPFederationConfiguration.java:
##########
@@ -76,12 +77,20 @@ public final class AMQPFederationConfiguration {
public static final boolean DEFAULT_IGNNORE_QUEUE_CONSUMER_PRIORITIES = false;
/**
- * Default timeout (milliseconds) applied to federation receivers that are being closed due to removal
+ * Default timeout (milliseconds) applied to federation receivers that are being stopped due to removal
* of local demand and need to drain link credit and process any in-flight deliveries before closure.
* If the timeout elapses before the link has quiesced the link is forcibly closed.
*/
public static final int DEFAULT_RECEIVER_QUIESCE_TIMEOUT = 60_000;
+ /**
+ * Default timeout (milliseconds) applied to federation receivers that have been stopped due to lack of
+ * local demand. The close delay prevent a link from detaching in cases where demand drops and returns
+ * in quick succession allowing for faster recovery. The idle timeout kicks in once the link has completed
+ * its drain of outstanding credit.
+ */
+ public static final int DEFAULT_RECEIVER_IDLE_TIMEOUT = 60_000;
Review Comment:
I've added a commit which splits the timeout into two separate properties one for address and one for queues with the 60s value for queue consumer and a much reduced 5s value for address consumers. I chose not to retain a top level receiver idle timeout that applied to both at the federation configuration level as that may just make it more confusing in the end.
If this looks good I will squash the two commits but I kept it split for now to show the difference.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "tabish121 (via GitHub)" <gi...@apache.org>.
tabish121 commented on PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#issuecomment-2654818991
> I was expecting some documentation for this new setting, but I couldn't find it. Was that an oversight or does this not need any user-facing docs?
As follow work for this plus many other updates there will need to be rework of the federation docs but that's a bit out of scope here. The defaults on this should suffice for the majority of users as this should be transparent in practice.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#discussion_r1954954935
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/federation/AMQPFederationConfiguration.java:
##########
@@ -76,12 +77,20 @@ public final class AMQPFederationConfiguration {
public static final boolean DEFAULT_IGNNORE_QUEUE_CONSUMER_PRIORITIES = false;
/**
- * Default timeout (milliseconds) applied to federation receivers that are being closed due to removal
+ * Default timeout (milliseconds) applied to federation receivers that are being stopped due to removal
* of local demand and need to drain link credit and process any in-flight deliveries before closure.
* If the timeout elapses before the link has quiesced the link is forcibly closed.
*/
public static final int DEFAULT_RECEIVER_QUIESCE_TIMEOUT = 60_000;
+ /**
+ * Default timeout (milliseconds) applied to federation receivers that have been stopped due to lack of
+ * local demand. The close delay prevent a link from detaching in cases where demand drops and returns
+ * in quick succession allowing for faster recovery. The idle timeout kicks in once the link has completed
+ * its drain of outstanding credit.
+ */
+ public static final int DEFAULT_RECEIVER_IDLE_TIMEOUT = 60_000;
Review Comment:
I'd maybe be inclined to lower this given it acts as a default for both queue and address types. Its fine for queue federation, but given the scope for oddities to arise in some Address federation cases due to the federation consumer (and its related queue) sticking around between source demand, it seems like lower default would be appropriate there. Although I realise it can of course be configured to 0 both overall and/or on a per-policy basis for anyone that needs to.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "tabish121 (via GitHub)" <gi...@apache.org>.
tabish121 commented on code in PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#discussion_r1955201294
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/federation/AMQPFederationConfiguration.java:
##########
@@ -76,12 +77,20 @@ public final class AMQPFederationConfiguration {
public static final boolean DEFAULT_IGNNORE_QUEUE_CONSUMER_PRIORITIES = false;
/**
- * Default timeout (milliseconds) applied to federation receivers that are being closed due to removal
+ * Default timeout (milliseconds) applied to federation receivers that are being stopped due to removal
* of local demand and need to drain link credit and process any in-flight deliveries before closure.
* If the timeout elapses before the link has quiesced the link is forcibly closed.
*/
public static final int DEFAULT_RECEIVER_QUIESCE_TIMEOUT = 60_000;
+ /**
+ * Default timeout (milliseconds) applied to federation receivers that have been stopped due to lack of
+ * local demand. The close delay prevent a link from detaching in cases where demand drops and returns
+ * in quick succession allowing for faster recovery. The idle timeout kicks in once the link has completed
+ * its drain of outstanding credit.
+ */
+ public static final int DEFAULT_RECEIVER_IDLE_TIMEOUT = 60_000;
Review Comment:
Fair point and I thought about it but was thinking of starting simpler with one configuration option for both. After thinking on it I will split the configuration for address and queue consumers as there is a chance of a race if federation of durable subscriptions is in play where it could still read message sent when it was removed and re-added for instance. Queue consumers are fine with a longer timeout as the behavior is the same on the remote regardless of any timeout.
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#issuecomment-2654805604
I was expecting some documentation for this new setting, but I couldn't find it. Was that an oversight or does this not need any user-facing docs?
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493#discussion_r1955913777
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/federation/AMQPFederationConfiguration.java:
##########
@@ -76,12 +77,20 @@ public final class AMQPFederationConfiguration {
public static final boolean DEFAULT_IGNNORE_QUEUE_CONSUMER_PRIORITIES = false;
/**
- * Default timeout (milliseconds) applied to federation receivers that are being closed due to removal
+ * Default timeout (milliseconds) applied to federation receivers that are being stopped due to removal
* of local demand and need to drain link credit and process any in-flight deliveries before closure.
* If the timeout elapses before the link has quiesced the link is forcibly closed.
*/
public static final int DEFAULT_RECEIVER_QUIESCE_TIMEOUT = 60_000;
+ /**
+ * Default timeout (milliseconds) applied to federation receivers that have been stopped due to lack of
+ * local demand. The close delay prevent a link from detaching in cases where demand drops and returns
+ * in quick succession allowing for faster recovery. The idle timeout kicks in once the link has completed
+ * its drain of outstanding credit.
+ */
+ public static final int DEFAULT_RECEIVER_IDLE_TIMEOUT = 60_000;
Review Comment:
The addition looks good, and the new defaults seem a better fit
--
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] ARTEMIS-5304 Allow delayed close of federation consumers [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr merged PR #5493:
URL: https://github.com/apache/activemq-artemis/pull/5493
--
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