commit 1df211186efe6b9ae21121644890c616b8601b2a Author: iwakeh <iwakeh@torproject.org> Date: Wed Oct 26 21:56:42 2016 +0200 Reduce file-io by first writing all to temporary files and renaming these temporary files after sync. --- .../collector/persist/PersistenceUtils.java | 68 +++++++++++++++------- .../torproject/collector/sync/SyncPersistence.java | 7 +++ .../collector/persist/PersistUtilsTest.java | 7 ++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/torproject/collector/persist/PersistenceUtils.java b/src/main/java/org/torproject/collector/persist/PersistenceUtils.java index 8133451..d3678f2 100644 --- a/src/main/java/org/torproject/collector/persist/PersistenceUtils.java +++ b/src/main/java/org/torproject/collector/persist/PersistenceUtils.java @@ -9,10 +9,14 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.io.IOException; import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; import java.text.SimpleDateFormat; import java.util.Date; import java.util.TimeZone; @@ -22,6 +26,8 @@ public class PersistenceUtils { private static final Logger log = LoggerFactory.getLogger( PersistenceUtils.class); + public static final String TEMPFIX = ".tmp"; + /** Stores a descriptor adding missing annotations with the given options. */ public static boolean storeToFileSystem(byte[] typeAnnotation, byte[] data, Path outputPath, StandardOpenOption option) { @@ -29,34 +35,25 @@ public class PersistenceUtils { } /** Stores a descriptor adding missing annotations with the given options. - * Uses a temporary file and the copies to the final location. */ - public static boolean storeToFileSystem(byte[] typeAnnotation, byte[] data, - Path outputPath, StandardOpenOption option, boolean useTmp) { + * Uses a temporary file and requires a run of cleanDirectory for moving + * files to the final location. */ + public static boolean storeToFileSystem(byte[] typeAnnotation, + byte[] data, Path outputPath, StandardOpenOption option, boolean useTmp) { Path tmpPath = outputPath; try { if (useTmp) { tmpPath = new File(outputPath.toFile().getParent(), - outputPath.toFile().getName() + ".tmp").toPath(); - if (Files.exists(outputPath) && StandardOpenOption.APPEND == option) { - Files.copy(outputPath, tmpPath, StandardCopyOption.REPLACE_EXISTING); - } else if (Files.exists(outputPath) + outputPath.toFile().getName() + TEMPFIX).toPath(); + if (Files.exists(outputPath) && StandardOpenOption.CREATE_NEW == option) { return false; } + if (Files.exists(outputPath) && !Files.exists(tmpPath) + && StandardOpenOption.APPEND == option) { + Files.copy(outputPath, tmpPath, StandardCopyOption.REPLACE_EXISTING); + } } - StandardOpenOption appendOption = option; - Files.createDirectories(tmpPath.getParent()); - if (data.length > 0 && data[0] != '@') { - Files.write(tmpPath, typeAnnotation, appendOption, - StandardOpenOption.CREATE); - appendOption = StandardOpenOption.APPEND; - } - Files.write(tmpPath, data, appendOption, StandardOpenOption.CREATE); - if (useTmp) { - Files.deleteIfExists(outputPath); - tmpPath.toFile().renameTo(outputPath.toFile()); - } - return true; + return createOrAppend(typeAnnotation, data, tmpPath, option); } catch (FileAlreadyExistsException faee) { log.debug("Already have descriptor(s) of type '{}': {}. Skipping.", new String(typeAnnotation), outputPath); @@ -71,6 +68,37 @@ public class PersistenceUtils { return false; } + private static boolean createOrAppend(byte[] annotation, byte[] data, + Path path, StandardOpenOption option) throws IOException { + StandardOpenOption appendOption = option; + Files.createDirectories(path.getParent()); + if (data.length > 0 && data[0] != '@') { + Files.write(path, annotation, appendOption, StandardOpenOption.CREATE); + appendOption = StandardOpenOption.APPEND; + } + Files.write(path, data, appendOption, StandardOpenOption.CREATE); + return true; + } + + /** Move temporary files to their final location. */ + public static void cleanDirectory(Path pathToClean) throws IOException { + SimpleFileVisitor<Path> sfv = new SimpleFileVisitor<Path>() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) + throws IOException { + String tempName = file.toString(); + if (tempName.endsWith(TEMPFIX)) { + Path outputPath = Paths + .get(tempName.substring(0, tempName.length() - TEMPFIX.length())); + Files.deleteIfExists(outputPath); + file.toFile().renameTo(outputPath.toFile()); + } + return FileVisitResult.CONTINUE; + } + }; + Files.walkFileTree(pathToClean, sfv); + } + /** Return all date-time parts as array. */ public static String[] dateTimeParts(long dateTime) { return dateTimeParts(new Date(dateTime)); diff --git a/src/main/java/org/torproject/collector/sync/SyncPersistence.java b/src/main/java/org/torproject/collector/sync/SyncPersistence.java index 535334a..5c67a75 100644 --- a/src/main/java/org/torproject/collector/sync/SyncPersistence.java +++ b/src/main/java/org/torproject/collector/sync/SyncPersistence.java @@ -13,6 +13,7 @@ import org.torproject.collector.persist.DescriptorPersistence; import org.torproject.collector.persist.ExitlistPersistence; import org.torproject.collector.persist.ExtraInfoPersistence; import org.torproject.collector.persist.MicroConsensusPersistence; +import org.torproject.collector.persist.PersistenceUtils; import org.torproject.collector.persist.ServerDescriptorPersistence; import org.torproject.collector.persist.StatusPersistence; import org.torproject.collector.persist.VotePersistence; @@ -29,6 +30,7 @@ import org.torproject.descriptor.RelayServerDescriptor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Path; import java.util.List; @@ -116,6 +118,11 @@ public class SyncPersistence { desc.getClass().getSimpleName(), desc.getClass().getInterfaces()); } } + try { + PersistenceUtils.cleanDirectory(recentPath); + } catch (IOException ioe) { + log.error("Cleaning of {} failed.", recentPath.toString(), ioe); + } } } diff --git a/src/test/java/org/torproject/collector/persist/PersistUtilsTest.java b/src/test/java/org/torproject/collector/persist/PersistUtilsTest.java index 50ace87..32fa76a 100644 --- a/src/test/java/org/torproject/collector/persist/PersistUtilsTest.java +++ b/src/test/java/org/torproject/collector/persist/PersistUtilsTest.java @@ -45,10 +45,15 @@ public class PersistUtilsTest { public void testCreate() throws Exception { Path out = tmpf.newFolder().toPath(); Path pathToCreate = Paths.get(out.toString(), "very-new-file"); + Path pathToCreateTmp = Paths + .get(out.toString(), "very-new-file" + PersistenceUtils.TEMPFIX); String theText = "some text"; assertTrue("Files should be created.", PersistenceUtils.storeToFileSystem(ANNO1.getBytes(), - (theText + "\n").getBytes(), pathToCreate, StandardOpenOption.CREATE)); + (theText + "\n").getBytes(), pathToCreate, StandardOpenOption.CREATE, + true)); + assertTrue("File wasn't created.", Files.exists(pathToCreateTmp)); + PersistenceUtils.cleanDirectory(out); List<String> text = Files.readAllLines(pathToCreate, StandardCharsets.UTF_8); assertEquals("File contained: " + text, 2, text.size());