[
https://issues.apache.org/jira/browse/KNOX-2689?focusedWorklogId=683263&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-683263
]
ASF GitHub Bot logged work on KNOX-2689:
----------------------------------------
Author: ASF GitHub Bot
Created on: 18/Nov/21 12:29
Start Date: 18/Nov/21 12:29
Worklog Time Spent: 10m
Work Description: 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 683263)
Time Spent: 1h 10m (was: 1h)
> 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: 1h 10m
> 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)
