[ 
https://issues.apache.org/jira/browse/ARTEMIS-3138?focusedWorklogId=557788&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-557788
 ]

ASF GitHub Bot logged work on ARTEMIS-3138:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 25/Feb/21 08:16
            Start Date: 25/Feb/21 08:16
    Worklog Time Spent: 10m 
      Work Description: franz1981 commented on a change in pull request #3468:
URL: https://github.com/apache/activemq-artemis/pull/3468#discussion_r582625856



##########
File path: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java
##########
@@ -1546,20 +1546,22 @@ public synchronized void 
connectorsChanged(List<DiscoveryEntry> newConnectors) {
       if (receivedTopology) {
          return;
       }
-      TransportConfiguration[] newInitialconnectors = 
(TransportConfiguration[]) Array.newInstance(TransportConfiguration.class, 
newConnectors.size());
 
-      int count = 0;
-      for (DiscoveryEntry entry : newConnectors) {
-         newInitialconnectors[count++] = entry.getConnector();
+      final List<TransportConfiguration> newInitialconnectors = new 
ArrayList<>(newConnectors.size());
 
+      for (DiscoveryEntry entry : newConnectors) {
          if (ha && topology.getMember(entry.getNodeID()) == null) {
             TopologyMemberImpl member = new 
TopologyMemberImpl(entry.getNodeID(), null, null, entry.getConnector(), null);
             // on this case we set it as zero as any update coming from server 
should be accepted
             topology.updateMember(0, entry.getNodeID(), member);
          }
+         // ignore its own transport connector
+         if (!entry.getConnector().equals(clusterTransportConfiguration)) {

Review comment:
       > As long as your own connection is getting into the Topology for the 
clients, this is ok.
   
   That's why the check 
`entry.getConnector().equals(clusterTransportConfiguration)` is placed after 
updating `topology`: the new `entry` should affect the topology. but not the 
`initialConnectors` list, because the latter would make it create a connection 
to itself ie is not a valid connector.
   
   I'm just concerned that the we throw multiple times 
`ActiveMQClientMessageBundle.BUNDLE.noTCForSessionFactory()` that's a 
`ActiveMQException` and it will cause the `ConnectRunnable` to re-schedule 
itself on `retry interval`, see:
   ```java
      /**
       * used for making the initial connection in the cluster
       */
      private final class ConnectRunnable implements Runnable {
   
         private final ServerLocatorInternal serverLocator;
   
         private ConnectRunnable(ServerLocatorInternal serverLocator) {
            this.serverLocator = serverLocator;
         }
   
         @Override
         public void run() {
            try {
               if (started) {
                  serverLocator.connect();
                  if (serverLocator == replicationLocator) {
                     replicationClusterConnectedLatch.countDown();
                  }
               }
            } catch (ActiveMQException e) {
               if (!started) {
                  return;
               }
               if (logger.isDebugEnabled()) {
   
                  logger.debug("retry on Cluster Controller " + 
System.identityHashCode(ClusterController.this) + " server = " + server);
               }
               server.getScheduledPool().schedule(this, 
serverLocator.getRetryInterval(), TimeUnit.MILLISECONDS);
            }
         }
      }
   ```
   Throwing many exceptions isn't nice and I see that it will keep on doing it 
during the whole life-cycle of a live server *if* there is a single pair, 
because the backup won't have any acceptor opened to allow the live to connect 
to 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.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 557788)
    Time Spent: 1h  (was: 50m)

> Shared Nothing Live broker shouldn't try to connect to itself
> -------------------------------------------------------------
>
>                 Key: ARTEMIS-3138
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3138
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>            Reporter: Francesco Nigro
>            Priority: Major
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> While starting a shared nothing master broker configured with discovery it 
> performs discovery operations that could end up connecting the broker to 
> itself.
>  The discoveries happen in different code paths:
>  * while checking for a live server 
>  * on cluster manager deploy
> Because the broker can receive its same broadcast connector transport and for 
> some reason it won't filter it out.
> The existing logic to filter out connectors is based on the Node ID source of 
> the broadcast event vs the one on the local server locator.
>  
> The former case won't end up connecting the broker to itself, because the 
> broker acceptors are still closed and won't accept any connection, while the 
> latter yes and the established connection remain alive for the whole broker 
> life-time.
> The relevant stack traces are:
> {code:java}
>       at 
> org.apache.activemq.artemis.core.cluster.DiscoveryGroup.<init>(DiscoveryGroup.java:96)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.createDiscoveryGroup(ServerLocatorImpl.java:298)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.startDiscovery(ServerLocatorImpl.java:275)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.initialize(ServerLocatorImpl.java:263)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.createSessionFactory(ServerLocatorImpl.java:650)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.connect(ServerLocatorImpl.java:549)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.connectNoWarnings(ServerLocatorImpl.java:555)
>       at 
> org.apache.activemq.artemis.core.server.impl.SharedNothingLiveActivation.isNodeIdUsed(SharedNothingLiveActivation.java:331)
>       at 
> org.apache.activemq.artemis.core.server.impl.SharedNothingLiveActivation.run(SharedNothingLiveActivation.java:102)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.internalStart(ActiveMQServerImpl.java:634)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.start(ActiveMQServerImpl.java:558)
>       at 
> org.apache.activemq.artemis.integration.FileBroker.start(FileBroker.java:64)
>       at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:115)
>       at 
> org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:154)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:102)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:129)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-1)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:134)
>       at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:50)
> {code}
>  And
> {code:java}
>       at 
> org.apache.activemq.artemis.core.cluster.DiscoveryGroup.<init>(DiscoveryGroup.java:96)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.createDiscoveryGroup(ServerLocatorImpl.java:298)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.startDiscovery(ServerLocatorImpl.java:275)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.initialize(ServerLocatorImpl.java:263)
>       at 
> org.apache.activemq.artemis.core.server.cluster.ClusterController.configAndAdd(ClusterController.java:230)
>       at 
> org.apache.activemq.artemis.core.server.cluster.ClusterController.addClusterConnection(ClusterController.java:197)
>       at 
> org.apache.activemq.artemis.core.server.cluster.ClusterManager.deployClusterConnection(ClusterManager.java:612)
>       at 
> org.apache.activemq.artemis.core.server.cluster.ClusterManager.deploy(ClusterManager.java:245)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.initialisePart1(ActiveMQServerImpl.java:3066)
>       at 
> org.apache.activemq.artemis.core.server.impl.SharedNothingLiveActivation.run(SharedNothingLiveActivation.java:114)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.internalStart(ActiveMQServerImpl.java:634)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.start(ActiveMQServerImpl.java:558)
>       at 
> org.apache.activemq.artemis.integration.FileBroker.start(FileBroker.java:64)
>       at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:115)
>       at 
> org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:154)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:102)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:129)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-1)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:134)
>       at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:50)
> {code}
> On both cases, the same broker transport isn't ignored because the discovery 
> has been started with a randomly generated Node ID on the server locator, 
> instead of using the broker real Node ID.
> The subsequent cluster manager start, instead, set the broker Node ID on the 
> sever locator while its cluster connection is activated and that's why it 
> ignores its same connector.
> The relevant stack trace is:
> {code:java}
>       at 
> org.apache.activemq.artemis.core.cluster.DiscoveryGroup.<init>(DiscoveryGroup.java:96)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.createDiscoveryGroup(ServerLocatorImpl.java:298)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.startDiscovery(ServerLocatorImpl.java:275)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.initialize(ServerLocatorImpl.java:263)
>       at 
> org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.start(ServerLocatorImpl.java:477)
>       at 
> org.apache.activemq.artemis.core.server.cluster.impl.ClusterConnectionImpl.activate(ClusterConnectionImpl.java:680)
>       at 
> org.apache.activemq.artemis.core.server.cluster.impl.ClusterConnectionImpl.start(ClusterConnectionImpl.java:399)
>       at 
> org.apache.activemq.artemis.core.server.cluster.ClusterManager.start(ClusterManager.java:270)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.initialisePart2(ActiveMQServerImpl.java:3225)
>       at 
> org.apache.activemq.artemis.core.server.impl.SharedNothingLiveActivation.run(SharedNothingLiveActivation.java:118)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.internalStart(ActiveMQServerImpl.java:634)
>       at 
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.start(ActiveMQServerImpl.java:558)
>       at 
> org.apache.activemq.artemis.integration.FileBroker.start(FileBroker.java:64)
>       at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:115)
>       at 
> org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:154)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:102)
>       at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:129)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-1)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:134)
>       at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:50)
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to