Return-Path: X-Original-To: apmail-flume-commits-archive@www.apache.org Delivered-To: apmail-flume-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 15F4ADD6E for ; Wed, 12 Sep 2012 19:31:19 +0000 (UTC) Received: (qmail 20576 invoked by uid 500); 12 Sep 2012 19:31:19 -0000 Delivered-To: apmail-flume-commits-archive@flume.apache.org Received: (qmail 20529 invoked by uid 500); 12 Sep 2012 19:31:19 -0000 Mailing-List: contact commits-help@flume.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flume.apache.org Delivered-To: mailing list commits@flume.apache.org Received: (qmail 20522 invoked by uid 99); 12 Sep 2012 19:31:18 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 12 Sep 2012 19:31:18 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id AA92935F06; Wed, 12 Sep 2012 19:31:18 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: hshreedharan@apache.org To: commits@flume.apache.org X-Mailer: ASF-Git Admin Mailer Subject: git commit: FLUME-1564. FileChannel log file creation could be clarified and tested Message-Id: <20120912193118.AA92935F06@tyr.zones.apache.org> Date: Wed, 12 Sep 2012 19:31:18 +0000 (UTC) Updated Branches: refs/heads/trunk 3080ce09a -> fc86cd540 FLUME-1564. FileChannel log file creation could be clarified and tested (Brock Noland via Hari Shreedharan) Project: http://git-wip-us.apache.org/repos/asf/flume/repo Commit: http://git-wip-us.apache.org/repos/asf/flume/commit/fc86cd54 Tree: http://git-wip-us.apache.org/repos/asf/flume/tree/fc86cd54 Diff: http://git-wip-us.apache.org/repos/asf/flume/diff/fc86cd54 Branch: refs/heads/trunk Commit: fc86cd540d25cf43388cee01818d17cd8c30c5df Parents: 3080ce0 Author: Hari Shreedharan Authored: Wed Sep 12 12:30:22 2012 -0700 Committer: Hari Shreedharan Committed: Wed Sep 12 12:30:22 2012 -0700 ---------------------------------------------------------------------- .../java/org/apache/flume/channel/file/Log.java | 16 ++++---- .../apache/flume/channel/file/LogFileFactory.java | 9 +++-- .../org/apache/flume/channel/file/TestLogFile.java | 26 +++++++++++++++ 3 files changed, 40 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flume/blob/fc86cd54/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ---------------------------------------------------------------------- diff --git a/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java b/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java index f834148..1d91460 100644 --- a/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java +++ b/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java @@ -562,7 +562,10 @@ class Log { } if (logFiles != null) { for (int index = 0; index < logFiles.length(); index++) { - logFiles.get(index).close(); + LogFile.Writer writer = logFiles.get(index); + if(writer != null) { + writer.close(); + } } } synchronized (idLogFileMap) { @@ -677,16 +680,13 @@ class Log { LOGGER.info("Roll start " + logDirs[index]); int fileID = nextFileID.incrementAndGet(); File file = new File(logDirs[index], PREFIX + fileID); - Preconditions.checkState(!file.exists(), - "File already exists " + file); - Preconditions.checkState(file.createNewFile(), - "File could not be created " + file); + LogFile.Writer writer = LogFileFactory.getWriter(file, fileID, + maxFileSize, encryptionKey, encryptionKeyAlias, + encryptionCipherProvider); idLogFileMap.put(fileID, LogFileFactory.getRandomReader(file, encryptionKeyProvider)); // writer from this point on will get new reference - logFiles.set(index, LogFileFactory.getWriter(file, fileID, - maxFileSize, encryptionKey, encryptionKeyAlias, - encryptionCipherProvider)); + logFiles.set(index, writer); // close out old log if (oldLogFile != null) { oldLogFile.close(); http://git-wip-us.apache.org/repos/asf/flume/blob/fc86cd54/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java ---------------------------------------------------------------------- diff --git a/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java b/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java index 7bf6de4..82e14b0 100644 --- a/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java +++ b/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java @@ -29,6 +29,8 @@ import org.apache.flume.channel.file.encryption.KeyProvider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Preconditions; + @SuppressWarnings("deprecation") class LogFileFactory { private static final Logger LOGGER = @@ -65,9 +67,10 @@ class LogFileFactory { long maxFileSize, @Nullable Key encryptionKey, @Nullable String encryptionKeyAlias, @Nullable String encryptionCipherProvider) throws IOException { - if(!(file.exists() || file.createNewFile())) { - throw new IOException("Cannot create " + file); - } + Preconditions.checkState(!file.exists(), "File already exists " + + file.getAbsolutePath()); + Preconditions.checkState(file.createNewFile(), "File could not be created " + + file.getAbsolutePath()); return new LogFileV3.Writer(file, logFileID, maxFileSize, encryptionKey, encryptionKeyAlias, encryptionCipherProvider); } http://git-wip-us.apache.org/repos/asf/flume/blob/fc86cd54/flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java ---------------------------------------------------------------------- diff --git a/flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java b/flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java index 1efdb80..87d9c3f 100644 --- a/flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java +++ b/flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java @@ -66,6 +66,32 @@ public class TestLogFile { } } @Test + public void testWriterRefusesToOverwriteFile() throws IOException { + Assert.assertTrue(dataFile.isFile() || dataFile.createNewFile()); + try { + LogFileFactory.getWriter(dataFile, fileID, 1000, null, null, + null); + Assert.fail(); + } catch (IllegalStateException e) { + Assert.assertEquals("File already exists " + dataFile.getAbsolutePath(), + e.getMessage()); + } + } + @Test + public void testWriterFailsWithDirectory() throws IOException { + FileUtils.deleteQuietly(dataFile); + Assert.assertFalse(dataFile.exists()); + Assert.assertTrue(dataFile.mkdirs()); + try { + LogFileFactory.getWriter(dataFile, fileID, 1000, null, null, + null); + Assert.fail(); + } catch (IllegalStateException e) { + Assert.assertEquals("File already exists " + dataFile.getAbsolutePath(), + e.getMessage()); + } + } + @Test public void testPutGet() throws InterruptedException, IOException { final List errors = Collections.synchronizedList(new ArrayList());