From 56fdffcf92d73fee4a4bafdf730987d10eff3667 Mon Sep 17 00:00:00 2001 From: Ciprian Ciubotariu Date: Tue, 15 Sep 2015 11:34:44 +0300 Subject: [PATCH 1/3] Use java7 Files.setPosixFilePermissions on posix filesystems Fall back to java6 File.setReadable/setWritable/setExecutable on windows and when posix fails. This improves performance considerably on posix. --- .../persist/FileBasedStoreObjectAccessor.java | 2 - ...ileBasedStoreObjectAccessorWriterTest.java | 13 +++ .../brooklyn/util/io/FilePermissions.java | 93 +++++++++++++++++++ .../org/apache/brooklyn/util/io/FileUtil.java | 75 +++++++-------- 4 files changed, 137 insertions(+), 46 deletions(-) create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/io/FilePermissions.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 41aefc0d42..b0ae877b40 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 @@ -75,8 +75,6 @@ public boolean exists() { return file.exists(); } - // Setting permissions to 600 reduces objectAccessor.put performance from about 5000 per second to 3000 per second - // in java 6. With Java 7's Files.setPosixFilePermissions, this might well improve. @Override public void put(String val) { try { 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 3ee71df172..02e08f570a 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 @@ -62,4 +62,17 @@ public void testFilePermissions600() throws Exception { FileBasedObjectStoreTest.assertFilePermission600(file); } + + @Test(enabled = false) + public void testFilePermissionsPerformance() throws Exception { + long interval = 10 * 1000; // millis + long start = System.currentTimeMillis(); + + int count = 0; + while (System.currentTimeMillis() < start + interval) { + accessor.put("abc" + count); + ++count; + } + System.out.println("writes per second:" + (count * 1000 / interval)); + } } diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/io/FilePermissions.java b/utils/common/src/main/java/org/apache/brooklyn/util/io/FilePermissions.java new file mode 100644 index 0000000000..d366af5828 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/io/FilePermissions.java @@ -0,0 +1,93 @@ +/* + * Copyright 2015 The Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.brooklyn.util.io; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import static java.nio.file.attribute.PosixFilePermission.GROUP_EXECUTE; +import static java.nio.file.attribute.PosixFilePermission.GROUP_READ; +import static java.nio.file.attribute.PosixFilePermission.GROUP_WRITE; +import static java.nio.file.attribute.PosixFilePermission.OTHERS_READ; +import static java.nio.file.attribute.PosixFilePermission.OTHERS_WRITE; +import static java.nio.file.attribute.PosixFilePermission.OTHERS_EXECUTE; +import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; +import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; +import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE; +import java.util.EnumSet; +import java.util.Set; +import org.apache.brooklyn.util.os.Os; + +/** + * Portable file permissions. + * + * @author Ciprian Ciubotariu + */ +public class FilePermissions { + + private final int posixMode; + private final Set posixPermissions = EnumSet.noneOf(PosixFilePermission.class); + + public FilePermissions(int posixMode) { + this.posixMode = posixMode; + if ((posixMode & 0400) != 0) posixPermissions.add(OWNER_READ); + if ((posixMode & 0200) != 0) posixPermissions.add(OWNER_WRITE); + if ((posixMode & 0100) != 0) posixPermissions.add(OWNER_EXECUTE); + if ((posixMode & 0040) != 0) posixPermissions.add(GROUP_READ); + if ((posixMode & 0020) != 0) posixPermissions.add(GROUP_WRITE); + if ((posixMode & 0010) != 0) posixPermissions.add(GROUP_EXECUTE); + if ((posixMode & 0004) != 0) posixPermissions.add(OTHERS_READ); + if ((posixMode & 0002) != 0) posixPermissions.add(OTHERS_WRITE); + if ((posixMode & 0001) != 0) posixPermissions.add(OTHERS_EXECUTE); + } + + public void apply(File file) throws IOException { + Path filePath = file.toPath(); + + // the appropriate condition is actually Files.getFileStore(filePath).supportsFileAttributeView(PosixFileAttributeView.class) + // but that downs performance to ~9000 calls per second + + boolean done = false; + try { + // ~59000 calls per sec + if (!Os.isMicrosoftWindows()) { + Files.setPosixFilePermissions(filePath, posixPermissions); + } + done = true; + } catch (UnsupportedOperationException ex) {} + + if (!done) { + // ~42000 calls per sec + // TODO: what happens to group permissions ? + boolean setRead = file.setReadable(posixPermissions.contains(OTHERS_READ), false) & file.setReadable(posixPermissions.contains(OWNER_READ), true); + boolean setWrite = file.setWritable(posixPermissions.contains(OTHERS_WRITE), false) & file.setWritable(posixPermissions.contains(OWNER_WRITE), true); + boolean setExec = file.setExecutable(posixPermissions.contains(OTHERS_EXECUTE), false) & file.setExecutable(posixPermissions.contains(OWNER_EXECUTE), true); + + if (!(setRead && setWrite && setExec)) { + throw new IOException("setting file permissions failed: read=" + setRead + " write=" + setWrite + " exec=" + setExec); + } + } + } + + @Override + public String toString() { + return "posix mode " + Integer.toOctalString(posixMode); + } + +} 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 cdc65a5a21..45bea31859 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 @@ -40,6 +40,12 @@ 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 { @@ -47,61 +53,42 @@ public class FileUtil { private static boolean loggedSetFilePermissionsWarning = false; - // When we move to java 7, we can use Files.setPosixFilePermissions + private static final FilePermissions permissions600 = new FilePermissions(0600); + private static final FilePermissions permissions700 = new FilePermissions(0700); + public static void setFilePermissionsTo700(File file) throws IOException { file.createNewFile(); - boolean setRead = file.setReadable(false, false) & file.setReadable(true, true); - boolean setWrite = file.setWritable(false, false) & file.setWritable(true, true); - boolean setExec = file.setExecutable(false, false) & file.setExecutable(true, true); - - if (setRead && setWrite && setExec) { + try { + permissions700.apply(file); if (LOG.isTraceEnabled()) LOG.trace("Set permissions to 700 for file {}", file.getAbsolutePath()); - } else { - if (loggedSetFilePermissionsWarning) { - if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); - } else { - if (Os.isMicrosoftWindows()) { - if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 700 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); - } else { - LOG.warn("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); - } - loggedSetFilePermissionsWarning = true; - } + } catch (IOException ex) { + logSetFilePermissionsFailure("700", file, ex); } } - - // When we move to java 7, we can use Files.setPosixFilePermissions + public static void setFilePermissionsTo600(File file) throws IOException { file.createNewFile(); - file.setExecutable(false, false); - file.setReadable(false, false); - file.setWritable(false, false); - file.setReadable(true, true); - file.setWritable(true, true); - - boolean setRead = file.setReadable(false, false) & file.setReadable(true, true); - boolean setWrite = file.setWritable(false, false) & file.setWritable(true, true); - boolean setExec = file.setExecutable(false, false); - - if (setRead && setWrite && setExec) { + try { + permissions600.apply(file); if (LOG.isTraceEnabled()) LOG.trace("Set permissions to 600 for file {}", file.getAbsolutePath()); + } catch (IOException ex) { + logSetFilePermissionsFailure("600", file, ex); + } + } + + private static void logSetFilePermissionsFailure(final String permissions, File file, IOException ex) { + if (loggedSetFilePermissionsWarning) { + if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to {} for file {}: {}", + new Object[] {permissions, file.getAbsolutePath(), ex}); } else { - if (loggedSetFilePermissionsWarning) { - if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); + if (Os.isMicrosoftWindows()) { + if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to {} for file {}; expected behaviour on Windows; {}; subsequent failures (on any file) will be logged at trace", + new Object[] {permissions, file.getAbsolutePath(), ex}); } else { - if (Os.isMicrosoftWindows()) { - if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 600 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); - } else { - LOG.warn("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace", - new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec}); - } - loggedSetFilePermissionsWarning = true; + LOG.warn("Failed to set permissions to {} for file {}: {}; subsequent failures (on any file) will be logged at trace", + new Object[] {permissions, file.getAbsolutePath(), ex}); } + loggedSetFilePermissionsWarning = true; } } From 56e75af2099c23683f9b234fe9a78616246b506e Mon Sep 17 00:00:00 2001 From: Ciprian Ciubotariu Date: Tue, 15 Sep 2015 11:35:57 +0300 Subject: [PATCH 2/3] Remove unneeded imports --- .../brooklyn/core/mgmt/persist/FileBasedObjectStoreTest.java | 2 -- .../mgmt/persist/FileBasedStoreObjectAccessorWriterTest.java | 2 -- .../apache/brooklyn/core/mgmt/persist/ListeningObjectStore.java | 2 -- 3 files changed, 6 deletions(-) diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStoreTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStoreTest.java index fc681dc0c1..a072b69755 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStoreTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStoreTest.java @@ -27,8 +27,6 @@ import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode; import org.apache.brooklyn.core.entity.Entities; -import org.apache.brooklyn.core.mgmt.persist.FileBasedObjectStore; -import org.apache.brooklyn.core.mgmt.persist.PersistMode; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.util.io.FileUtil; import org.apache.brooklyn.util.os.Os; 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 02e08f570a..14ae4b274f 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 @@ -23,8 +23,6 @@ import java.io.File; import java.io.IOException; -import org.apache.brooklyn.core.mgmt.persist.FileBasedStoreObjectAccessor; -import org.apache.brooklyn.core.mgmt.persist.StoreObjectAccessorLocking; import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore.StoreObjectAccessorWithLock; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.time.Duration; diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/ListeningObjectStore.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/ListeningObjectStore.java index c27011eb72..436638d39f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/ListeningObjectStore.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/ListeningObjectStore.java @@ -26,8 +26,6 @@ import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode; -import org.apache.brooklyn.core.mgmt.persist.PersistMode; -import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.CountdownTimer; From 4fde5cdc194bc5c324456db604aba737a72be33d Mon Sep 17 00:00:00 2001 From: Ciprian Ciubotariu Date: Thu, 17 Sep 2015 20:01:32 +0300 Subject: [PATCH 3/3] Optimize Os.isMicrosoftWindows() --- .../src/main/java/org/apache/brooklyn/util/os/Os.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java index 13ba63b715..6c4ee567d5 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java @@ -51,6 +51,7 @@ import com.google.common.collect.Iterables; import com.google.common.io.ByteStreams; import com.google.common.io.Files; +import java.util.concurrent.atomic.AtomicBoolean; public class Os { @@ -63,6 +64,8 @@ public class Os { public static final String LINE_SEPARATOR = System.getProperty("line.separator"); + private static final boolean isMSWin = testForMicrosoftWindows(); + /** returns the best tmp dir to use; see {@link TmpDirFinder} for the logic * (and the explanation why this is needed!) */ public static String tmp() { @@ -514,6 +517,10 @@ public static String nativePath(String path) { } public static boolean isMicrosoftWindows() { + return isMSWin; + } + + private static boolean testForMicrosoftWindows() { String os = System.getProperty("os.name").toLowerCase(); //see org.apache.commons.lang.SystemUtils.IS_WINDOWS return os.startsWith("windows");