diff --git a/.mvn/modernizer/violations.xml b/.mvn/modernizer/violations.xml index 6016f7e8ab6c..ebe585ac7b3e 100644 --- a/.mvn/modernizer/violations.xml +++ b/.mvn/modernizer/violations.xml @@ -158,6 +158,12 @@ Prefer ConfigurationInstantiator.newEmptyConfiguration() for two reasons: (1) loading default resources is unlikely desired and (2) ConfigurationInstantiator adds additional safety checks + + org/apache/hadoop/fs/FileSystem.close:()V + 1.1 + Hadoop FileSystem instances are shared and should not be closed + + org/apache/hadoop/conf/Configuration."<init>":(Z)V 1.1 diff --git a/lib/trino-hdfs/pom.xml b/lib/trino-hdfs/pom.xml index b8d13640657a..7220b1dbe67d 100644 --- a/lib/trino-hdfs/pom.xml +++ b/lib/trino-hdfs/pom.xml @@ -88,6 +88,11 @@ validation-api + + org.gaul + modernizer-maven-annotations + + org.weakref jmxutils diff --git a/lib/trino-hdfs/src/main/java/io/trino/hdfs/TrinoFileSystemCache.java b/lib/trino-hdfs/src/main/java/io/trino/hdfs/TrinoFileSystemCache.java index 5fceb8cd5c86..7a493236ad2c 100644 --- a/lib/trino-hdfs/src/main/java/io/trino/hdfs/TrinoFileSystemCache.java +++ b/lib/trino-hdfs/src/main/java/io/trino/hdfs/TrinoFileSystemCache.java @@ -34,6 +34,7 @@ import org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod; import org.apache.hadoop.util.Progressable; import org.apache.hadoop.util.ReflectionUtils; +import org.gaul.modernizer_maven_annotations.SuppressModernizer; import javax.annotation.concurrent.GuardedBy; @@ -167,7 +168,7 @@ private static FileSystem createFileSystem(URI uri, Configuration conf) FilterFileSystem wrapper = new FileSystemWrapper(original); FileSystemFinalizerService.getInstance().addFinalizer(wrapper, () -> { try { - original.close(); + closeFileSystem(original); } catch (IOException e) { log.error(e, "Error occurred when finalizing file system"); @@ -188,11 +189,18 @@ public synchronized void closeAll() throws IOException { for (FileSystemHolder fileSystemHolder : ImmutableList.copyOf(map.values())) { - fileSystemHolder.getFileSystem().close(); + closeFileSystem(fileSystemHolder.getFileSystem()); } map.clear(); } + @SuppressModernizer + private static void closeFileSystem(FileSystem fileSystem) + throws IOException + { + fileSystem.close(); + } + private static FileSystemKey createFileSystemKey(URI uri, UserGroupInformation userGroupInformation, long unique) { String scheme = nullToEmpty(uri.getScheme()).toLowerCase(ENGLISH); diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java index a19cf769cc83..24276b906846 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java @@ -640,10 +640,9 @@ public void dropSchema(ConnectorSession session, String schemaName) // If we see files in the schema location, don't delete it. // If we see no files or can't see the location at all, use fallback. boolean deleteData = location.map(path -> { - HdfsContext context = new HdfsContext(session); // don't catch errors here - - try (FileSystem fs = hdfsEnvironment.getFileSystem(context, path)) { - return !fs.listLocatedStatus(path).hasNext(); + try { + return !hdfsEnvironment.getFileSystem(new HdfsContext(session), path) + .listLocatedStatus(path).hasNext(); } catch (IOException | RuntimeException e) { LOG.warn(e, "Could not check schema directory '%s'", path); diff --git a/plugin/trino-hive/pom.xml b/plugin/trino-hive/pom.xml index 8f3c17dd1f98..4f6f707c4a4c 100644 --- a/plugin/trino-hive/pom.xml +++ b/plugin/trino-hive/pom.xml @@ -274,6 +274,11 @@ libthrift + + org.gaul + modernizer-maven-annotations + + org.weakref jmxutils diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java index da27d90380f4..63c7bf450d0f 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java @@ -438,9 +438,9 @@ public synchronized void dropDatabase(ConnectorSession session, String schemaNam // If we see no files, request deletion. // If we fail to check the schema location, behave according to fallback. boolean deleteData = location.map(path -> { - HdfsContext context = new HdfsContext(session); - try (FileSystem fs = hdfsEnvironment.getFileSystem(context, path)) { - return !fs.listLocatedStatus(path).hasNext(); + try { + return !hdfsEnvironment.getFileSystem(new HdfsContext(session), path) + .listLocatedStatus(path).hasNext(); } catch (IOException | RuntimeException e) { log.warn(e, "Could not check schema directory '%s'", path); diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java index eeb83d5b7852..3fca3f28ad02 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java @@ -92,6 +92,7 @@ import org.apache.hadoop.fs.RemoteIterator; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.util.Progressable; +import org.gaul.modernizer_maven_annotations.SuppressModernizer; import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; @@ -341,7 +342,7 @@ public void close() throws IOException { try (Closer closer = Closer.create()) { - closer.register(super::close); + closer.register(this::closeSuper); if (credentialsProvider instanceof Closeable) { closer.register((Closeable) credentialsProvider); } @@ -350,6 +351,13 @@ public void close() } } + @SuppressModernizer + private void closeSuper() + throws IOException + { + super.close(); + } + @Override public URI getUri() { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/rubix/TestRubixCaching.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/rubix/TestRubixCaching.java index 211505b85814..4e0e1778df68 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/rubix/TestRubixCaching.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/rubix/TestRubixCaching.java @@ -50,6 +50,7 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FilterFileSystem; import org.apache.hadoop.fs.Path; +import org.gaul.modernizer_maven_annotations.SuppressModernizer; import org.testng.annotations.AfterClass; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; @@ -235,7 +236,7 @@ private FileSystem getCachingFileSystem(HdfsContext context, Path path) public void tearDown() throws IOException { - nonCachingFileSystem.close(); + closeFileSystem(nonCachingFileSystem); } @AfterMethod(alwaysRun = true) @@ -251,7 +252,7 @@ public void closeRubix() }); closer.register(() -> { if (cachingFileSystem != null) { - cachingFileSystem.close(); + closeFileSystem(cachingFileSystem); cachingFileSystem = null; } }); @@ -274,6 +275,13 @@ public void closeRubix() } } + @SuppressModernizer + private static void closeFileSystem(FileSystem fileSystem) + throws IOException + { + fileSystem.close(); + } + @DataProvider public static Object[][] readMode() { @@ -469,6 +477,7 @@ public void testLargeFile(ReadMode readMode) }); } + @SuppressModernizer @Test public void testFileSystemBindings() throws Exception diff --git a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileRewriter.java b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileRewriter.java index bae8ee455c96..cc9891f85837 100644 --- a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileRewriter.java +++ b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileRewriter.java @@ -62,38 +62,37 @@ private OrcFileRewriter() {} public static OrcFileInfo rewrite(File input, File output, BitSet rowsToDelete) throws IOException { - try (FileSystem fileSystem = new SyncingFileSystem(CONFIGURATION)) { - Reader reader = createReader(fileSystem, path(input)); + FileSystem fileSystem = new SyncingFileSystem(CONFIGURATION); + Reader reader = createReader(fileSystem, path(input)); - if (reader.getNumberOfRows() < rowsToDelete.length()) { - throw new IOException("File has fewer rows than deletion vector"); - } - int deleteRowCount = rowsToDelete.cardinality(); - if (reader.getNumberOfRows() == deleteRowCount) { - return new OrcFileInfo(0, 0); - } - if (reader.getNumberOfRows() >= Integer.MAX_VALUE) { - throw new IOException("File has too many rows"); - } - int inputRowCount = toIntExact(reader.getNumberOfRows()); - - WriterOptions writerOptions = OrcFile.writerOptions(CONFIGURATION) - .memory(new NullMemoryManager()) - .fileSystem(fileSystem) - .compress(reader.getCompression()) - .inspector(reader.getObjectInspector()); - - long start = System.nanoTime(); - try (Closer recordReader = closer(reader.rows(), RecordReader::close); - Closer writer = closer(createWriter(path(output), writerOptions), Writer::close)) { - if (reader.hasMetadataValue(OrcFileMetadata.KEY)) { - ByteBuffer orcFileMetadata = reader.getMetadataValue(OrcFileMetadata.KEY); - writer.get().addUserMetadata(OrcFileMetadata.KEY, orcFileMetadata); - } - OrcFileInfo fileInfo = rewrite(recordReader.get(), writer.get(), rowsToDelete, inputRowCount); - log.debug("Rewrote file %s in %s (input rows: %s, output rows: %s)", input.getName(), nanosSince(start), inputRowCount, inputRowCount - deleteRowCount); - return fileInfo; + if (reader.getNumberOfRows() < rowsToDelete.length()) { + throw new IOException("File has fewer rows than deletion vector"); + } + int deleteRowCount = rowsToDelete.cardinality(); + if (reader.getNumberOfRows() == deleteRowCount) { + return new OrcFileInfo(0, 0); + } + if (reader.getNumberOfRows() >= Integer.MAX_VALUE) { + throw new IOException("File has too many rows"); + } + int inputRowCount = toIntExact(reader.getNumberOfRows()); + + WriterOptions writerOptions = OrcFile.writerOptions(CONFIGURATION) + .memory(new NullMemoryManager()) + .fileSystem(fileSystem) + .compress(reader.getCompression()) + .inspector(reader.getObjectInspector()); + + long start = System.nanoTime(); + try (Closer recordReader = closer(reader.rows(), RecordReader::close); + Closer writer = closer(createWriter(path(output), writerOptions), Writer::close)) { + if (reader.hasMetadataValue(OrcFileMetadata.KEY)) { + ByteBuffer orcFileMetadata = reader.getMetadataValue(OrcFileMetadata.KEY); + writer.get().addUserMetadata(OrcFileMetadata.KEY, orcFileMetadata); } + OrcFileInfo fileInfo = rewrite(recordReader.get(), writer.get(), rowsToDelete, inputRowCount); + log.debug("Rewrote file %s in %s (input rows: %s, output rows: %s)", input.getName(), nanosSince(start), inputRowCount, inputRowCount - deleteRowCount); + return fileInfo; } } diff --git a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileWriter.java b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileWriter.java index 45fda7448415..22d7c83dccf9 100644 --- a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileWriter.java +++ b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/storage/OrcFileWriter.java @@ -40,7 +40,6 @@ import io.trino.spi.type.VarbinaryType; import io.trino.spi.type.VarcharType; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.ql.io.IOConstants; import org.apache.hadoop.hive.ql.io.orc.OrcFile; @@ -217,10 +216,10 @@ private static OrcSerde createSerializer(Properties properties) private static RecordWriter createRecordWriter(Path target, List columnIds, List columnTypes, boolean writeMetadata) { - try (FileSystem fileSystem = new SyncingFileSystem(CONFIGURATION)) { + try { OrcFile.WriterOptions options = OrcFile.writerOptions(CONFIGURATION) .memory(new NullMemoryManager()) - .fileSystem(fileSystem) + .fileSystem(new SyncingFileSystem(CONFIGURATION)) .compress(SNAPPY); if (writeMetadata) {