brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [07/11] incubator-brooklyn git commit: BROOKLYN-144: avoid err when persisting ha-record
Date Fri, 13 Nov 2015 02:08:56 GMT
BROOKLYN-144: avoid err when persisting ha-record

- Adds Exception.propagate(msg, throwable) so can include info about
  the file that could not be created.
- Fix FileUtil.setFilePermissions so that createNewFile creates the
  parent directory if necessary.
- HighAvailabilityManager’s polling task: log.err on first exception,
  then log.debug for subsequent consecutive exceptions.

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

Branch: refs/heads/master
Commit: 315babf59cdf7206c7f8aac129a3454e5d82c4b6
Parents: 9460544
Author: Aled Sage <aled.sage@gmail.com>
Authored: Thu Nov 12 20:42:39 2015 +0000
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Thu Nov 12 20:42:39 2015 +0000

----------------------------------------------------------------------
 .../mgmt/ha/HighAvailabilityManagerImpl.java    | 17 ++++++---
 .../persist/FileBasedStoreObjectAccessor.java   | 10 +++---
 .../FileBasedStoreObjectAccessorWriterTest.java | 38 ++++++++++++++++++--
 .../brooklyn/util/exceptions/Exceptions.java    | 29 ++++++++++++---
 .../org/apache/brooklyn/util/io/FileUtil.java   | 18 +++++-----
 .../util/exceptions/ExceptionsTest.java         | 22 ++++++++++++
 6 files changed, 108 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
index 0d8de30..d6a3efa 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
@@ -540,12 +540,20 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager
{
     @SuppressWarnings("unchecked")
     protected void registerPollTask() {
         final Runnable job = new Runnable() {
+            private boolean lastFailed;
+            
             @Override public void run() {
                 try {
                     publishAndCheck(false);
+                    lastFailed = false;
                 } catch (Exception e) {
                     if (running) {
-                        LOG.error("Problem in HA-poller: "+e, e);
+                        if (lastFailed) {
+                            if (LOG.isDebugEnabled()) LOG.debug("Recurring problem in HA-poller:
"+e, e);
+                        } else {
+                            LOG.error("Problem in HA-poller: "+e, e);
+                            lastFailed = true;
+                        }
                     } else {
                         if (LOG.isDebugEnabled()) LOG.debug("Problem in HA-poller, but no
longer running: "+e, e);
                     }
@@ -562,8 +570,9 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager
{
             }
         };
         
-        LOG.debug("Registering poll task for "+this+", period "+getPollPeriod());
-        if (getPollPeriod().equals(Duration.PRACTICALLY_FOREVER)) {
+        Duration pollPeriod = getPollPeriod();
+        LOG.debug("Registering poll task for "+this+", period "+pollPeriod);
+        if (pollPeriod.equals(Duration.PRACTICALLY_FOREVER)) {
             // don't schedule - used for tests
             // (scheduling fires off one initial task in the background before the delay,

             // which affects tests that want to know exactly when publishing happens;
@@ -571,7 +580,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager
{
         } else {
             if (pollingTask!=null) pollingTask.cancel(true);
             
-            ScheduledTask task = new ScheduledTask(MutableMap.of("period", getPollPeriod(),
"displayName", "scheduled:[HA poller task]"), taskFactory);
+            ScheduledTask task = new ScheduledTask(MutableMap.of("period", pollPeriod, "displayName",
"scheduled:[HA poller task]"), taskFactory);
             pollingTask = managementContext.getExecutionManager().submit(task);
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessor.java
b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessor.java
index b0ae877..f061884 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessor.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessor.java
@@ -30,7 +30,6 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Charsets;
 import com.google.common.base.Objects;
-import com.google.common.base.Throwables;
 import com.google.common.io.Files;
 
 /**
@@ -56,7 +55,7 @@ public class FileBasedStoreObjectAccessor implements PersistenceObjectStore.Stor
             if (!exists()) return null;
             return Files.asCharSource(file, Charsets.UTF_8).read();
         } catch (IOException e) {
-            throw Throwables.propagate(e);
+            throw Exceptions.propagate("Problem reading String contents of file "+file, e);
         }
     }
 
@@ -66,7 +65,7 @@ public class FileBasedStoreObjectAccessor implements PersistenceObjectStore.Stor
             if (!exists()) return null;
             return Files.asByteSource(file).read();
         } catch (IOException e) {
-            throw Throwables.propagate(e);
+            throw Exceptions.propagate("Problem reading bytes of file "+file, e);
         }
     }
 
@@ -83,12 +82,13 @@ public class FileBasedStoreObjectAccessor implements PersistenceObjectStore.Stor
             Files.write(val, tmpFile, Charsets.UTF_8);
             FileBasedObjectStore.moveFile(tmpFile, file);
         } catch (IOException e) {
-            throw Exceptions.propagate(e);
+            throw Exceptions.propagate("Problem writing data to file "+file+" (via temporary
file "+tmpFile+")", e);
         } catch (InterruptedException e) {
             throw Exceptions.propagate(e);
         }
     }
 
+    // TODO Should this write to the temporary file? Otherwise we'll risk getting a partial
view of the write.
     @Override
     public void append(String val) {
         try {
@@ -97,7 +97,7 @@ public class FileBasedStoreObjectAccessor implements PersistenceObjectStore.Stor
             Files.append(val, file, Charsets.UTF_8);
             
         } catch (IOException e) {
-            throw Exceptions.propagate(e);
+            throw Exceptions.propagate("Problem appending to file "+file, e);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java
b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java
index 14ae4b2..f6db2df 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java
@@ -22,6 +22,7 @@ import static org.testng.Assert.assertEquals;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore.StoreObjectAccessorWithLock;
 import org.apache.brooklyn.util.os.Os;
@@ -29,6 +30,7 @@ import org.apache.brooklyn.util.time.Duration;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Charsets;
+import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.Files;
 
@@ -61,16 +63,46 @@ public class FileBasedStoreObjectAccessorWriterTest extends PersistenceStoreObje
         FileBasedObjectStoreTest.assertFilePermission600(file);
     }
 
-    @Test(enabled = false)
+    @Test(groups="Integration")
+    public void testPutCreatesNewFile() throws Exception {
+        File nonExistantFile = Os.newTempFile(getClass(), "txt");
+        nonExistantFile.delete();
+        StoreObjectAccessorLocking accessor = new StoreObjectAccessorLocking(new FileBasedStoreObjectAccessor(nonExistantFile,
".tmp"));
+        try {
+            accessor.put("abc");
+            assertEquals(Files.readLines(nonExistantFile, Charsets.UTF_8), ImmutableList.of("abc"));
+        } finally {
+            accessor.delete();
+        }
+    }
+
+    @Test(groups="Integration")
+    public void testPutCreatesNewFileAndParentDir() throws Exception {
+        File nonExistantDir = Os.newTempDir(getClass());
+        nonExistantDir.delete();
+        File nonExistantFile = new File(nonExistantDir, "file.txt");
+        StoreObjectAccessorLocking accessor = new StoreObjectAccessorLocking(new FileBasedStoreObjectAccessor(nonExistantFile,
".tmp"));
+        try {
+            accessor.put("abc");
+            assertEquals(Files.readLines(nonExistantFile, Charsets.UTF_8), ImmutableList.of("abc"));
+        } finally {
+            accessor.delete();
+        }
+    }
+
+    @Test(groups={"Integration", "Acceptance"})
     public void testFilePermissionsPerformance() throws Exception {
         long interval = 10 * 1000; // millis
         long start = System.currentTimeMillis();
 
         int count = 0;
+        Stopwatch stopwatch = Stopwatch.createStarted();
         while (System.currentTimeMillis() < start + interval) {
             accessor.put("abc" + count);
-            ++count;
+            count++;
         }
-        System.out.println("writes per second:" + (count * 1000 / interval));
+        stopwatch.stop();
+        double writesPerSec = ((double)count) / stopwatch.elapsed(TimeUnit.MILLISECONDS)
* 1000;
+        System.out.println("writes per second: " + writesPerSec);
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
index 9439faa..9643018 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
@@ -97,12 +97,31 @@ public class Exceptions {
      * <li> wraps as PropagatedRuntimeException for easier filtering
      */
     public static RuntimeException propagate(Throwable throwable) {
-        if (throwable instanceof InterruptedException)
+        if (throwable instanceof InterruptedException) {
             throw new RuntimeInterruptedException((InterruptedException) throwable);
+        } else if (throwable instanceof RuntimeInterruptedException) {
+            Thread.currentThread().interrupt();
+            throw (RuntimeInterruptedException) throwable;
+        }
         Throwables.propagateIfPossible(checkNotNull(throwable));
         throw new PropagatedRuntimeException(throwable);
     }
 
+    /**
+     * See {@link #propagate(Throwable)}. If wrapping the exception, then include the given
message;
+     * otherwise the message is not used.
+     */
+    public static RuntimeException propagate(String msg, Throwable throwable) {
+        if (throwable instanceof InterruptedException) {
+            throw new RuntimeInterruptedException(msg, (InterruptedException) throwable);
+        } else if (throwable instanceof RuntimeInterruptedException) {
+            Thread.currentThread().interrupt();
+            throw (RuntimeInterruptedException) throwable;
+        }
+        Throwables.propagateIfPossible(checkNotNull(throwable));
+        throw new PropagatedRuntimeException(msg, throwable);
+    }
+    
     /** 
      * Propagate exceptions which are fatal.
      * <p>
@@ -110,12 +129,14 @@ public class Exceptions {
      * such as {@link InterruptedException} and {@link Error}s.
      */
     public static void propagateIfFatal(Throwable throwable) {
-        if (throwable instanceof InterruptedException)
+        if (throwable instanceof InterruptedException) {
             throw new RuntimeInterruptedException((InterruptedException) throwable);
-        if (throwable instanceof RuntimeInterruptedException)
+        } else if (throwable instanceof RuntimeInterruptedException) {
+            Thread.currentThread().interrupt();
             throw (RuntimeInterruptedException) throwable;
-        if (throwable instanceof Error)
+        } else if (throwable instanceof Error) {
             throw (Error) throwable;
+        }
     }
 
     /** returns the first exception of the given type, or null */

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java b/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java
index 45bea31..a4908f2 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/io/FileUtil.java
@@ -26,11 +26,9 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.List;
-import java.util.regex.Pattern;
 
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.io.FileUtil;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.stream.StreamGobbler;
 import org.apache.brooklyn.util.stream.Streams;
@@ -40,12 +38,6 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableList;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
-import java.util.Set;
 
 public class FileUtil {
 
@@ -57,7 +49,7 @@ public class FileUtil {
     private static final FilePermissions permissions700 = new FilePermissions(0700);
 
     public static void setFilePermissionsTo700(File file) throws IOException {
-        file.createNewFile();
+        createNewFile(file);
         try {
             permissions700.apply(file);
             if (LOG.isTraceEnabled()) LOG.trace("Set permissions to 700 for file {}", file.getAbsolutePath());
@@ -67,7 +59,7 @@ public class FileUtil {
     }
 
     public static void setFilePermissionsTo600(File file) throws IOException {
-        file.createNewFile();
+        createNewFile(file);
         try {
             permissions600.apply(file);
             if (LOG.isTraceEnabled()) LOG.trace("Set permissions to 600 for file {}", file.getAbsolutePath());
@@ -186,4 +178,10 @@ public class FileUtil {
         }
     }
 
+    private static boolean createNewFile(File file) throws IOException {
+        if (!file.getParentFile().exists()) {
+            file.getParentFile().mkdirs();
+        }
+        return file.createNewFile();
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/315babf5/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
index 0e7d590..dee7651 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
@@ -59,6 +59,28 @@ public class ExceptionsTest {
     }
     
     @Test
+    public void testPropagateCheckedExceptionWithMessage() throws Exception {
+        String extraMsg = "my message";
+        Exception tothrow = new Exception("simulated");
+        try {
+            throw Exceptions.propagate(extraMsg, tothrow);
+        } catch (RuntimeException e) {
+            assertEquals(e.getMessage(), "my message");
+            assertEquals(e.getCause(), tothrow);
+        }
+    }
+    
+    @Test
+    public void testPropagateRuntimeExceptionIgnoresMessage() throws Exception {
+        NullPointerException tothrow = new NullPointerException("simulated");
+        try {
+            throw Exceptions.propagate("my message", tothrow);
+        } catch (NullPointerException e) {
+            assertEquals(e, tothrow);
+        }
+    }
+    
+    @Test
     public void testPropagateIfFatalPropagatesInterruptedException() throws Exception {
         InterruptedException tothrow = new InterruptedException("simulated");
         try {


Mime
View raw message