You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/11/18 12:29:40 UTC
[GitHub] [knox] moresandeep commented on a change in pull request #518: KNOX-2689 - Redeploying a topology only if it was actually changed
moresandeep commented on a change in pull request #518:
URL: https://github.com/apache/knox/pull/518#discussion_r752184668
##########
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:
Isn't this an expensive operation, reading the entire file? what I am thinking is, relying on file timestamp was quick and dirty solution. Reading and parsing xml file is a bit error prone.
##########
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);
+ String updated = currentTopologyContent.replaceAll("^*<redeployTime>.*", "");
+
+ //add the current timestamp
+ updated = updated.replace(TOPOLOGY_CLOSING_XML_ELEMENT, String.format(Locale.getDefault(), REDEPLOY_TIME_TEMPLATE, System.currentTimeMillis()));
+
+ //save the updated content in the file
+ FileUtils.write(topologyFile, updated, StandardCharsets.UTF_8);
+ } catch (IOException e) {
Review comment:
We should be closing the File object in finally.
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org