Skip to content

HDDS-12456. Avoid FileInputStream and FileOutputStream #8015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
szetszwo merged 7 commits into from
Mar 6, 2025
Merged

HDDS-12456. Avoid FileInputStream and FileOutputStream #8015

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d1b20df
HDDS-12456. AvoidFileStream
adoroszlai Mar 2, 2025
f694659
Merge remote-tracking branch 'origin/master' into HDDS-12456
adoroszlai Mar 5, 2025
9960ed2
rely on DEFAULT_OPEN_OPTIONS
adoroszlai Mar 5, 2025
1b2e19f
replace useless syncFD
adoroszlai Mar 5, 2025
63d4556
Merge remote-tracking branch 'origin/master' into HDDS-12456
adoroszlai Mar 6, 2025
65313df
use AtomicFileOutputStream
adoroszlai Mar 6, 2025
f9efe62
Merge remote-tracking branch 'origin/master' into HDDS-12456
adoroszlai Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dev-support/pmd/pmd-ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,7 @@
<rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty" />

<rule ref="category/java/performance.xml/AvoidFileStream"/>

<exclude-pattern>.*/generated-sources/.*</exclude-pattern>
</ruleset>
4 changes: 2 additions & 2 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
import static org.apache.commons.collections.EnumerationUtils.toList;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -109,7 +109,7 @@ public NodeSchemaLoadResult loadSchemaFromFile(String schemaFilePath)
if (schemaFile.exists()) {
LOG.info("Load network topology schema file {}",
schemaFile.getAbsolutePath());
try (FileInputStream inputStream = new FileInputStream(schemaFile)) {
try (InputStream inputStream = Files.newInputStream(schemaFile.toPath())) {
return loadSchemaFromStream(schemaFilePath, inputStream);
}
} else {
Expand Down
6 changes: 2 additions & 4 deletions ...rc/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/CertificateCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand Down Expand Up @@ -244,7 +242,7 @@ public synchronized void writeCertificate(Path basePath, String fileName,
File certificateFile =
Paths.get(basePath.toString(), fileName).toFile();

try (FileOutputStream file = new FileOutputStream(certificateFile)) {
try (OutputStream file = Files.newOutputStream(certificateFile.toPath())) {
file.write(pemEncodedCertificate.getBytes(DEFAULT_CHARSET));
}
LOG.info("Save certificate to {}", certificateFile.getAbsolutePath());
Expand All @@ -271,7 +269,7 @@ private CertPath getCertPath(Path path, String fileName) throws IOException,
throw new IOException("Unable to find the requested certificate file. " +
"Path: " + certFile);
}
try (FileInputStream is = new FileInputStream(certFile)) {
try (InputStream is = Files.newInputStream(certFile.toPath())) {
return generateCertPathFromInputStream(is);
}
}
Expand Down
25 changes: 25 additions & 0 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/IOUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,17 @@

package org.apache.hadoop.hdds.utils;

import static java.nio.charset.StandardCharsets.UTF_8;

import jakarta.annotation.Nonnull;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.Properties;
import org.slf4j.Logger;

/**
Expand Down Expand Up @@ -95,4 +104,20 @@ public static void closeQuietly(AutoCloseable... closeables) {
public static void closeQuietly(Collection<? extends AutoCloseable> closeables) {
close(null, closeables);
}

/** Write {@code properties} to the file at {@code path}, truncating any existing content. */
public static void writePropertiesToFile(Path path, Properties properties) throws IOException {
StringWriter out = new StringWriter();
properties.store(out, null);
Files.write(path, out.toString().getBytes(UTF_8));

@szetszwo szetszwo Mar 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous implementation with store(OutputStream, String) was using ISO 8859-1 instead of UTF8. How about keeping it?

    try (OutputStream out = Files.newOutputStream(path, CREATE, TRUNCATE_EXISTING, WRITE)) {
      properties.store(out, null);
    }

@adoroszlai adoroszlai Mar 6, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @szetszwo for the review.

always first write to a tmp file and then rename

Then I'll change it to:

    try (OutputStream out = new AtomicFileOutputStream(file)) {
      properties.store(out, null);
    }

using AtomicFileOutputStream from Ratis.

}

/** Read {@link Properties} from the file at {@code path}. */
public static @Nonnull Properties readPropertiesFromFile(Path path) throws IOException {
Properties props = new Properties();
try (InputStream in = Files.newInputStream(path)) {
props.load(in);
}
return props;
}
}
31 changes: 3 additions & 28 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/StorageInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@

import com.google.common.base.Preconditions;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.util.Properties;
import java.util.UUID;
import org.apache.hadoop.hdds.annotation.InterfaceAudience;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.ozone.OzoneConsts;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -192,34 +190,11 @@ private void verifyCreationTime() {

public void writeTo(File to)
throws IOException {
try (RandomAccessFile file = new RandomAccessFile(to, "rws");
FileOutputStream out = new FileOutputStream(file.getFD())) {
file.seek(0);
/*
* If server is interrupted before this line,
* the version file will remain unchanged.
*/
properties.store(out, null);
/*
* Now the new fields are flushed to the head of the file, but file
* length can still be larger then required and therefore the file can
* contain whole or corrupted fields from its old contents in the end.
* If server is interrupted here and restarted later these extra fields
* either should not effect server behavior or should be handled
* by the server correctly.
*/
file.setLength(out.getChannel().position());
}
IOUtils.writePropertiesToFile(to.toPath(), properties);
}

private Properties readFrom(File from) throws IOException {
try (RandomAccessFile file = new RandomAccessFile(from, "rws");
FileInputStream in = new FileInputStream(file.getFD())) {
Properties props = new Properties();
file.seek(0);
props.load(in);
return props;
}
return IOUtils.readPropertiesFromFile(from.toPath());
}

/**
Expand Down
9 changes: 5 additions & 4 deletions hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.time.Duration;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
Expand Down Expand Up @@ -83,7 +84,7 @@ public void testGetAllPropertiesByTags(@TempDir File tempDir)
throws Exception {
File coreDefault = new File(tempDir, "core-default-test.xml");
File coreSite = new File(tempDir, "core-site-test.xml");
FileOutputStream coreDefaultStream = new FileOutputStream(coreDefault);
OutputStream coreDefaultStream = Files.newOutputStream(coreDefault.toPath());
try (BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
coreDefaultStream, StandardCharsets.UTF_8))) {
startConfig(out);
Expand All @@ -102,7 +103,7 @@ public void testGetAllPropertiesByTags(@TempDir File tempDir)
.getProperty("dfs.random.key"));
}

FileOutputStream coreSiteStream = new FileOutputStream(coreSite);
OutputStream coreSiteStream = Files.newOutputStream(coreSite.toPath());
try (BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
coreSiteStream, StandardCharsets.UTF_8))) {
startConfig(out);
Expand Down Expand Up @@ -286,7 +287,7 @@ public void testInstantiationWithInputConfiguration(@TempDir File tempDir)
Configuration configuration = new Configuration(true);

File ozoneSite = new File(tempDir, "ozone-site.xml");
FileOutputStream ozoneSiteStream = new FileOutputStream(ozoneSite);
OutputStream ozoneSiteStream = Files.newOutputStream(ozoneSite.toPath());
try (BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
ozoneSiteStream, StandardCharsets.UTF_8))) {
startConfig(out);
Expand Down
5 changes: 3 additions & 2 deletions ...ervice/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@

import com.google.common.base.Preconditions;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -177,7 +178,7 @@ public static synchronized DatanodeDetails readDatanodeDetailsFrom(File path)
LOG.warn("Error loading DatanodeDetails yaml from {}",
path.getAbsolutePath(), e);
// Try to load as protobuf before giving up
try (FileInputStream in = new FileInputStream(path)) {
try (InputStream in = Files.newInputStream(path.toPath())) {
return DatanodeDetails.getFromProtoBuf(
HddsProtos.DatanodeDetailsProto.parseFrom(in));
} catch (IOException io) {
Expand Down
8 changes: 4 additions & 4 deletions ...ervice/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
package org.apache.hadoop.ozone.container.common.helpers;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -72,7 +72,7 @@ public static void createDatanodeIdFile(DatanodeDetails datanodeDetails,
Yaml yaml = new Yaml(options);

try (Writer writer = new OutputStreamWriter(
new FileOutputStream(path), StandardCharsets.UTF_8)) {
Files.newOutputStream(path.toPath()), StandardCharsets.UTF_8)) {
yaml.dump(getDatanodeDetailsYaml(datanodeDetails, conf), writer);
}
}
Expand All @@ -83,7 +83,7 @@ public static void createDatanodeIdFile(DatanodeDetails datanodeDetails,
public static DatanodeDetails readDatanodeIdFile(File path)
throws IOException {
DatanodeDetails datanodeDetails;
try (FileInputStream inputFileStream = new FileInputStream(path)) {
try (InputStream inputFileStream = Files.newInputStream(path.toPath())) {
DatanodeDetailsYaml datanodeDetailsYaml;
try {
datanodeDetailsYaml =
Expand Down
23 changes: 3 additions & 20 deletions ...e/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeVersionFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@
package org.apache.hadoop.ozone.container.common.helpers;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.util.Properties;
import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.ozone.OzoneConsts;

/**
Expand Down Expand Up @@ -64,31 +62,16 @@ private Properties createProperties() {
*/
public void createVersionFile(File path) throws
IOException {
try (RandomAccessFile file = new RandomAccessFile(path, "rws");
FileOutputStream out = new FileOutputStream(file.getFD())) {
file.getChannel().truncate(0);
Properties properties = createProperties();
/*
* If server is interrupted before this line,
* the version file will remain unchanged.
*/
properties.store(out, null);
}
IOUtils.writePropertiesToFile(path.toPath(), createProperties());
}


/**
* Creates a property object from the specified file content.
* @param versionFile
* @return Properties
* @throws IOException
*/
public static Properties readFrom(File versionFile) throws IOException {
try (RandomAccessFile file = new RandomAccessFile(versionFile, "rws");
FileInputStream in = new FileInputStream(file.getFD())) {
Properties props = new Properties();
props.load(in);
return props;
}
return IOUtils.readPropertiesFromFile(versionFile.toPath());
}
}
19 changes: 10 additions & 9 deletions ...ervice/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerDataYaml.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,34 @@

package org.apache.hadoop.ozone.container.common.impl;

import static java.nio.file.StandardOpenOption.CREATE;
import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;
import static java.nio.file.StandardOpenOption.WRITE;
import static org.apache.hadoop.ozone.OzoneConsts.REPLICA_INDEX;
import static org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData.KEYVALUE_YAML_TAG;

import com.google.common.base.Preconditions;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import org.apache.commons.io.IOUtils;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerType;
import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
import org.apache.ratis.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.DumperOptions;
Expand Down Expand Up @@ -81,7 +85,7 @@ private ContainerDataYaml() {
public static void createContainerFile(ContainerType containerType,
ContainerData containerData, File containerFile) throws IOException {
Writer writer = null;
FileOutputStream out = null;
OutputStream out = null;
try {
boolean withReplicaIndex =
containerData instanceof KeyValueContainerData &&
Expand All @@ -93,16 +97,13 @@ public static void createContainerFile(ContainerType containerType,
containerData.computeAndSetChecksum(yaml);

// Write the ContainerData with checksum to Yaml file.
out = new FileOutputStream(
containerFile);
out = FileUtils.newOutputStreamForceAtClose(containerFile, CREATE, TRUNCATE_EXISTING, WRITE);
writer = new OutputStreamWriter(out, StandardCharsets.UTF_8);
yaml.dump(containerData, writer);
} finally {
try {
if (writer != null) {
writer.flush();
// make sure the container metadata is synced to disk.
out.getFD().sync();
writer.close();
}
} catch (IOException ex) {
Expand All @@ -121,7 +122,7 @@ public static void createContainerFile(ContainerType containerType,
public static ContainerData readContainerFile(File containerFile)
throws IOException {
Preconditions.checkNotNull(containerFile, "containerFile cannot be null");
try (FileInputStream inputFileStream = new FileInputStream(containerFile)) {
try (InputStream inputFileStream = Files.newInputStream(containerFile.toPath())) {
return readContainer(inputFileStream);
}

Expand Down
Loading