From 00d1c46d60cff0abd504b215f876181da7514298 Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Thu, 30 Jul 2020 23:19:43 +0000 Subject: [PATCH 1/3] Ensure Replay Input Stream and File Channels are closed after writing --- .../modules/writer/WARCWriterChainProcessor.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java b/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java index 4fc45558..b25d9ed7 100644 --- a/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java +++ b/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java @@ -1,5 +1,6 @@ package org.archive.modules.writer; +import java.io.InputStream; import java.io.IOException; import java.net.URI; import java.util.Arrays; @@ -7,6 +8,7 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import org.apache.commons.io.IOUtils; import org.archive.io.warc.WARCRecordInfo; import org.archive.io.warc.WARCWriter; import org.archive.modules.CrawlURI; @@ -159,6 +161,17 @@ public class WARCWriterChainProcessor extends BaseWARCWriterProcessor implements WARCRecordInfo record = recordBuilder.buildRecord(curi, concurrentTo); if (record != null) { writer.writeRecord(record); + InputStream is = null; + try { + is = record.getContentStream(); + is.close(); + } + catch (Exception e){ + logger.log(Level.WARNING, "problem closing youtube-dl temp file " + e); + } + finally { + IOUtils.closeQuietly(record.getContentStream()); //but for real, close this time + } if (concurrentTo == null) { concurrentTo = record.getRecordId(); } From 52a8f345ed2686b0ac7de7acebaf4e2871a84b35 Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Thu, 30 Jul 2020 23:30:08 +0000 Subject: [PATCH 2/3] Fixing up logging and comments --- .../org/archive/modules/writer/WARCWriterChainProcessor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java b/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java index b25d9ed7..bab6dbc3 100644 --- a/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java +++ b/modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java @@ -167,10 +167,10 @@ public class WARCWriterChainProcessor extends BaseWARCWriterProcessor implements is.close(); } catch (Exception e){ - logger.log(Level.WARNING, "problem closing youtube-dl temp file " + e); + logger.log(Level.WARNING, "problem closing Warc Record Content Stream " + e); } finally { - IOUtils.closeQuietly(record.getContentStream()); //but for real, close this time + IOUtils.closeQuietly(record.getContentStream()); //Closing one way or the other seems to leave some file handles open. Calling close() and using closeQuietly() handles both FileStreams and FileChannels } if (concurrentTo == null) { concurrentTo = record.getRecordId(); From a9c0c6588b0b7cc456996b47f7b72bb75fe2e2fd Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Fri, 31 Jul 2020 22:22:25 +0000 Subject: [PATCH 3/3] Manage youtube dl temp files which can be closed in the warc writer. --- .../modules/extractor/ExtractorYoutubeDL.java | 74 +++++++++++++++---- 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/contrib/src/main/java/org/archive/modules/extractor/ExtractorYoutubeDL.java b/contrib/src/main/java/org/archive/modules/extractor/ExtractorYoutubeDL.java index 661e0669..b824146d 100644 --- a/contrib/src/main/java/org/archive/modules/extractor/ExtractorYoutubeDL.java +++ b/contrib/src/main/java/org/archive/modules/extractor/ExtractorYoutubeDL.java @@ -117,17 +117,52 @@ public class ExtractorYoutubeDL extends Extractor // unnamed toethread-local temporary file protected transient ThreadLocal tempfile = new ThreadLocal() { protected RandomAccessFile initialValue() { - File t; - try { - t = File.createTempFile("ydl", ".json"); - RandomAccessFile f = new RandomAccessFile(t, "rw"); - t.delete(); - return f; - } catch (IOException e) { - throw new RuntimeException(e); - } + return null; } }; + protected void closeLocalTempFile() { + RandomAccessFile localTemp = tempfile.get(); + if(localTemp == null || !isOpen(localTemp)) + return; // avoid making a new temp file just to close it immediately + try { + getLocalTempFile().close(); + tempfile.set(null); + } + catch (Exception e) { + logger.log(Level.WARNING, "problem closing ydl temp file " + e); + } + } + protected RandomAccessFile getLocalTempFile() { + RandomAccessFile localTemp = tempfile.get(); + if(localTemp == null || !isOpen(localTemp)) { + localTemp = openNewTempFile(); + tempfile.set(localTemp); + } + logger.info("Getting youtube-dl temp file "); + return localTemp; + } + protected boolean isOpen(RandomAccessFile f) { + try { + f.length(); + return true; + } + catch (IOException e) { + logger.info("youtube-dl temp file is not open"); + return false ; + } + } + protected RandomAccessFile openNewTempFile() { + logger.info("Opening New youtube-dl temp file "); + File t; + try { + t = File.createTempFile("ydl", ".json"); + RandomAccessFile f = new RandomAccessFile(t, "rw"); + t.delete(); + return f; + } catch (IOException e) { + throw new RuntimeException(e); + } + } protected CrawlerLoggerModule crawlerLoggerModule; public CrawlerLoggerModule getCrawlerLoggerModule() { @@ -446,7 +481,7 @@ public class ExtractorYoutubeDL extends Extractor } }); - YoutubeDLResults results = new YoutubeDLResults(tempfile.get()); + YoutubeDLResults results = new YoutubeDLResults(getLocalTempFile()); try { try { @@ -524,7 +559,14 @@ public class ExtractorYoutubeDL extends Extractor // should build record for containing page, which has an // annotation like "youtube-dl:3" (no slash) String annotation = findYdlAnnotation(uri); - return annotation != null && !annotation.contains("/"); + boolean shouldBuild = (annotation != null && !annotation.contains("/")); + + // If we processed this uri, then we have an open temp file that won't get closed + // for us by the warc writer + if(!shouldBuild) + closeLocalTempFile(); + + return shouldBuild; } @Override @@ -545,10 +587,10 @@ public class ExtractorYoutubeDL extends Extractor recordInfo.setMimetype("application/vnd.youtube-dl_formats+json;charset=utf-8"); recordInfo.setEnforceLength(true); - tempfile.get().seek(0); - InputStream inputStream = Channels.newInputStream(tempfile.get().getChannel()); + getLocalTempFile().seek(0); + InputStream inputStream = Channels.newInputStream(getLocalTempFile().getChannel()); recordInfo.setContentStream(inputStream); - recordInfo.setContentLength(tempfile.get().length()); + recordInfo.setContentLength(getLocalTempFile().length()); logger.info("built record timestamp=" + timestamp + " url=" + recordInfo.getUrl()); @@ -574,7 +616,7 @@ public class ExtractorYoutubeDL extends Extractor ExtractorYoutubeDL e = new ExtractorYoutubeDL(); FileInputStream in = new FileInputStream("/tmp/ydl-single-video.json"); - YoutubeDLResults results = new YoutubeDLResults(e.tempfile.get()); + YoutubeDLResults results = new YoutubeDLResults(e.getLocalTempFile()); e.streamYdlOutput(in, results); System.out.println("video urls: " + results.videoUrls); System.out.println("page urls: " + results.pageUrls); @@ -590,7 +632,7 @@ public class ExtractorYoutubeDL extends Extractor } in = new FileInputStream("/tmp/ydl-uncgreensboro-limited.json"); - results = new YoutubeDLResults(e.tempfile.get()); + results = new YoutubeDLResults(e.getLocalTempFile()); e.streamYdlOutput(in, results); System.out.println("video urls: " + results.videoUrls); System.out.println("page urls: " + results.pageUrls);