Repository: incubator-brooklyn Updated Branches: refs/heads/master 8c269d5be -> e81e9dc79
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. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/56fdffcf Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/56fdffcf Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/56fdffcf Branch: refs/heads/master Commit: 56fdffcf92d73fee4a4bafdf730987d10eff3667 Parents: 7ca2781 Author: Ciprian Ciubotariu <[email protected]> Authored: Tue Sep 15 11:34:44 2015 +0300 Committer: Ciprian Ciubotariu <[email protected]> Committed: Thu Sep 17 20:14:34 2015 +0300 ---------------------------------------------------------------------- .../persist/FileBasedStoreObjectAccessor.java | 2 - .../FileBasedStoreObjectAccessorWriterTest.java | 13 +++ .../brooklyn/util/io/FilePermissions.java | 93 ++++++++++++++++++++ .../org/apache/brooklyn/util/io/FileUtil.java | 75 +++++++--------- 4 files changed, 137 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/56fdffcf/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 41aefc0..b0ae877 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 class FileBasedStoreObjectAccessor implements PersistenceObjectStore.Stor 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 { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/56fdffcf/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 3ee71df..02e08f5 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 class FileBasedStoreObjectAccessorWriterTest extends PersistenceStoreObje 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)); + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/56fdffcf/utils/common/src/main/java/org/apache/brooklyn/util/io/FilePermissions.java ---------------------------------------------------------------------- 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 0000000..d366af5 --- /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 <[email protected]> + */ +public class FilePermissions { + + private final int posixMode; + private final Set<PosixFilePermission> 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); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/56fdffcf/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 cdc65a5..45bea31 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 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 { @@ -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; } }
