tabish121 commented on code in PR #5854:
URL: https://github.com/apache/activemq-artemis/pull/5854#discussion_r2254513482


##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java:
##########
@@ -571,11 +610,110 @@ public void loadProtocolServices(List<ActiveMQComponent> 
protocolServices) {
 
    @Override
    public void updateProtocolServices(List<ActiveMQComponent> 
protocolServices) throws Exception {
+      updateAcceptors();
+
       for (ProtocolManagerFactory protocolManagerFactory : 
protocolMap.values()) {
          protocolManagerFactory.updateProtocolServices(this.server, 
protocolServices);
       }
    }
 
+   private void updateAcceptors() throws Exception {
+      final Set<TransportConfiguration> updatedConfigurationSet = 
Objects.requireNonNullElse(server.getConfiguration().getAcceptorConfigurations(),
 Collections.emptySet());
+      final Map<String, TransportConfiguration> updatedConfiguration =
+         updatedConfigurationSet.stream()
+                                .collect(Collectors.toMap(c -> c.getName(), 
Function.identity()));
+
+      final Set<TransportConfiguration> acceptorsToStop = new HashSet<>();
+      final Set<TransportConfiguration> acceptorsToCreate = new HashSet<>();
+
+      for (TransportConfiguration candidateConfiguration : 
updatedConfiguration.values()) {
+         final TransportConfiguration previous = 
acceptorsConfig.get(candidateConfiguration.getName());
+
+         if (previous == null) {
+            // New configuration that was added during the update
+            acceptorsToCreate.add(candidateConfiguration);
+         } else if (!previous.equals(candidateConfiguration)) {
+            // Updated configuration that needs to be stopped and restarted.
+            acceptorsToCreate.add(candidateConfiguration);
+            acceptorsToStop.add(candidateConfiguration);
+         }
+      }
+
+      for (TransportConfiguration currentConfiguration : 
acceptorsConfig.values()) {
+         if 
(!updatedConfiguration.containsKey(currentConfiguration.getName())) {
+            // Acceptor that was removed from the configuration which needs 
stopped and removed.
+            acceptorsToStop.add(currentConfiguration);
+         }
+      }
+
+      // Replace old configuration map with new configurations ahead of the 
stop and restart phase.
+      acceptorsConfig.clear();
+      acceptorsConfig.putAll(updatedConfiguration);
+
+      final CountDownLatch acceptorsStoppedLatch = new 
CountDownLatch(acceptorsToStop.size());
+
+      for (TransportConfiguration acceptorToStop : acceptorsToStop) {
+         final Acceptor acceptor = acceptors.remove(acceptorToStop.getName());
+
+         if (acceptor == null) {
+            continue;
+         }
+
+         final Map<String, Object> acceptorToStopParams = 
acceptorToStop.getCombinedParams();
+
+         removeAcceptorStoreReloadCallback(acceptorToStop.getName(),
+            fileUrlFrom(acceptorToStopParams.get(KEYSTORE_PATH_PROP_NAME)),
+            storeTypeFrom(acceptorToStopParams.get(KEYSTORE_TYPE_PROP_NAME)));
+
+         removeAcceptorStoreReloadCallback(acceptorToStop.getName(),
+            fileUrlFrom(acceptorToStopParams.get(TRUSTSTORE_PATH_PROP_NAME)),
+            
storeTypeFrom(acceptorToStopParams.get(TRUSTSTORE_TYPE_PROP_NAME)));
+
+         try {
+            managementService.unregisterAcceptor(acceptor.getName());
+         } catch (Throwable t) {
+            
ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(acceptor.getName());
+         }
+
+         try {
+            acceptor.notifyStop();
+         } catch (Throwable t) {
+            
ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(acceptor.getName());
+         }
+
+         try {
+            acceptor.pause();
+         } catch (Throwable t) {
+            
ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(acceptor.getName());
+         }
+
+         try {
+            acceptor.asyncStop(acceptorsStoppedLatch::countDown);
+         } catch (Throwable t) {
+            
ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(acceptor.getName());
+         }
+      }
+
+      // In some cases an acceptor stopping could be locked ie NettyAcceptor 
stopping could be locked by a network failure.
+      if (!acceptorsStoppedLatch.await(UPDATE_ACCEPTORS_STOP_TIMEOUT, 
TimeUnit.MILLISECONDS)) {
+         logger.warn("Timed out waiting on removed or updated acceptors 
stopping.");
+      }
+
+      // Add all the new or updated acceptors now that removed or updated 
acceptors have been stopped.
+      for (TransportConfiguration candidateConfiguration : acceptorsToCreate) {
+         final Acceptor acceptor = createAcceptor(candidateConfiguration);
+
+         if (isStarted() && acceptor instanceof NettyAcceptor startable && 
startable.isAutoStart()) {
+            try {
+               startable.start();
+            } catch (Throwable t) {
+               
ActiveMQServerLogger.LOGGER.errorStartingAcceptor(acceptor.getName(), 
acceptor.getConfiguration());
+               throw t;
+            }
+         }
+      }

Review Comment:
   Since the update is likely to be small and there would still be existing 
acceptors that are started likely it seems you still just end up with some 
active and some not, so it feels like rearranging deck chairs on the 
Hindenburg. You likely need to stop the broker at that point if you added or 
updated acceptors with invalid configuration. 



-- 
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-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to