You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2015/09/16 14:22:30 UTC

[1/2] incubator-brooklyn git commit: Fix FileBasedObjectStore#moveFile

Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 0aed0a1e3 -> bf3d67412


Fix FileBasedObjectStore#moveFile

- change method to use java.nio.file.Files
- add condition to delete existing destFile


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/1689a6f1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/1689a6f1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/1689a6f1

Branch: refs/heads/master
Commit: 1689a6f192b2c98774622430a45a442e7154f7ca
Parents: 4ea5247
Author: Alexandru Muntean <al...@apifocal.com>
Authored: Mon Sep 14 11:58:13 2015 +0300
Committer: Alexandru Muntean <al...@apifocal.com>
Committed: Tue Sep 15 18:50:07 2015 +0300

----------------------------------------------------------------------
 .../core/mgmt/persist/FileBasedObjectStore.java | 56 ++++++--------------
 1 file changed, 15 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1689a6f1/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
index cd54053..40f9055 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
@@ -24,6 +24,10 @@ import static java.lang.String.format;
 import java.io.File;
 import java.io.FileFilter;
 import java.io.IOException;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.DirectoryNotEmptyException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
 import java.text.SimpleDateFormat;
 import java.util.Arrays;
 import java.util.Date;
@@ -37,9 +41,6 @@ import javax.annotation.Nullable;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.core.internal.ssh.process.ProcessTool;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.FatalConfigurationRuntimeException;
 import org.apache.brooklyn.util.io.FileUtil;
@@ -53,7 +54,6 @@ import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
-import com.google.common.io.Files;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 
@@ -346,53 +346,27 @@ public class FileBasedObjectStore implements PersistenceObjectStore {
         return newDir;
     }
 
-    private static boolean WARNED_ON_NON_ATOMIC_FILE_UPDATES = false; 
     /** 
      * Attempts an fs level atomic move then fall back to pure java rename.
      * Assumes files are on same mount point.
-     * <p>
-     * TODO Java 7 gives an atomic Files.move() which would be preferred.
+     * Overwriting existing destFile
      */
     static void moveFile(File srcFile, File destFile) throws IOException, InterruptedException {
-        // Try rename first - it is a *much* cheaper call than invoking a system call in Java. 
-        // However, rename is not guaranteed cross platform to succeed if the destination exists,
-        // and not guaranteed to be atomic, but it usually seems to do the right thing...
-        boolean result;
-        result = srcFile.renameTo(destFile);
-        if (result) {
-            if (log.isTraceEnabled()) log.trace("java rename of {} to {} completed", srcFile, destFile);
-            return;
+        if (destFile.isDirectory()) {
+            deleteCompletely(destFile);
         }
-        
-        if (!Os.isMicrosoftWindows()) {
-            // this command, if it succeeds, is guaranteed to be atomic, and it will usually overwrite
-            String cmd = "mv '"+srcFile.getAbsolutePath()+"' '"+destFile.getAbsolutePath()+"'";
-            
-            int exitStatus = new ProcessTool().execCommands(MutableMap.<String,String>of(), MutableList.of(cmd), null);
-            // prefer the above to the below because it wraps it in the appropriate bash
-//            Process proc = Runtime.getRuntime().exec(cmd);
-//            result = proc.waitFor();
-            
-            if (log.isTraceEnabled()) log.trace("FS move of {} to {} completed, code {}", new Object[] { srcFile, destFile, exitStatus });
-            if (exitStatus == 0) return;
+
+        try {
+            Files.move(srcFile.toPath(), destFile.toPath(), StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
+        } catch (AtomicMoveNotSupportedException e) {
+            Files.move(srcFile.toPath(), destFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
         }
         
-        // finally try a delete - but explicitly warn this is not going to be atomic
-        // so if another node reads it might see no master
-        if (!WARNED_ON_NON_ATOMIC_FILE_UPDATES) {
-            WARNED_ON_NON_ATOMIC_FILE_UPDATES = true;
-            log.warn("Unable to perform atomic file update ("+srcFile+" to "+destFile+"); file system not recommended for production HA/DR");
+        if (log.isTraceEnabled()) {
+            log.trace("Completly moved from {} to {} completed", new Object[] { srcFile, destFile });
         }
-        destFile.delete();
-        result = srcFile.renameTo(destFile);
-        if (log.isTraceEnabled()) log.trace("java delete and rename of {} to {} completed, code {}", new Object[] { srcFile, destFile, result });
-        if (result) 
-            return;
-        Files.copy(srcFile, destFile);
-        srcFile.delete();
-        throw new IOException("Could not move "+destFile+" to "+srcFile);
     }
-    
+
     /**
      * True if directory exists, but is entirely empty, or only contains empty directories.
      */


[2/2] incubator-brooklyn git commit: This closes #891

Posted by al...@apache.org.
This closes #891


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/bf3d6741
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/bf3d6741
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/bf3d6741

Branch: refs/heads/master
Commit: bf3d67412356306fbe9dc8ef24f708f8d17c0c49
Parents: 0aed0a1 1689a6f
Author: Aled Sage <al...@gmail.com>
Authored: Wed Sep 16 13:22:16 2015 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Wed Sep 16 13:22:16 2015 +0100

----------------------------------------------------------------------
 .../core/mgmt/persist/FileBasedObjectStore.java | 56 ++++++--------------
 1 file changed, 15 insertions(+), 41 deletions(-)
----------------------------------------------------------------------