Files in "tmp" should not be too large resp. removed

[dr0i]

There is one really big file in tmp:

~/git/metafacture-playground/tmp$ l -Shal |head
total 39G
-rw-r--r--  1 sol sol  39G Aug 22 09:55 metafix1405254507378886918.txt

We should somehow restrict log files to be too big.

[katauber]

This is no log file. It's result file of a workflow. But I don't understand why this isn't deleted after returning the result to the client. I will have a look what is going on there.

[katauber]

functional review: @dr0i
code review: @fsteeg

[katauber]

The files are not deleted at the moment.

@dr0i and I discussed offline, that we first need a mechanism to delete these files after a successful processed workflow and a configurable limit how big a file may be.

[Phu2]

The size of these result files depends on the size of the input, right?
I think we need to set a limit on the input files. too.

[blackwinter]

Regarding the input file not being deleted: Flux.main() opens a FileInputStream via ResourceUtil.getStream() that isn't closed afterwards. Could this be it?

Possible fix:

diff --git metafacture-runner/src/main/java/org/metafacture/runner/Flux.java metafacture-runner/src/main/java/org/metafacture/runner/Flux.java
index 9d889b45..5acdab6f 100644
--- metafacture-runner/src/main/java/org/metafacture/runner/Flux.java
+++ metafacture-runner/src/main/java/org/metafacture/runner/Flux.java
@@ -25,6 +25,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -82,7 +83,9 @@ public static void main(final String[] args) throws IOException, RecognitionExce
             }
 
             // run parser and builder
-            FluxCompiler.compile(ResourceUtil.getStream(fluxFile), vars).start();
+            try (InputStream inputStream = ResourceUtil.getStream(fluxFile)) {
+                FluxCompiler.compile(inputStream, vars).start();
+            }
         }
     }

[fsteeg]

Hm, but that stream is for the Flux file, not the input file, right?

Still looks like a good change fixing a resource leak.

[blackwinter]

Hm, but that stream is for the Flux file, not the input file, right?

D'oh :) Sorry, you're absolutely right!

[blackwinter]

Then it's the FileOpener?

diff --git metafacture-io/src/main/java/org/metafacture/io/FileOpener.java metafacture-io/src/main/java/org/metafacture/io/FileOpener.java
index b88b9aab..a2a8eeac 100644
--- metafacture-io/src/main/java/org/metafacture/io/FileOpener.java
+++ metafacture-io/src/main/java/org/metafacture/io/FileOpener.java
@@ -154,8 +154,8 @@ public Reader open(final InputStream stream) throws IOException {
 
     @Override
     public void process(final String file) {
-        try {
-            getReceiver().process(open(file));
+        try (Reader reader = open(file)) {
+            getReceiver().process(reader);
         }
         catch (final IOException e) {
             throw new MetafactureException(e);

(Breaks some tests, though, since they try to read from the reader a second time in order to verify its contents.)

[fsteeg]

Oh, yes, that looks good!

(Seems the FileInputStream is only closed if something goes wrong.)

[katauber]

I move this ticket back to backlog and it becomes ready, when the problem with the FileInputStream is fixed in Fix

[blackwinter]

But can you confirm that this is indeed the issue here? We've identified two unclosed resources, one of which still allows deleting the file (FileInputStream on Flux file) and the other one - apparently - doesn't (Reader on input file).

[katauber]

I think I can confirm it (or not), when these unclosed resources are closed. I think the probability is very high that one of these two is the reason that I can't delete the input file, isn't it?

[blackwinter]

Well, let me put it this way: If you could confirm beforehand that properly closing the Reader on the input file would indeed allow it to be deleted, then we could justify investing the effort into fixing the test cases for this change.

So would it be possible for you to apply the fix locally and observe its effect in the Playground?

[katauber]

I'll try 👍

[katauber]

It works with the fix in Fix :)

[blackwinter]

Great, thank you for confirming! Then we'll have to see how we can fix it (in -core, not -fix) while still preserving testability.

[blackwinter]

I've opened metafacture/metafacture-core#514 to address the unclosed resources.

[blackwinter]

Resources are now closed in metafacture-core; should be released with version 6.0 (see metafacture/metafacture-core#515).

[katauber]

I would like to split this issue in two:

1. Remove every tmp file after a succesful answer to the client Remove tmp files #158
2. Restrict the size of tmp files in general Restrict file size of tmp files #159

[dr0i]

Resources are now closed in metafacture-core; should be released with version 6.0 (see metafacture/metafacture-core#515).

metafacture-core 6.0.0 is released as is metafacture-playground 1.0.0 (which is based on the former). Also the latter is deployed in it's newest version.

Subissues are opend, so this issue can be closed..