zeroflag commented on a change in pull request #518:
URL: https://github.com/apache/knox/pull/518#discussion_r750131887



##########
File path: 
gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
##########
@@ -179,32 +182,21 @@ private void redeployTopology(Topology topology) {
         }
       }
 
-      long start = System.currentTimeMillis();
-      long limit = 1000L; // One second.
-      long elapsed = 1;
-      while (elapsed <= limit) {
-        try {
-          long origTimestamp = topologyFile.lastModified();
-          long setTimestamp = Math.max(System.currentTimeMillis(), 
topologyFile.lastModified() + elapsed);
-          if(topologyFile.setLastModified(setTimestamp)) {
-            long newTimstamp = topologyFile.lastModified();
-            if(newTimstamp > origTimestamp) {
-              break;
-            } else {
-              Thread.sleep(10);
-              elapsed = System.currentTimeMillis() - start;
-            }
-          } else {
-            auditor.audit(Action.REDEPLOY, topology.getName(), 
ResourceType.TOPOLOGY,
-                ActionOutcome.FAILURE);
-            log.failedToRedeployTopology(topology.getName());
-            break;
-          }
-        } catch (InterruptedException e) {
-          auditor.audit(Action.REDEPLOY, topology.getName(), 
ResourceType.TOPOLOGY, ActionOutcome.FAILURE);
-          log.failedToRedeployTopology(topology.getName(), e);
-          Thread.currentThread().interrupt();
-        }
+      // Since KNOX-2689, updating the topology file's timestamp is not enough.
+      // We need to make an actual change in the topology XML to redeploy it
+      // This change is: updating a new XML element called redeployTime
+      try {
+        final String currentTopologyContent = 
FileUtils.readFileToString(topologyFile, StandardCharsets.UTF_8);

Review comment:
       > Of course, we could implement a framework that listens/receives such 
events
   
   What's the difference between modifying the file and calling:
   
   ```java
   notifyChangeListeners(Arrays.asList(new 
TopologyEvent(TopologyEvent.Type.UPDATED, topology)));
   ```
   
   All the infrastructure that is required to trigger a redeploy seems to be 
already available.
   
   edit: we discussed this offline, the problem with this is that knox cli also 
uses this method which runs from a different process




-- 
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: [email protected]

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


Reply via email to