You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "brusdev (via GitHub)" <gi...@apache.org> on 2025/04/03 16:31:19 UTC
[PR] ARTEMIS-5382 Scan cluster topology to merge subgroups [activemq-artemis]
brusdev opened a new pull request, #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603
After the first cluster connection a broker only connects to the other brokers in its topology. Temporary network issues can cause cluster subgroups. In case of cluster subgroups the topology only contains the broker of its subgroup. So the cluster subgroups will remain separate.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2027973247
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/SymmetricClusterTest.java:
##########
@@ -1396,7 +1408,12 @@ public void testNoLocalQueueLoadBalancedQueues() throws Exception {
@Test
public void testStartStopServers() throws Exception {
- doTestStartStopServers();
+ doTestStartStopServers(false);
+ }
+
+ @Test
+ public void testStartStopServersWithPartition() throws Exception {
+ doTestStartStopServers(true);
}
protected void validateTopologSize(int expectedSize, int... serverParameters) throws Exception {
Review Comment:
Typo...should be `validateTopologySize`
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#issuecomment-2776911500
The wildfly Testsuite has an extensive and additional number of tests on clustering. IMO We should ask help from @ehsavoie to validate this change in there before we merge 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
---------------------------------------------------------------------
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2042302335
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java:
##########
@@ -239,6 +239,17 @@ public boolean isSameParams(TransportConfiguration that) {
return true;
}
+ public boolean isSameTarget(TransportConfiguration otherConfig) {
+ if (this.getFactoryClassName().contains("Netty") && otherConfig.getFactoryClassName().contains("Netty")) {
+ return Objects.equals(this.params.get("host"), otherConfig.params.get("host")) &&
Review Comment:
I added a check for the resolved addresses when the ports match and the host names don't match.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#issuecomment-2776908619
what is the TL;DR of this change?
I think that the main logic is in ClusterTopology::updateTopology... but what does it do?
It is connecting to all the nodes in the topology and waiting for the Topology?
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2042325658
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -2630,6 +2630,14 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="topology-scanner-attempts" type="xsd:int" default="-1" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ maximum number of topology scanner attempts, -1 means 'no limits'
Review Comment:
I changed the default attempts to 30 because it is sufficient to update the topology when the cluster connection uses a discovery group and doesn't cause infinite reties in case of a cluster connection configuration with a wrong static connector.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "gtully (via GitHub)" <gi...@apache.org>.
gtully commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2034846528
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -2630,6 +2630,14 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="topology-scanner-attempts" type="xsd:int" default="-1" maxOccurs="1" minOccurs="0">
+ <xsd:annotation>
+ <xsd:documentation>
+ maximum number of topology scanner attempts, -1 means 'no limits'
Review Comment:
Is there any down side to leaving this scanner running, with a scan period. 0, don't run, > 0 delay X between runs. Is it possible for a subgroup to evolve without a restart, but just by temp network partition. If it is possible, then having the scanner run in the background always by default would make sense.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "gtully (via GitHub)" <gi...@apache.org>.
gtully commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2034850966
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/TransportConfiguration.java:
##########
@@ -239,6 +239,17 @@ public boolean isSameParams(TransportConfiguration that) {
return true;
}
+ public boolean isSameTarget(TransportConfiguration otherConfig) {
+ if (this.getFactoryClassName().contains("Netty") && otherConfig.getFactoryClassName().contains("Netty")) {
+ return Objects.equals(this.params.get("host"), otherConfig.params.get("host")) &&
Review Comment:
does dns resolution come into play for the comparison? if a host is resolvable under two ips, would we try and cluster twice?
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2028402096
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/SymmetricClusterTest.java:
##########
@@ -1396,7 +1408,12 @@ public void testNoLocalQueueLoadBalancedQueues() throws Exception {
@Test
public void testStartStopServers() throws Exception {
- doTestStartStopServers();
+ doTestStartStopServers(false);
+ }
+
+ @Test
+ public void testStartStopServersWithPartition() throws Exception {
+ doTestStartStopServers(true);
}
protected void validateTopologSize(int expectedSize, int... serverParameters) throws Exception {
Review Comment:
Good catch, I'll fix it in this PR.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#issuecomment-2778047591
> what is the TL;DR of this change?
>
> I think that the main logic is in ClusterTopology::updateTopology... but what does it do?
>
> It is connecting to all the nodes in the topology and waiting for the Topology?
ClusterTopology::updateTopology connects to all known nodes (static connectors or discovered entries) not included in the cluster topology to update it. This is necessary because the only way to the cluster topology is to receive a cluster topology message or a node announcement from another cluster node.
In case of cluster subgroups the each cluster node only receives updates related to its subgroup. To unlock this status and merge the subgroups a node of one subgroup needs to connect to a node of another subgroup to update the topology.
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603
--
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-5382 Scan cluster topology to merge subgroups [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #5603:
URL: https://github.com/apache/activemq-artemis/pull/5603#discussion_r2027733373
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java:
##########
@@ -593,6 +617,129 @@ public void onConnection(ClientSessionFactoryInternal sf) {
// false,
// localMember.getConnector().a,
// localMember.getConnector().b);
+
+ startTopologyScanner();
+ }
+
+ @Override
+ public void connectorsChanged(List<DiscoveryEntry> newConnectors) {
+ discoveryEntries = newConnectors;
+
+ if (topology.getMembers().size() > 1) {
+ startTopologyScanner();
+ }
+ }
+
+ public TopologyScanner getTopologyScanner() {
+ return topologyScanner;
+ }
+
+ private synchronized void startTopologyScanner() {
+ if (topologyScannerAttempts != 0 && !stopping) {
+ if (topologyScanner == null) {
+ topologyScanner = new TopologyScanner(scheduledExecutor, executor, retryInterval, TimeUnit.MILLISECONDS, true);
+ }
+
+ if (!topologyScanner.isStarted()) {
+ topologyScanner.start();
+ }
+
+ topologyScannerCounter.set(0);
+
+ topologyScanner.delay();
+ }
+ }
+
+ public final class TopologyScanner extends ActiveMQScheduledComponent {
+ private volatile boolean running = false;
+
+ public boolean isRunning() {
+ return running;
+ }
+
+ TopologyScanner(ScheduledExecutorService scheduledExecutorService, Executor executor, long checkPeriod, TimeUnit timeUnit, boolean onDemand) {
+ super(scheduledExecutorService, executor, checkPeriod, timeUnit, onDemand);
+ }
+
+ @Override
+ public boolean delay() {
+ running = true;
+
+ return super.delay();
+ }
+
+ @Override
+ public void run() {
+ TransportConfiguration[] transportConfigurations = null;
+
+ if (clusterConnector instanceof DiscoveryClusterConnector) {
+ List<DiscoveryEntry> discoveredEntries = discoveryEntries;
+
+ if (discoveredEntries != null) {
+ transportConfigurations = discoveryEntries.stream()
+ .map(discoveryEntry -> discoveryEntry.getConnector())
+ .toArray(TransportConfiguration[]::new);
+ } else {
+ logger.debug("No discovered entries");
+ transportConfigurations = new TransportConfiguration[0];
+ }
+ } else if (clusterConnector instanceof StaticClusterConnector) {
+ transportConfigurations = serverLocator.getStaticTransportConfigurations();
+ } else {
+ throw new IllegalStateException("Cluster connector not supported");
+ }
+
+ boolean topologyUpdated = updateTopology(transportConfigurations);
+
+ int topologyScannerCount = topologyScannerCounter.incrementAndGet();
+
+ boolean retry = (topologyScannerAttempts == -1 || topologyScannerCount < topologyScannerAttempts);
+
+ if (!topologyUpdated && !stopping && retry) {
+ delay();
+ } else {
+ running = false;
+
+ if (!topologyUpdated && !stopping) {
+ ActiveMQServerLogger.LOGGER.incompleteClusterTopology(name.toString(), topology, topology.getMembers().toString());
+ }
+ }
+ }
+
+ private boolean updateTopology(TransportConfiguration[] transportConfigurations) {
+ boolean result = true;
+
+ for (TransportConfiguration transportConfiguration : transportConfigurations) {
+ if (!topology.getMembers().stream().anyMatch(member -> connector.isSameTarget(transportConfiguration) ||
+ member.getPrimary() != null && member.getPrimary().isSameTarget(transportConfiguration) ||
+ member.getBackup() != null && member.getBackup().isSameTarget(transportConfiguration))) {
+
+ try (ServerLocatorInternal targetLocator = new ServerLocatorImpl(topology, true, transportConfiguration)) {
+ targetLocator.setReconnectAttempts(0);
+ targetLocator.setInitialConnectAttempts(0);
+ targetLocator.setConnectionTTL(connectionTTL);
+ targetLocator.setCallTimeout(serverLocator.getCallTimeout());
+ targetLocator.setNodeID(serverLocator.getNodeID());
+ targetLocator.setClusterTransportConfiguration(serverLocator.getClusterTransportConfiguration());
+ targetLocator.setIdentity("(Cluster-topology-scanner::" + server.toString() + ")");
+
+ try {
+ try (ClientSessionFactoryInternal targetClientSessionFactory = targetLocator.connect()) {
+ targetClientSessionFactory.waitForTopology(serverLocator.getCallTimeout(), TimeUnit.MILLISECONDS);
+ }
+ } catch (ActiveMQException e) {
Review Comment:
I would log the outcome of this at least as a log.debug
--
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