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 2024/09/04 16:40:28 UTC
[PR] ARTEMIS-4182 specify client ID on bridge as node ID [activemq-artemis]
jbertram opened a new pull request, #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208
Automatically fill in the client ID on bridge connections with the node ID of the server where the bridge is configured. This makes the bridge connection easier to identify on the target broker and also mirrors what is already done for both Core and AMQP federation.
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#discussion_r1751789318
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -2598,6 +2606,14 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="client-id" type="xsd:string" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ the identifier to use for the bridge connection; helps with identifying the connection on the remote broker
Review Comment:
bridge -> cluster?
##########
docs/user-manual/clusters.adoc:
##########
@@ -582,6 +583,11 @@ Here we have defined 2 servers that we know for sure will that at least one will
There may be many more servers in the cluster but these will;
be discovered via one of these connectors once an initial connection has been made.
+client-id::
+An optional identifier to use for the bridge connection.
Review Comment:
bridge -> cluster ?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/bridge/BridgeTest.java:
##########
@@ -554,6 +559,44 @@ public void testClientSessionFactoryLeak() throws Exception {
assertTrue(serverLocator.getClientSessionFactoryCount() <= 1);
}
+ @TestTemplate
+ public void testBridgeClientID() throws Exception {
+ final String clientId = RandomUtil.randomString();
+ Map<String, Object> server0Params = new HashMap<>();
+ server0 = createClusteredServerWithParams(isNetty(), 0, true, server0Params);
+
+ Map<String, Object> server1Params = new HashMap<>();
+ addTargetParameters(server1Params);
+ server1 = createClusteredServerWithParams(isNetty(), 1, true, server1Params);
+
+ final String testAddress = "testAddress";
+ final String queueName0 = "queue0";
+ final String forwardAddress = "forwardAddress";
+ final String queueName1 = "queue1";
+
+ TransportConfiguration server1tc = new TransportConfiguration(getConnector(), server1Params);
+
+ HashMap<String, TransportConfiguration> connectors = new HashMap<>();
+ connectors.put(server1tc.getName(), server1tc);
+ server0.getConfiguration().setConnectorConfigurations(connectors);
+
+ List<String> connectorConfig = List.of(server1tc.getName());
+ // intentionally configure a bridge that will fail to connect and attempt to reconnect multiple times quickly
+ server0.getConfiguration().setBridgeConfigurations(List.of(new BridgeConfiguration().setName("bridge1").setQueueName(queueName0).setForwardingAddress(forwardAddress).setRetryInterval(0).setReconnectAttempts(-1).setStaticConnectors(connectorConfig).setClientId(clientId)));
Review Comment:
We should avoid setting infinite reconnects in tests, it just makes it more likely for bad things to happen later, e.g. cross-test contamination when something unexpected isnt handled nicely.
The test only seems to allow 2 seconds for connection, so the attempts + intervals only need to span that.
(I'd also name the bridge based on the test name, helps identify them in logs later in debug, and also useful for finding contaminations)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/bridge/BridgeTest.java:
##########
@@ -554,6 +559,44 @@ public void testClientSessionFactoryLeak() throws Exception {
assertTrue(serverLocator.getClientSessionFactoryCount() <= 1);
}
+ @TestTemplate
+ public void testBridgeClientID() throws Exception {
+ final String clientId = RandomUtil.randomString();
+ Map<String, Object> server0Params = new HashMap<>();
+ server0 = createClusteredServerWithParams(isNetty(), 0, true, server0Params);
+
+ Map<String, Object> server1Params = new HashMap<>();
+ addTargetParameters(server1Params);
+ server1 = createClusteredServerWithParams(isNetty(), 1, true, server1Params);
+
+ final String testAddress = "testAddress";
+ final String queueName0 = "queue0";
+ final String forwardAddress = "forwardAddress";
+ final String queueName1 = "queue1";
+
+ TransportConfiguration server1tc = new TransportConfiguration(getConnector(), server1Params);
+
+ HashMap<String, TransportConfiguration> connectors = new HashMap<>();
+ connectors.put(server1tc.getName(), server1tc);
+ server0.getConfiguration().setConnectorConfigurations(connectors);
+
+ List<String> connectorConfig = List.of(server1tc.getName());
+ // intentionally configure a bridge that will fail to connect and attempt to reconnect multiple times quickly
Review Comment:
Is this true? The test seems to verify a successfull connection
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterConnectionConfigTest.java:
##########
@@ -103,4 +109,30 @@ private void makeSureForwardingFlowControl(int producerWindow, int... indices) t
}
}
+ @Test
+ public void testBridgeClientId() throws Exception {
Review Comment:
Bridge -> Cluster?
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "tabish121 (via GitHub)" <gi...@apache.org>.
tabish121 commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2332168895
I ran a CI job on this and saw several new failures I hadn't seen before, some examples:
> Test and Examples / Test / 2 / org.apache.activemq.artemis.tests.integration.routing.AutoClientIDShardClusterTest.testWithConsistentHashClientIDModTwo() protocol: AMQP
> Test and Examples / Test / 2 / org.apache.activemq.artemis.tests.integration.routing.AutoClientIDShardClusterTest.testWithConsistentHashClientIDModTwo() protocol: CORE
>
Might be worth a look
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "erwindon (via GitHub)" <gi...@apache.org>.
erwindon commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2338905620
:-) :-)
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2334840956
@tabish121, thanks for the heads-up. I see what's happening now after much head-scratching. I'll need to rework this PR so that the client ID is configurable because having it always-on may not fit every use-case.
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#discussion_r1744263184
##########
docs/user-manual/versions.adoc:
##########
@@ -12,6 +12,19 @@ NOTE: If the upgrade spans multiple versions then the steps from *each* version
NOTE: Follow the general upgrade procedure outlined in the xref:upgrading.adoc#upgrading-the-broker[Upgrading the Broker] chapter in addition to any version-specific upgrade instructions outlined here.
+== 2.38.0
+
+https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12315920&version=[Full release notes]
+
+=== Highlights
+
+*
+
+=== Upgrading from 2.37.0
+
+* Due to https://issues.apache.org/jira/browse/ARTEMIS-4182[ARTEMIS-4182] the "client ID" field on bridge connections will not automatically be configured with the node ID of the source broker.
Review Comment:
Good catch! Corrected.
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2338297458
For core connections client ID doesn't _have_ to be unique.
--
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-4182 support client ID config on bridges [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr merged PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2338230847
Configurable sounds like it would be needed in case of ClientID, since isn't it possible to have multiple bridge connections going to the same broker? If so, they cant use the same ClientID. (Aside: the AMQP Federation bit sets a container-id, but it doesnt add the mechanism to request it be enforced as unique, i.e to have it treated like a ClientID)
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#issuecomment-2338379686
Ah, didnt realise the Core client ClientID, and Core JMS client ClientID, aren't the same thing.
--
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-4182 specify client ID on bridge as node ID [activemq-artemis]
Posted by "erwindon (via GitHub)" <gi...@apache.org>.
erwindon commented on code in PR #5208:
URL: https://github.com/apache/activemq-artemis/pull/5208#discussion_r1744254055
##########
docs/user-manual/versions.adoc:
##########
@@ -12,6 +12,19 @@ NOTE: If the upgrade spans multiple versions then the steps from *each* version
NOTE: Follow the general upgrade procedure outlined in the xref:upgrading.adoc#upgrading-the-broker[Upgrading the Broker] chapter in addition to any version-specific upgrade instructions outlined here.
+== 2.38.0
+
+https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12315920&version=[Full release notes]
+
+=== Highlights
+
+*
+
+=== Upgrading from 2.37.0
+
+* Due to https://issues.apache.org/jira/browse/ARTEMIS-4182[ARTEMIS-4182] the "client ID" field on bridge connections will not automatically be configured with the node ID of the source broker.
Review Comment:
"not" or "now"
--
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