Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id EA8C3200CD7 for ; Tue, 1 Aug 2017 17:47:28 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id E8B55167680; Tue, 1 Aug 2017 15:47:28 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id C65BF167676 for ; Tue, 1 Aug 2017 17:47:27 +0200 (CEST) Received: (qmail 19857 invoked by uid 500); 1 Aug 2017 15:47:25 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 19559 invoked by uid 99); 1 Aug 2017 15:47:25 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 01 Aug 2017 15:47:25 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 36B48F3260; Tue, 1 Aug 2017 15:47:24 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: xgong@apache.org To: common-commits@hadoop.apache.org Date: Tue, 01 Aug 2017 15:47:31 -0000 Message-Id: In-Reply-To: <378bcba2df4f450496806a63f2a18670@git.apache.org> References: <378bcba2df4f450496806a63f2a18670@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [08/20] hadoop git commit: HADOOP-14397. Pull up the builder pattern to FileSystem and add AbstractContractCreateTest for it. (Lei (Eddy) Xu) archived-at: Tue, 01 Aug 2017 15:47:29 -0000 HADOOP-14397. Pull up the builder pattern to FileSystem and add AbstractContractCreateTest for it. (Lei (Eddy) Xu) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9586b0e2 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9586b0e2 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9586b0e2 Branch: refs/heads/YARN-5734 Commit: 9586b0e24fce29c278134658e68b8c47cd9d8c51 Parents: abbf412 Author: Lei Xu Authored: Mon Jul 31 20:04:57 2017 -0700 Committer: Lei Xu Committed: Mon Jul 31 20:12:40 2017 -0700 ---------------------------------------------------------------------- .../hadoop/fs/FSDataOutputStreamBuilder.java | 4 +- .../java/org/apache/hadoop/fs/FileSystem.java | 24 ++++-- .../apache/hadoop/fs/TestLocalFileSystem.java | 2 +- .../fs/contract/AbstractContractAppendTest.java | 33 ++++++- .../fs/contract/AbstractContractCreateTest.java | 90 ++++++++++++++------ .../hadoop/fs/contract/ContractTestUtils.java | 43 ++++++++-- .../hadoop/hdfs/DistributedFileSystem.java | 3 +- 7 files changed, 154 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSDataOutputStreamBuilder.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSDataOutputStreamBuilder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSDataOutputStreamBuilder.java index 0527202..8608a7b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSDataOutputStreamBuilder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSDataOutputStreamBuilder.java @@ -44,8 +44,8 @@ import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.IO_FILE_BUFFER_ * * To create missing parent directory, use {@link #recursive()}. */ -@InterfaceAudience.Private -@InterfaceStability.Unstable +@InterfaceAudience.Public +@InterfaceStability.Evolving public abstract class FSDataOutputStreamBuilder > { private final FileSystem fs; http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java index d7cd7dd..fc7b9b2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java @@ -4153,9 +4153,21 @@ public abstract class FileSystem extends Configured implements Closeable { @Override public FSDataOutputStream build() throws IOException { - return getFS().create(getPath(), getPermission(), getFlags(), - getBufferSize(), getReplication(), getBlockSize(), getProgress(), - getChecksumOpt()); + if (getFlags().contains(CreateFlag.CREATE) || + getFlags().contains(CreateFlag.OVERWRITE)) { + if (isRecursive()) { + return getFS().create(getPath(), getPermission(), getFlags(), + getBufferSize(), getReplication(), getBlockSize(), getProgress(), + getChecksumOpt()); + } else { + return getFS().createNonRecursive(getPath(), getPermission(), + getFlags(), getBufferSize(), getReplication(), getBlockSize(), + getProgress()); + } + } else if (getFlags().contains(CreateFlag.APPEND)) { + return getFS().append(getPath(), getBufferSize(), getProgress()); + } + throw new IOException("Must specify either create, overwrite or append"); } @Override @@ -4174,8 +4186,7 @@ public abstract class FileSystem extends Configured implements Closeable { * HADOOP-14384. Temporarily reduce the visibility of method before the * builder interface becomes stable. */ - @InterfaceAudience.Private - protected FSDataOutputStreamBuilder createFile(Path path) { + public FSDataOutputStreamBuilder createFile(Path path) { return new FileSystemDataOutputStreamBuilder(this, path) .create().overwrite(true); } @@ -4185,8 +4196,7 @@ public abstract class FileSystem extends Configured implements Closeable { * @param path file path. * @return a {@link FSDataOutputStreamBuilder} to build file append request. */ - @InterfaceAudience.Private - protected FSDataOutputStreamBuilder appendFile(Path path) { + public FSDataOutputStreamBuilder appendFile(Path path) { return new FileSystemDataOutputStreamBuilder(this, path).append(); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java index 527b9eb..00cedc3 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java @@ -659,7 +659,7 @@ public class TestLocalFileSystem { try { FSDataOutputStreamBuilder builder = - fileSys.createFile(path); + fileSys.createFile(path).recursive(); FSDataOutputStream out = builder.build(); String content = "Create with a generic type of createFile!"; byte[] contentOrigin = content.getBytes("UTF8"); http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java index 6b3e98b..d61b635 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java @@ -61,6 +61,19 @@ public abstract class AbstractContractAppendTest extends AbstractFSContractTestB } @Test + public void testBuilderAppendToEmptyFile() throws Throwable { + touch(getFileSystem(), target); + byte[] dataset = dataset(256, 'a', 'z'); + try (FSDataOutputStream outputStream = + getFileSystem().appendFile(target).build()) { + outputStream.write(dataset); + } + byte[] bytes = ContractTestUtils.readDataset(getFileSystem(), target, + dataset.length); + ContractTestUtils.compareByteArrays(dataset, bytes, dataset.length); + } + + @Test public void testAppendNonexistentFile() throws Throwable { try { FSDataOutputStream out = getFileSystem().append(target); @@ -78,9 +91,9 @@ public abstract class AbstractContractAppendTest extends AbstractFSContractTestB byte[] original = dataset(8192, 'A', 'Z'); byte[] appended = dataset(8192, '0', '9'); createFile(getFileSystem(), target, false, original); - FSDataOutputStream outputStream = getFileSystem().append(target); - outputStream.write(appended); - outputStream.close(); + try (FSDataOutputStream out = getFileSystem().append(target)) { + out.write(appended); + } byte[] bytes = ContractTestUtils.readDataset(getFileSystem(), target, original.length + appended.length); ContractTestUtils.validateFileContent(bytes, @@ -88,6 +101,20 @@ public abstract class AbstractContractAppendTest extends AbstractFSContractTestB } @Test + public void testBuilderAppendToExistingFile() throws Throwable { + byte[] original = dataset(8192, 'A', 'Z'); + byte[] appended = dataset(8192, '0', '9'); + createFile(getFileSystem(), target, false, original); + try (FSDataOutputStream out = getFileSystem().appendFile(target).build()) { + out.write(appended); + } + byte[] bytes = ContractTestUtils.readDataset(getFileSystem(), target, + original.length + appended.length); + ContractTestUtils.validateFileContent(bytes, + new byte[][]{original, appended}); + } + + @Test public void testAppendMissingTarget() throws Throwable { try { FSDataOutputStream out = getFileSystem().append(target); http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java index a9ce607..2053f50 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java @@ -47,24 +47,37 @@ public abstract class AbstractContractCreateTest extends */ public static final int CREATE_TIMEOUT = 15000; - @Test - public void testCreateNewFile() throws Throwable { - describe("Foundational 'create a file' test"); - Path path = path("testCreateNewFile"); + protected Path path(String filepath, boolean useBuilder) throws IOException { + return super.path(filepath + (useBuilder ? "" : "-builder")); + } + + private void testCreateNewFile(boolean useBuilder) throws Throwable { + describe("Foundational 'create a file' test, using builder API=" + + useBuilder); + Path path = path("testCreateNewFile", useBuilder); byte[] data = dataset(256, 'a', 'z'); - writeDataset(getFileSystem(), path, data, data.length, 1024 * 1024, false); + writeDataset(getFileSystem(), path, data, data.length, 1024 * 1024, false, + useBuilder); ContractTestUtils.verifyFileContents(getFileSystem(), path, data); } @Test - public void testCreateFileOverExistingFileNoOverwrite() throws Throwable { - describe("Verify overwriting an existing file fails"); - Path path = path("testCreateFileOverExistingFileNoOverwrite"); + public void testCreateNewFile() throws Throwable { + testCreateNewFile(true); + testCreateNewFile(false); + } + + private void testCreateFileOverExistingFileNoOverwrite(boolean useBuilder) + throws Throwable { + describe("Verify overwriting an existing file fails, using builder API=" + + useBuilder); + Path path = path("testCreateFileOverExistingFileNoOverwrite", useBuilder); byte[] data = dataset(256, 'a', 'z'); writeDataset(getFileSystem(), path, data, data.length, 1024, false); byte[] data2 = dataset(10 * 1024, 'A', 'Z'); try { - writeDataset(getFileSystem(), path, data2, data2.length, 1024, false); + writeDataset(getFileSystem(), path, data2, data2.length, 1024, false, + useBuilder); fail("writing without overwrite unexpectedly succeeded"); } catch (FileAlreadyExistsException expected) { //expected @@ -76,6 +89,26 @@ public abstract class AbstractContractCreateTest extends } } + @Test + public void testCreateFileOverExistingFileNoOverwrite() throws Throwable { + testCreateFileOverExistingFileNoOverwrite(false); + testCreateFileOverExistingFileNoOverwrite(true); + } + + private void testOverwriteExistingFile(boolean useBuilder) throws Throwable { + describe("Overwrite an existing file and verify the new data is there, " + + "use builder API=" + useBuilder); + Path path = path("testOverwriteExistingFile", useBuilder); + byte[] data = dataset(256, 'a', 'z'); + writeDataset(getFileSystem(), path, data, data.length, 1024, false, + useBuilder); + ContractTestUtils.verifyFileContents(getFileSystem(), path, data); + byte[] data2 = dataset(10 * 1024, 'A', 'Z'); + writeDataset(getFileSystem(), path, data2, data2.length, 1024, true, + useBuilder); + ContractTestUtils.verifyFileContents(getFileSystem(), path, data2); + } + /** * This test catches some eventual consistency problems that blobstores exhibit, * as we are implicitly verifying that updates are consistent. This @@ -84,25 +117,21 @@ public abstract class AbstractContractCreateTest extends */ @Test public void testOverwriteExistingFile() throws Throwable { - describe("Overwrite an existing file and verify the new data is there"); - Path path = path("testOverwriteExistingFile"); - byte[] data = dataset(256, 'a', 'z'); - writeDataset(getFileSystem(), path, data, data.length, 1024, false); - ContractTestUtils.verifyFileContents(getFileSystem(), path, data); - byte[] data2 = dataset(10 * 1024, 'A', 'Z'); - writeDataset(getFileSystem(), path, data2, data2.length, 1024, true); - ContractTestUtils.verifyFileContents(getFileSystem(), path, data2); + testOverwriteExistingFile(false); + testOverwriteExistingFile(true); } - @Test - public void testOverwriteEmptyDirectory() throws Throwable { - describe("verify trying to create a file over an empty dir fails"); + private void testOverwriteEmptyDirectory(boolean useBuilder) + throws Throwable { + describe("verify trying to create a file over an empty dir fails, " + + "use builder API=" + useBuilder); Path path = path("testOverwriteEmptyDirectory"); mkdirs(path); assertIsDirectory(path); byte[] data = dataset(256, 'a', 'z'); try { - writeDataset(getFileSystem(), path, data, data.length, 1024, true); + writeDataset(getFileSystem(), path, data, data.length, 1024, true, + useBuilder); assertIsDirectory(path); fail("write of file over empty dir succeeded"); } catch (FileAlreadyExistsException expected) { @@ -121,8 +150,15 @@ public abstract class AbstractContractCreateTest extends } @Test - public void testOverwriteNonEmptyDirectory() throws Throwable { - describe("verify trying to create a file over a non-empty dir fails"); + public void testOverwriteEmptyDirectory() throws Throwable { + testOverwriteEmptyDirectory(false); + testOverwriteEmptyDirectory(true); + } + + private void testOverwriteNonEmptyDirectory(boolean useBuilder) + throws Throwable { + describe("verify trying to create a file over a non-empty dir fails, " + + "use builder API=" + useBuilder); Path path = path("testOverwriteNonEmptyDirectory"); mkdirs(path); try { @@ -140,7 +176,7 @@ public abstract class AbstractContractCreateTest extends byte[] data = dataset(256, 'a', 'z'); try { writeDataset(getFileSystem(), path, data, data.length, 1024, - true); + true, useBuilder); FileStatus status = getFileSystem().getFileStatus(path); boolean isDir = status.isDirectory(); @@ -167,6 +203,12 @@ public abstract class AbstractContractCreateTest extends } @Test + public void testOverwriteNonEmptyDirectory() throws Throwable { + testOverwriteNonEmptyDirectory(false); + testOverwriteNonEmptyDirectory(true); + } + + @Test public void testCreatedFileIsImmediatelyVisible() throws Throwable { describe("verify that a newly created file exists as soon as open returns"); Path path = path("testCreatedFileIsImmediatelyVisible"); http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java index e60fd43..c66dabf 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java @@ -146,16 +146,45 @@ public class ContractTestUtils extends Assert { int len, int buffersize, boolean overwrite) throws IOException { + writeDataset(fs, path, src, len, buffersize, overwrite, false); + } + + /** + * Write a file. + * Optional flags control + * whether file overwrite operations should be enabled + * Optional using {@link org.apache.hadoop.fs.FSDataOutputStreamBuilder} + * + * @param fs filesystem + * @param path path to write to + * @param len length of data + * @param overwrite should the create option allow overwrites? + * @param useBuilder should use builder API to create file? + * @throws IOException IO problems + */ + public static void writeDataset(FileSystem fs, Path path, byte[] src, + int len, int buffersize, boolean overwrite, boolean useBuilder) + throws IOException { assertTrue( "Not enough data in source array to write " + len + " bytes", src.length >= len); - FSDataOutputStream out = fs.create(path, - overwrite, - fs.getConf() - .getInt(IO_FILE_BUFFER_SIZE_KEY, - IO_FILE_BUFFER_SIZE_DEFAULT), - (short) 1, - buffersize); + FSDataOutputStream out; + if (useBuilder) { + out = fs.createFile(path) + .overwrite(overwrite) + .replication((short) 1) + .bufferSize(buffersize) + .blockSize(buffersize) + .build(); + } else { + out = fs.create(path, + overwrite, + fs.getConf() + .getInt(IO_FILE_BUFFER_SIZE_KEY, + IO_FILE_BUFFER_SIZE_DEFAULT), + (short) 1, + buffersize); + } out.write(src, 0, len); out.close(); assertFileHasLength(fs, path, len); http://git-wip-us.apache.org/repos/asf/hadoop/blob/9586b0e2/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java index 34c631a..13c5eb9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java @@ -2892,7 +2892,8 @@ public class DistributedFileSystem extends FileSystem { */ @Override public FSDataOutputStream build() throws IOException { - if (getFlags().contains(CreateFlag.CREATE)) { + if (getFlags().contains(CreateFlag.CREATE) || + getFlags().contains(CreateFlag.OVERWRITE)) { if (isRecursive()) { return dfs.create(getPath(), getPermission(), getFlags(), getBufferSize(), getReplication(), getBlockSize(), --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org