commit 56840df1956bc3ac4521d6fddac4d9abddf926ee Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Tue Nov 1 15:38:00 2016 +0100 Add methods for loading and saving a history file. The history file implementation in `DescriptorReader` writes the history file passed in `setExcludeFiles()` immediately after reading and parsing the last descriptor and putting it into the queue, regardless of whether the application has finished processing those descriptors. If the application fails after the history file is written, it may not be able to process descriptors in the next execution that have still been in the queue at the time of failing. This commit deprecates the `setExcludeFiles()` method and replaces it by a `setHistoryFile()` and a `saveHistoryFile()` method. Applications would use `setHistoryFile()` before starting to read descriptors, process all descriptors, perform any cleaning up, and then call `saveHistoryFile()`. Implements #20521. --- CHANGELOG.md | 8 +++ .../torproject/descriptor/DescriptorReader.java | 29 +++++++++++ .../descriptor/impl/DescriptorReaderImpl.java | 60 ++++++++++++++++------ 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a62c987..2a90b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# Changes in version 1.6.0 - 2016-??-?? + + * Medium changes + - Add two methods for loading and saving a parse history file in + the descriptor reader to avoid situations where applications fail + after all descriptors are read but before they are all processed. + + # Changes in version 1.5.0 - 2016-10-19 * Major changes diff --git a/src/main/java/org/torproject/descriptor/DescriptorReader.java b/src/main/java/org/torproject/descriptor/DescriptorReader.java index 771755e..076a7f0 100644 --- a/src/main/java/org/torproject/descriptor/DescriptorReader.java +++ b/src/main/java/org/torproject/descriptor/DescriptorReader.java @@ -68,11 +68,40 @@ public interface DescriptorReader { * <p>Lines in the history file contain the last modified time in * milliseconds since the epoch and the absolute path of a file.</p> * + * @deprecated Replaced by {@link #setHistoryFile()} and + * {@link #saveHistoryFile()} which let the application explicitly tell us + * when it's done processing read descriptors. + * * @since 1.0.0 */ public void setExcludeFiles(File historyFile); /** + * Set a history file to load before reading descriptors and exclude + * descriptor files that haven't changed since they have last been read. + * + * <p>Lines in the history file contain the last modified time in + * milliseconds since the epoch and the absolute path of a file, separated by + * a space.</p> + * + * @since 1.6.0 + */ + public void setHistoryFile(File historyFile); + + /** + * Save a history file with file names and last modified timestamps of + * descriptor files that exist in the input directory or directories and that + * have either been parsed or excluded from parsing. + * + * <p>Lines in the history file contain the last modified time in + * milliseconds since the epoch and the absolute path of a file, separated by + * a space.</p> + * + * @since 1.6.0 + */ + public void saveHistoryFile(File historyFile); + + /** * Exclude files if they haven't changed since the corresponding last * modified timestamps. * diff --git a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java index 020cdd7..9adc446 100644 --- a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java @@ -62,7 +62,9 @@ public class DescriptorReaderImpl implements DescriptorReader { this.tarballs.add(tarball); } - private File historyFile; + private File autoSaveHistoryFile; + + private File manualSaveHistoryFile; @Override public void setExcludeFiles(File historyFile) { @@ -70,7 +72,16 @@ public class DescriptorReaderImpl implements DescriptorReader { throw new IllegalStateException("Reconfiguration is not permitted " + "after starting to read."); } - this.historyFile = historyFile; + this.autoSaveHistoryFile = historyFile; + } + + @Override + public void setHistoryFile(File historyFile) { + if (this.hasStartedReading) { + throw new IllegalStateException("Reconfiguration is not permitted " + + "after starting to read."); + } + this.manualSaveHistoryFile = historyFile; } private SortedMap<String, Long> excludedFiles; @@ -139,12 +150,22 @@ public class DescriptorReaderImpl implements DescriptorReader { : new BlockingIteratorImpl<DescriptorFile>( this.maxDescriptorFilesInQueue); this.reader = new DescriptorReaderRunnable(this.directories, - this.tarballs, descriptorQueue, this.historyFile, - this.excludedFiles, this.failUnrecognizedDescriptorLines); + this.tarballs, descriptorQueue, this.autoSaveHistoryFile, + this.manualSaveHistoryFile, this.excludedFiles, + this.failUnrecognizedDescriptorLines); new Thread(this.reader).start(); return descriptorQueue; } + @Override + public void saveHistoryFile(File historyFile) { + if (!this.reader.hasFinishedReading) { + throw new IllegalStateException("Saving history is only permitted after " + + "reading descriptors."); + } + this.reader.writeNewHistory(historyFile); + } + private static class DescriptorReaderRunnable implements Runnable { private List<File> directories; @@ -153,7 +174,9 @@ public class DescriptorReaderImpl implements DescriptorReader { private BlockingIteratorImpl<DescriptorFile> descriptorQueue; - private File historyFile; + private File autoSaveHistoryFile; + + private File manualSaveHistoryFile; private SortedMap<String, Long> excludedFilesBefore = new TreeMap<>(); @@ -168,12 +191,14 @@ public class DescriptorReaderImpl implements DescriptorReader { private DescriptorReaderRunnable(List<File> directories, List<File> tarballs, BlockingIteratorImpl<DescriptorFile> descriptorQueue, - File historyFile, SortedMap<String, Long> excludedFiles, + File autoSaveHistoryFile, File manualSaveHistoryFile, + SortedMap<String, Long> excludedFiles, boolean failUnrecognizedDescriptorLines) { this.directories = directories; this.tarballs = tarballs; this.descriptorQueue = descriptorQueue; - this.historyFile = historyFile; + this.autoSaveHistoryFile = autoSaveHistoryFile; + this.manualSaveHistoryFile = manualSaveHistoryFile; if (excludedFiles != null) { this.excludedFilesBefore = excludedFiles; } @@ -184,7 +209,8 @@ public class DescriptorReaderImpl implements DescriptorReader { public void run() { try { - this.readOldHistory(); + this.readOldHistory(this.autoSaveHistoryFile); + this.readOldHistory(this.manualSaveHistoryFile); this.readDescriptors(); this.readTarballs(); this.hasFinishedReading = true; @@ -195,17 +221,17 @@ public class DescriptorReaderImpl implements DescriptorReader { this.descriptorQueue.setOutOfDescriptors(); } if (this.hasFinishedReading) { - this.writeNewHistory(); + this.writeNewHistory(this.autoSaveHistoryFile); } } - private void readOldHistory() { - if (this.historyFile == null || !this.historyFile.exists()) { + private void readOldHistory(File historyFile) { + if (historyFile == null || !historyFile.exists()) { return; } try { BufferedReader br = new BufferedReader(new FileReader( - this.historyFile)); + historyFile)); String line; while ((line = br.readLine()) != null) { if (!line.contains(" ")) { @@ -223,16 +249,16 @@ public class DescriptorReaderImpl implements DescriptorReader { } } - private void writeNewHistory() { - if (this.historyFile == null) { + private void writeNewHistory(File historyFile) { + if (historyFile == null) { return; } try { - if (this.historyFile.getParentFile() != null) { - this.historyFile.getParentFile().mkdirs(); + if (historyFile.getParentFile() != null) { + historyFile.getParentFile().mkdirs(); } BufferedWriter bw = new BufferedWriter(new FileWriter( - this.historyFile)); + historyFile)); SortedMap<String, Long> newHistory = new TreeMap<>(); newHistory.putAll(this.excludedFilesAfter); newHistory.putAll(this.parsedFilesAfter);