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

ASF GitHub Bot logged work on KNOX-2689:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 16/Nov/21 09:56
            Start Date: 16/Nov/21 09:56
    Worklog Time Spent: 10m 
      Work Description: smolnar82 commented on a change in pull request #518:
URL: https://github.com/apache/knox/pull/518#discussion_r750099023



##########
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:
       Thanks, @zeroflag for your review comments. Let me try to reply to them 
one by one:
   > What if there is no <redeployTime> tag in the file? Is this always 
supposed to be there?
   
   It's not a problem. In that case, the default of `-1` is used.
   
   > I wonder if it's possible to use some internal notification mechanism to 
send some event to the watcher to redeploy the topology without needing to 
modify the file.
   
   Unfortunately, currently, modifying the topology file is the _only_ option. 
Of course, we _could_ implement a framework that listens/receives such events, 
but it'd very fragile and - IMO - way too heavy for this operation.
   
   > I also wonder if it would make sense to make redeployTime as an attribute 
of the root tag for example, making it less apparent.
   
   That's a good idea and I also considered that option but I found it more 
convenient to implement it as a separate XML tag (following the current XML 
layout we have in topologies). If you, or the rest of the team, convinces me 
it's a must before we merge this change I'll update my changes.
   




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


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

    Worklog Id:     (was: 681899)
    Time Spent: 40m  (was: 0.5h)

> Avoid redeploying an unchanged topology
> ---------------------------------------
>
>                 Key: KNOX-2689
>                 URL: https://issues.apache.org/jira/browse/KNOX-2689
>             Project: Apache Knox
>          Issue Type: Improvement
>    Affects Versions: 1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.4.0, 1.5.0, 1.6.0
>            Reporter: Sandor Molnar
>            Assignee: Sandor Molnar
>            Priority: Major
>             Fix For: 2.0.0
>
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> Currently, if a topology is built from a provider/descriptor pair, Knox 
> checks the content of the "new" topology upon "onFileChange" (\{{touch 
> descriptor}}) and does not generate the topology if it's the same as the 
> currently deployed topology.
> This check is not implemented on the old-style XML-based topology handling 
> path.
> Note: as a side effect, touching a topology file will no longer trigger the 
> topology redeployment.
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to