smolnar82 commented on a change in pull request #324:
URL: https://github.com/apache/knox/pull/324#discussion_r413199270



##########
File path: 
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzer.java
##########
@@ -162,93 +162,98 @@ public void run() {
     isActive = true;
 
     while (isActive) {
-      List<String> clustersToStopMonitoring = new ArrayList<>();
+      try {
+        final List<String> clustersToStopMonitoring = new ArrayList<>();
+
+        for (Map.Entry<String, List<String>> entry : 
configCache.getClusterNames().entrySet()) {
+          String address = entry.getKey();
+          for (String clusterName : entry.getValue()) {
+            log.checkingClusterConfiguration(clusterName, address);
+
+            // Check here for existing descriptor references, and add to the 
removal list if there are not any
+            if (!clusterReferencesExist(address, clusterName)) {
+              clustersToStopMonitoring.add(address + FQCN_DELIM + clusterName);
+              continue;
+            }
 
-      for (Map.Entry<String, List<String>> entry : 
configCache.getClusterNames().entrySet()) {
-        String address = entry.getKey();
-        for (String clusterName : entry.getValue()) {
-          log.checkingClusterConfiguration(clusterName, address);
+            // Configuration changes don't mean anything without corresponding 
service start/restarts. Therefore, monitor
+            // start events, and check the configuration only of the restarted 
service(s) to identify changes
+            // that should trigger re-discovery.
+            final List<StartEvent> relevantEvents = getRelevantEvents(address, 
clusterName);
 
-          // Check here for existing descriptor references, and add to the 
removal list if there are not any
-          if (!clusterReferencesExist(address, clusterName)) {
-            clustersToStopMonitoring.add(address + FQCN_DELIM + clusterName);
-            continue;
+            // If there are no recent start events, then nothing to do now
+            if (!relevantEvents.isEmpty()) {
+              // If a change has occurred, notify the listeners
+              if (hasConfigChanged(address, clusterName, relevantEvents)) {
+                notifyChangeListener(address, clusterName);
+              }
+            }
           }
+        }
 
-          // Configuration changes don't mean anything without corresponding 
service start/restarts. Therefore, monitor
-          // start events, and check the configuration only of the restarted 
service(s) to identify changes
-          // that should trigger re-discovery.
-          List<StartEvent> relevantEvents = getRelevantEvents(address, 
clusterName);
-
-          // If there are no recent start events, then nothing to do now
-          if (!relevantEvents.isEmpty()) {
-            boolean configHasChanged = false;
-
-            // If there are start events, then check the previously-recorded 
properties for the same service to
-            // identify if the configuration has changed
-            Map<String, ServiceConfigurationModel> serviceConfigurations =
-                                    
configCache.getClusterServiceConfigurations(address, clusterName);
-
-            // Those services for which a start even has been handled
-            List<String> handledServiceTypes = new ArrayList<>();
-
-            for (StartEvent re : relevantEvents) {
-              String serviceType = re.getServiceType();
-
-              // Determine if we've already handled a start event for this 
service type
-              if (!handledServiceTypes.contains(serviceType)) {
-
-                // Get the previously-recorded configuration
-                ServiceConfigurationModel serviceConfig = 
serviceConfigurations.get(re.getServiceType());
-
-                if (serviceConfig != null) {
-                  // Get the current config for the started service, and 
compare with the previously-recorded config
-                  ServiceConfigurationModel currentConfig =
-                                  getCurrentServiceConfiguration(address, 
clusterName, re.getService());
-
-                  if (currentConfig != null) {
-                    log.analyzingCurrentServiceConfiguration(re.getService());
-                    try {
-                      configHasChanged = 
hasConfigurationChanged(serviceConfig, currentConfig);
-                    } catch (Exception e) {
-                      
log.errorAnalyzingCurrentServiceConfiguration(re.getService(), e);
-                    }
-                  }
-                } else {
-                  // A new service (no prior config) represent a config 
change, since a descriptor may have referenced
-                  // the "new" service, but discovery had previously not 
succeeded because the service had not been
-                  // configured (appropriately) at that time.
-                  log.serviceEnabled(re.getService());
-                  configHasChanged = true;
-                }
-
-                handledServiceTypes.add(serviceType);
-              }
+        // Remove outdated entries from the cache
+        for (String fqcn : clustersToStopMonitoring) {
+          String[] parts = fqcn.split(FQCN_DELIM);
+          stopMonitoring(parts[0], parts[1]);
+        }
+        clustersToStopMonitoring.clear(); // reset the removal list
 
-              if (configHasChanged) {
-                break; // No need to continue checking once we've identified 
one reason to perform discovery again
-              }
-            }
+        waitFor(interval);
+      } catch (Throwable t) {

Review comment:
       +1




----------------------------------------------------------------
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]


Reply via email to