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]
