This is an automated email from the ASF dual-hosted git repository. omalley pushed a commit to branch branch-1.5 in repository https://gitbox.apache.org/repos/asf/orc.git
View the commit online: https://github.com/apache/orc/commit/27d716f59acde838f9e659df1742218ff60c085a The following commit(s) were added to refs/heads/branch-1.5 by this push: new 27d716f ORC-570: Supply lazy filesystem to ReaderOptions & DataReaderProperties 27d716f is described below commit 27d716f59acde838f9e659df1742218ff60c085a Author: Mustafa Iman <[email protected]> AuthorDate: Fri Nov 15 16:11:39 2019 -0800 ORC-570: Supply lazy filesystem to ReaderOptions & DataReaderProperties Although these carry filesystem, it may not be needed at all if a file is not actually open (due to caching). Creation of an actual filesystem object can be deferred until a file is actually opened. Fixes #453 Change-Id: I1e2cad8b934a14fd0413caae68cf20417213d2b8 Signed-off-by: Owen O'Malley <[email protected]> --- .../org/apache/orc/impl/DataReaderProperties.java | 25 +++++++----- .../src/java/org/apache/orc/impl/ReaderImpl.java | 11 ++++++ .../java/org/apache/orc/impl/RecordReaderImpl.java | 2 +- .../org/apache/orc/impl/RecordReaderUtils.java | 8 ++-- .../apache/orc/impl/TestDataReaderProperties.java | 44 +++++++++++++++++----- java/pom.xml | 1 + 6 files changed, 68 insertions(+), 23 deletions(-) diff --git a/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java b/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java index 386324c..5f7e8fc 100644 --- a/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java +++ b/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java @@ -23,9 +23,11 @@ import org.apache.hadoop.fs.Path; import org.apache.orc.CompressionKind; import org.apache.orc.OrcConf; +import java.util.function.Supplier; + public final class DataReaderProperties { - private final FileSystem fileSystem; + private final Supplier<FileSystem> fileSystemSupplier; private final Path path; private final FSDataInputStream file; private final CompressionKind compression; @@ -35,7 +37,7 @@ public final class DataReaderProperties { private final int maxDiskRangeChunkLimit; private DataReaderProperties(Builder builder) { - this.fileSystem = builder.fileSystem; + this.fileSystemSupplier = builder.fileSystemSupplier; this.path = builder.path; this.file = builder.file; this.compression = builder.compression; @@ -45,8 +47,8 @@ public final class DataReaderProperties { this.maxDiskRangeChunkLimit = builder.maxDiskRangeChunkLimit; } - public FileSystem getFileSystem() { - return fileSystem; + public Supplier<FileSystem> getFileSystemSupplier() { + return fileSystemSupplier; } public Path getPath() { @@ -83,7 +85,7 @@ public final class DataReaderProperties { public static class Builder { - private FileSystem fileSystem; + private Supplier<FileSystem> fileSystemSupplier; private Path path; private FSDataInputStream file; private CompressionKind compression; @@ -96,8 +98,13 @@ public final class DataReaderProperties { } - public Builder withFileSystem(FileSystem fileSystem) { - this.fileSystem = fileSystem; + public Builder withFileSystemSupplier(Supplier<FileSystem> supplier) { + this.fileSystemSupplier = supplier; + return this; + } + + public Builder withFileSystem(FileSystem filesystem) { + this.fileSystemSupplier = () -> filesystem; return this; } @@ -137,8 +144,8 @@ public final class DataReaderProperties { } public DataReaderProperties build() { - if (fileSystem == null || path == null) { - throw new NullPointerException("Filesystem = " + fileSystem + + if (fileSystemSupplier == null || path == null) { + throw new NullPointerException("Filesystem = " + fileSystemSupplier + ", path = " + path); } diff --git a/java/core/src/java/org/apache/orc/impl/ReaderImpl.java b/java/core/src/java/org/apache/orc/impl/ReaderImpl.java index 01b449b..09cfe82 100644 --- a/java/core/src/java/org/apache/orc/impl/ReaderImpl.java +++ b/java/core/src/java/org/apache/orc/impl/ReaderImpl.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Supplier; import org.apache.hadoop.fs.FileStatus; import org.apache.orc.CompressionKind; @@ -398,6 +399,16 @@ public class ReaderImpl implements Reader { return this.fileSystem; } + protected Supplier<FileSystem> getFileSystemSupplier() { + return () -> { + try { + return getFileSystem(); + } catch (IOException e) { + throw new RuntimeException("Can't create filesystem", e); + } + }; + } + /** * Get the WriterVersion based on the ORC file postscript. * @param writerVersion the integer writer version diff --git a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java index 22ff8c9..84e1b93 100644 --- a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java +++ b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java @@ -245,7 +245,7 @@ public class RecordReaderImpl implements RecordReader { DataReaderProperties.builder() .withBufferSize(bufferSize) .withCompression(fileReader.compressionKind) - .withFileSystem(fileReader.getFileSystem()) + .withFileSystemSupplier(fileReader.getFileSystemSupplier()) .withPath(fileReader.path) .withTypeCount(types.size()) .withMaxDiskRangeChunkLimit(maxDiskRangeChunkLimit) diff --git a/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java b/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java index 36a11db..e2d4459 100644 --- a/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java +++ b/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java @@ -27,6 +27,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.function.Supplier; + import org.apache.commons.lang.builder.HashCodeBuilder; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileSystem; @@ -147,7 +149,7 @@ public class RecordReaderUtils { private FSDataInputStream file; private ByteBufferAllocatorPool pool; private HadoopShims.ZeroCopyReaderShim zcr = null; - private final FileSystem fs; + private final Supplier<FileSystem> fileSystemSupplier; private final Path path; private final boolean useZeroCopy; private CompressionCodec codec; @@ -158,7 +160,7 @@ public class RecordReaderUtils { private boolean isOpen = false; private DefaultDataReader(DataReaderProperties properties) { - this.fs = properties.getFileSystem(); + this.fileSystemSupplier = properties.getFileSystemSupplier(); this.path = properties.getPath(); this.file = properties.getFile(); this.useZeroCopy = properties.getZeroCopy(); @@ -172,7 +174,7 @@ public class RecordReaderUtils { @Override public void open() throws IOException { if (file == null) { - this.file = fs.open(path); + this.file = fileSystemSupplier.get().open(path); } if (useZeroCopy) { // ZCR only uses codec for boolean checks. diff --git a/java/core/src/test/org/apache/orc/impl/TestDataReaderProperties.java b/java/core/src/test/org/apache/orc/impl/TestDataReaderProperties.java index 46546b0..f9a3f29 100644 --- a/java/core/src/test/org/apache/orc/impl/TestDataReaderProperties.java +++ b/java/core/src/test/org/apache/orc/impl/TestDataReaderProperties.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -19,10 +19,12 @@ package org.apache.orc.impl; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.orc.CompressionCodec; import org.apache.orc.CompressionKind; import org.junit.Test; +import java.io.IOException; +import java.util.function.Supplier; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -30,31 +32,53 @@ import static org.mockito.Mockito.mock; public class TestDataReaderProperties { - private FileSystem mockedFileSystem = mock(FileSystem.class); + private Supplier<FileSystem> mockedSupplier = mock(Supplier.class); private Path mockedPath = mock(Path.class); private boolean mockedZeroCopy = false; @Test - public void testCompleteBuild() { + public void testCompleteBuild() throws IOException { DataReaderProperties properties = DataReaderProperties.builder() - .withFileSystem(mockedFileSystem) + .withFileSystemSupplier(mockedSupplier) .withPath(mockedPath) .withCompression(CompressionKind.ZLIB) .withZeroCopy(mockedZeroCopy) .build(); - assertEquals(mockedFileSystem, properties.getFileSystem()); + assertEquals(mockedSupplier, properties.getFileSystemSupplier()); assertEquals(mockedPath, properties.getPath()); assertEquals(CompressionKind.ZLIB, properties.getCompression()); assertEquals(mockedZeroCopy, properties.getZeroCopy()); } @Test - public void testMissingNonRequiredArgs() { + public void testFileSystemSupplier() throws IOException { + + DataReaderProperties properties = DataReaderProperties.builder() + .withFileSystemSupplier(mockedSupplier) + .withPath(mockedPath) + .build(); + + assertEquals(mockedSupplier, properties.getFileSystemSupplier()); + } + + @Test + public void testWhenFilesystemIsProvidedGetFileSystemSupplierReturnsSupplier() throws IOException { + DataReaderProperties properties = DataReaderProperties.builder() + .withFileSystemSupplier(mockedSupplier) + .withPath(mockedPath) + .build(); + + Supplier<FileSystem> supplierFromProperties = properties.getFileSystemSupplier(); + assertEquals(mockedSupplier, supplierFromProperties); + } + + @Test + public void testMissingNonRequiredArgs() throws IOException { DataReaderProperties properties = DataReaderProperties.builder() - .withFileSystem(mockedFileSystem) + .withFileSystemSupplier(mockedSupplier) .withPath(mockedPath) .build(); - assertEquals(mockedFileSystem, properties.getFileSystem()); + assertEquals(mockedSupplier, properties.getFileSystemSupplier()); assertEquals(mockedPath, properties.getPath()); assertNull(properties.getCompression()); assertFalse(properties.getZeroCopy()); @@ -68,7 +92,7 @@ public class TestDataReaderProperties { @Test(expected = java.lang.NullPointerException.class) public void testMissingPath() { DataReaderProperties.builder() - .withFileSystem(mockedFileSystem) + .withFileSystemSupplier(mockedSupplier) .withCompression(CompressionKind.NONE) .withZeroCopy(mockedZeroCopy) .build(); diff --git a/java/pom.xml b/java/pom.xml index 88e62db..aa0e373 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -208,6 +208,7 @@ <excludeSubProject>false</excludeSubProject> <excludes> <exclude>**/data/**</exclude> + <exclude>**/*.iml</exclude> <exclude>**/*.json</exclude> <exclude>**/*.json.gz</exclude> <exclude>**/*.md</exclude>
