This is an automated email from the ASF dual-hosted git repository. omalley pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/orc.git
View the commit online: https://github.com/apache/orc/commit/49f79dc97dda4261f25fc22fdba0ad5148c190e0 The following commit(s) were added to refs/heads/master by this push: new 49f79dc ORC-570: Supply lazy filesystem to ReaderOptions & DataReaderProperties 49f79dc is described below commit 49f79dc97dda4261f25fc22fdba0ad5148c190e0 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 +++++++++++++++++----- 5 files changed, 67 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 763c001..2abb901 100644 --- a/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java +++ b/java/core/src/java/org/apache/orc/impl/DataReaderProperties.java @@ -21,9 +21,11 @@ import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +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 InStream.StreamOptions compression; @@ -31,7 +33,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; @@ -39,8 +41,8 @@ public final class DataReaderProperties { this.maxDiskRangeChunkLimit = builder.maxDiskRangeChunkLimit; } - public FileSystem getFileSystem() { - return fileSystem; + public Supplier<FileSystem> getFileSystemSupplier() { + return fileSystemSupplier; } public Path getPath() { @@ -69,7 +71,7 @@ public final class DataReaderProperties { public static class Builder { - private FileSystem fileSystem; + private Supplier<FileSystem> fileSystemSupplier; private Path path; private FSDataInputStream file; private InStream.StreamOptions compression; @@ -80,8 +82,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; } @@ -111,8 +118,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 645f23f..a4c73c7 100644 --- a/java/core/src/java/org/apache/orc/impl/ReaderImpl.java +++ b/java/core/src/java/org/apache/orc/impl/ReaderImpl.java @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.security.Key; import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; import org.apache.orc.EncryptionAlgorithm; import org.apache.orc.EncryptionKey; @@ -530,6 +531,16 @@ public class ReaderImpl implements Reader { return 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 c4ce398..de55c85 100644 --- a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java +++ b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java @@ -199,7 +199,7 @@ public class RecordReaderImpl implements RecordReader { DataReaderProperties.Builder builder = DataReaderProperties.builder() .withCompression(unencryptedOptions) - .withFileSystem(fileReader.getFileSystem()) + .withFileSystemSupplier(fileReader.getFileSystemSupplier()) .withPath(fileReader.path) .withMaxDiskRangeChunkLimit(maxDiskRangeChunkLimit) .withZeroCopy(zeroCopy); 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 159b63e..850de9e 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.Iterator; 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; @@ -49,14 +51,14 @@ 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 InStream.StreamOptions options; 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(); @@ -66,7 +68,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 8fcf98d..a0ee057 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,21 +32,21 @@ 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 { InStream.StreamOptions options = InStream.options() .withCodec(OrcCodecPool.getCodec(CompressionKind.ZLIB)); DataReaderProperties properties = DataReaderProperties.builder() - .withFileSystem(mockedFileSystem) + .withFileSystemSupplier(mockedSupplier) .withPath(mockedPath) .withCompression(options) .withZeroCopy(mockedZeroCopy) .build(); - assertEquals(mockedFileSystem, properties.getFileSystem()); + assertEquals(mockedSupplier, properties.getFileSystemSupplier()); assertEquals(mockedPath, properties.getPath()); assertEquals(CompressionKind.ZLIB, properties.getCompression().getCodec().getKind()); @@ -52,12 +54,34 @@ public class TestDataReaderProperties { } @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()); @@ -71,7 +95,7 @@ public class TestDataReaderProperties { @Test(expected = java.lang.NullPointerException.class) public void testMissingPath() { DataReaderProperties.builder() - .withFileSystem(mockedFileSystem) + .withFileSystemSupplier(mockedSupplier) .withCompression(InStream.options()) .withZeroCopy(mockedZeroCopy) .build();
