-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clear tmp dir #344
Clear tmp dir #344
Changes from all commits
62e9766
1ee0ccf
9ea8783
eca1c9a
9b0fc86
8421be8
431b62d
5469a45
4f070ca
b88eb0b
673d8c1
423a428
8c38030
d70e6a0
96223fa
d12c5fd
d8b3f2a
51c6415
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,105 @@ | ||||||
package org.commonwl.view.util; | ||||||
|
||||||
|
||||||
import java.io.File; | ||||||
import java.nio.file.Files; | ||||||
import java.nio.file.attribute.FileTime; | ||||||
import java.io.IOException; | ||||||
import java.time.Instant; | ||||||
import java.time.Clock; | ||||||
import java.time.Duration; | ||||||
|
||||||
import org.slf4j.Logger; | ||||||
import org.slf4j.LoggerFactory; | ||||||
import org.commonwl.view.util.StreamGobbler; | ||||||
|
||||||
public class FileUtils { | ||||||
|
||||||
private final Logger logger; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
public FileUtils(Logger logger) { | ||||||
this.logger = logger; | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessary? ☝️ Normally utility classes have private constructors and static public methods. But since this is not using static methods, I think we should delete this constructor, and create a logger here too, as it's done in other files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I struggled a bit with this decision because
but looking at your comment. I guess I could have different logs at the scheduled function level and also the file utils level. That way the util logs would be nested and surrounded by the scheduled functions logs |
||||||
|
||||||
|
||||||
public void clearDirectory(String directoryPath, long days) { | ||||||
try { | ||||||
logger.info("Clearing "+ directoryPath + " directory for content older than " + days + " day" + (days > 1 ? "s" : "") + "..."); | ||||||
File file = new File(directoryPath); | ||||||
deleteWithinDirectory(file, days); | ||||||
logger.info("Successfully cleared " + directoryPath + " directory"); | ||||||
} catch(IOException e) { | ||||||
logger.warn("Could not clear " + directoryPath + " directory"); | ||||||
logger.error("Error running clear " + directoryPath + " dir process", e); | ||||||
} | ||||||
|
||||||
} | ||||||
|
||||||
|
||||||
private void deleteWithinDirectory(File file, long days) throws IOException{ | ||||||
File[] files = file.listFiles(); | ||||||
|
||||||
if (files != null) { | ||||||
for (File subfile : files) { | ||||||
if (subfile.isDirectory()) { | ||||||
deleteWithinDirectory(subfile, days); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong indentation. |
||||||
} | ||||||
|
||||||
long daysOld = fileAge(subfile); | ||||||
|
||||||
if (daysOld >= days ) { | ||||||
if (!subfile.isDirectory()) { | ||||||
logger.info("Deleting file " + subfile.getPath()); | ||||||
subfile.delete(); | ||||||
logger.info("Deleted file " + subfile.getPath()); | ||||||
} else { | ||||||
File[] contents = subfile.listFiles(); | ||||||
if (contents != null && contents.length == 0) { | ||||||
logger.info("Deleting directory " + subfile.getPath()); | ||||||
subfile.delete(); | ||||||
logger.info("Deleted directory " + subfile.getPath()); | ||||||
} | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
private long fileAge(File file) throws IOException { | ||||||
|
||||||
FileTime t = Files.getLastModifiedTime(file.toPath()); | ||||||
Instant fileInstant = t.toInstant(); | ||||||
Instant now = (Clock.systemUTC()).instant(); | ||||||
Duration difference = Duration.between(fileInstant, now); | ||||||
long days = difference.toDays(); | ||||||
return days; | ||||||
|
||||||
} | ||||||
|
||||||
private void deleteWithinDirectoryCMD(String directoryPath, int days) throws IOException, InterruptedException { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I searched this file in GitHub UI for where it was used... but apparently it's not used anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! Got it! We are now using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, will do that. Left them both for the review. |
||||||
|
||||||
String[] command = {"find", directoryPath, "-ctime", "+" + days, "-exec", "rm", "-rf", "{}", "+"}; | ||||||
ProcessBuilder clearProcess = new ProcessBuilder(command); | ||||||
Process process = clearProcess.start(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code was in
|
||||||
|
||||||
// Read output from the process using threads | ||||||
StreamGobbler inputGobbler = new StreamGobbler(process.getInputStream()); | ||||||
StreamGobbler errorGobbler = new StreamGobbler(process.getErrorStream()); | ||||||
errorGobbler.start(); | ||||||
inputGobbler.start(); | ||||||
|
||||||
// Wait for process to complete | ||||||
int exitCode = process.waitFor(); | ||||||
if (exitCode == 0) { | ||||||
inputGobbler.join(); | ||||||
logger.info(inputGobbler.getContent()); | ||||||
logger.info("Successfully cleared " + directoryPath + " directory"); | ||||||
} else { | ||||||
errorGobbler.join(); | ||||||
logger.warn("Could not clear " + directoryPath + " directory"); | ||||||
logger.warn(errorGobbler.getContent()); | ||||||
|
||||||
} | ||||||
} | ||||||
|
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ spring.data.mongodb.port = 27017 | |
sparql.endpoint = http://localhost:3030/cwlviewer/ | ||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit-picking, but unnecessary blank line 😬 |
||
#======================= | ||
# Scheduler settings | ||
#======================= | ||
|
@@ -57,5 +58,12 @@ sparql.endpoint = http://localhost:3030/cwlviewer/ | |
# The expression below implies every hour at the 0th second and 0th minute i.e (01:00:00, 02:00::00,... etc) | ||
cron.deleteOldQueuedWorkflows = 0 0 * * * ? | ||
|
||
# The expression below implies every day at the 0th second, 0th minute and 24th(0th) hour i.e ( time 00:00:00, every day) | ||
cron.clearTmpDir = 0 0 0 * * ? | ||
|
||
# Age limit for queued workflows in hours. | ||
queuedWorkflowAgeLimitHours = 24 | ||
queuedWorkflowAgeLimitHours = 24 | ||
|
||
# Age limit for tmp directories in days. | ||
tmpDirAgeLimitDays = 1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I run
cwlviewer
in my laptop, directly from IntelliJ, executing the SpringBoot application. If I understood correctly, here if I were to leave the process running until midnight, then it would go over my/tmp
directory removing any files older than the age limit.I have other applications running that also use
/tmp
to store temporary files. That would probably cause issues for me. So in this case, I would probably store temporary data forcwlviewer
elsewhere, and expect theScheduler
to be able to cope with that too.So I think we should:
cwlviewer
is using/tmp
(not sure if anyone uses Windows withcwlviewer
, but someone could be using a different TMPDIR in linux too, normally in Java we rely onjava.io.tmpdir
)/tmp
, or something that resolves/tmp
, then we need to make sure it doesn't affect other processes running too, so we either