Skip to content

Commit

Permalink
RATIS-1900. Do not use FileInputStream/FileOutputStream.
Browse files Browse the repository at this point in the history
  • Loading branch information
szetszwo committed Oct 25, 2023
1 parent 3e7f9e5 commit c4f04c1
Show file tree
Hide file tree
Showing 17 changed files with 136 additions and 117 deletions.
30 changes: 30 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
<build-helper-maven-plugin.version>3.3.0</build-helper-maven-plugin.version>
<exec-maven-plugin.version>3.1.0</exec-maven-plugin.version>
<extra-enforcer-rules.version>1.6.1</extra-enforcer-rules.version>
<restrict-imports-enforcer-rules.version>2.4.0</restrict-imports-enforcer-rules.version>
<license-maven-plugin.version>2.2.0</license-maven-plugin.version>

<protobuf-maven-plugin.version>0.6.1</protobuf-maven-plugin.version>
Expand Down Expand Up @@ -529,7 +530,36 @@
</requireJavaVersion>
</rules>
</configuration>
<dependencies>
<dependency>
<groupId>de.skuzzle.enforcer</groupId>
<artifactId>restrict-imports-enforcer-rule</artifactId>
<version>${restrict-imports-enforcer-rules.version}</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>bannedImports-finalizers</id>
<phase>process-sources</phase>
<goals>
<goal>enforce</goal>
</goals>
<configuration>
<rules>
<restrictImports>
<includeTestCode>false</includeTestCode>
<reason>Classes (e.g. FileInputStream, FileOutputStream) overriding the Object.finalize() method will cause gc to run a much longer time. As stated in Effective Java, there is a severe performance penalty for using finalizers.</reason>
<bannedImports>
<bannedImport>java.io.FileInputStream</bannedImport>
<bannedImport>java.io.FileOutputStream</bannedImport>
</bannedImports>
</restrictImports>
</rules>
</configuration>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.apache.ratis.thirdparty.com.google.common.cache.Cache;
import org.apache.ratis.thirdparty.com.google.common.cache.CacheBuilder;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteStringUtils;
import org.apache.ratis.thirdparty.com.google.protobuf.UnsafeByteOperations;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.Preconditions;

Expand Down Expand Up @@ -49,7 +49,7 @@ static ByteString toByteString(UUID uuid) {
ByteBuffer.wrap(array)
.putLong(uuid.getMostSignificantBits())
.putLong(uuid.getLeastSignificantBits());
return ByteStringUtils.unsafeWrap(array);
return UnsafeByteOperations.unsafeWrap(array);
}

abstract static class Factory<ID extends RaftId> {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@
import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage;
import org.apache.ratis.statemachine.impl.SingleFileSnapshotInfo;
import org.apache.ratis.util.AutoCloseableLock;
import org.apache.ratis.util.FileUtils;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.MD5FileUtil;

import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
Expand Down Expand Up @@ -97,8 +96,8 @@ public long takeSnapshot() {
final File snapshotFile = storage.getSnapshotFile(last.getTerm(), last.getIndex());
LOG.info("Taking a snapshot to file {}", snapshotFile);

try(ObjectOutputStream out = new ObjectOutputStream(
new BufferedOutputStream(new FileOutputStream(snapshotFile)))) {
try(ObjectOutputStream out = new ObjectOutputStream(new BufferedOutputStream(
FileUtils.newOutputStream(snapshotFile)))) {
out.writeObject(copy);
} catch(IOException ioe) {
LOG.warn("Failed to write snapshot file \"" + snapshotFile
Expand Down Expand Up @@ -130,8 +129,8 @@ public long loadSnapshot(SingleFileSnapshotInfo snapshot) throws IOException {

final TermIndex last = SimpleStateMachineStorage.getTermIndexFromSnapshotFile(snapshotFile);
try(AutoCloseableLock writeLock = writeLock();
ObjectInputStream in = new ObjectInputStream(
new BufferedInputStream(new FileInputStream(snapshotFile)))) {
ObjectInputStream in = new ObjectInputStream(new BufferedInputStream(
FileUtils.newInputStream(snapshotFile)))) {
reset();
setLastAppliedTermIndex(last);
variables.putAll(JavaUtils.cast(in.readObject()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@
import org.apache.ratis.protocol.RaftGroup;
import org.apache.ratis.protocol.RaftGroupId;
import org.apache.ratis.protocol.RaftPeer;
import org.apache.ratis.util.FileUtils;
import org.apache.ratis.util.TimeDuration;

import java.io.BufferedReader;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -52,10 +50,9 @@ public final class Constants {
static {
final Properties properties = new Properties();
final String conf = "ratis-examples/src/main/resources/conf.properties";
try(InputStream inputStream = new FileInputStream(conf);
Reader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8);
BufferedReader bufferedReader = new BufferedReader(reader)) {
properties.load(bufferedReader);
try(BufferedReader in = new BufferedReader(new InputStreamReader(
FileUtils.newInputStream(conf), StandardCharsets.UTF_8))) {
properties.load(in);
} catch (IOException e) {
throw new IllegalStateException("Failed to load " + conf, e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -26,14 +26,15 @@
import org.apache.ratis.protocol.RoutingTable;
import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
import org.apache.ratis.thirdparty.io.netty.buffer.PooledByteBufAllocator;
import org.apache.ratis.util.FileUtils;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.Preconditions;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.MappedByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -238,8 +239,7 @@ List<CompletableFuture<DataStreamReply>> transfer(

final List<CompletableFuture<DataStreamReply>> futures = new ArrayList<>();
final DataStreamOutput out = client.getStreamOutput(file.getName(), fileSize, routingTable);
try (FileInputStream fis = new FileInputStream(file)) {
final FileChannel in = fis.getChannel();
try (FileChannel in = FileUtils.newFileChannel(file, StandardOpenOption.READ)) {
for (long offset = 0L; offset < fileSize; ) {
offset += write(in, out, offset, futures);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -22,11 +22,12 @@
import org.apache.ratis.examples.filestore.FileStoreClient;
import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
import org.apache.ratis.thirdparty.io.netty.buffer.PooledByteBufAllocator;
import org.apache.ratis.util.FileUtils;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.channels.FileChannel;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -97,8 +98,7 @@ private Map<String, CompletableFuture<List<CompletableFuture<Long>>>> writeByHea
CompletableFuture.supplyAsync(() -> {
List<CompletableFuture<Long>> futures = new ArrayList<>();
File file = new File(path);
try (FileInputStream fis = new FileInputStream(file)) {
final FileChannel in = fis.getChannel();
try (FileChannel in = FileUtils.newFileChannel(file, StandardOpenOption.READ)) {
for (long offset = 0L; offset < getFileSizeInBytes(); ) {
offset += write(in, offset, client, file.getName(), futures);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@

import org.apache.ratis.io.CorruptedFileException;
import org.apache.ratis.metrics.Timekeeper;
import org.apache.ratis.proto.RaftProtos.LogEntryProto;
import org.apache.ratis.protocol.exceptions.ChecksumException;
import org.apache.ratis.server.metrics.SegmentedRaftLogMetrics;
import org.apache.ratis.server.raftlog.RaftLog;
import org.apache.ratis.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.ratis.thirdparty.com.google.protobuf.CodedOutputStream;
import org.apache.ratis.proto.RaftProtos.LogEntryProto;
import org.apache.ratis.util.FileUtils;
import org.apache.ratis.util.IOUtils;
import org.apache.ratis.util.Preconditions;
import org.apache.ratis.util.PureJavaCrc32C;
Expand All @@ -33,15 +34,22 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.*;
import java.io.BufferedInputStream;
import java.io.Closeable;
import java.io.DataInputStream;
import java.io.EOFException;
import java.io.File;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
import java.util.zip.Checksum;

class SegmentedRaftLogReader implements Closeable {
static final Logger LOG = LoggerFactory.getLogger(SegmentedRaftLogReader.class);
/**
* InputStream wrapper that keeps track of the current stream position.
*
* <p>
* This stream also allows us to set a limit on how many bytes we can read
* without getting an exception.
*/
Expand Down Expand Up @@ -141,11 +149,9 @@ public long skip(long amt) throws IOException {
private final SegmentedRaftLogMetrics raftLogMetrics;
private final SizeInBytes maxOpSize;

SegmentedRaftLogReader(File file, SizeInBytes maxOpSize, SegmentedRaftLogMetrics raftLogMetrics)
throws FileNotFoundException {
SegmentedRaftLogReader(File file, SizeInBytes maxOpSize, SegmentedRaftLogMetrics raftLogMetrics) throws IOException {
this.file = file;
this.limiter = new LimitedInputStream(
new BufferedInputStream(new FileInputStream(file)));
this.limiter = new LimitedInputStream(new BufferedInputStream(FileUtils.newInputStream(file)));
in = new DataInputStream(limiter);
checksum = new PureJavaCrc32C();
this.maxOpSize = maxOpSize;
Expand All @@ -154,13 +160,10 @@ public long skip(long amt) throws IOException {

/**
* Read header from the log file:
*
* (1) The header in file is verified successfully.
* Then, return true.
*
* (2) The header in file is partially written.
* Then, return false.
*
* (3) The header in file is corrupted or there is some other {@link IOException}.
* Then, throw an exception.
*/
Expand Down Expand Up @@ -197,7 +200,7 @@ LogEntryProto readEntry() throws IOException {
final Timekeeper timekeeper = Optional.ofNullable(raftLogMetrics)
.map(SegmentedRaftLogMetrics::getReadEntryTimer)
.orElse(null);
try(AutoCloseable readEntryContext = Timekeeper.start(timekeeper)) {
try(AutoCloseable ignored = Timekeeper.start(timekeeper)) {
return decodeEntry();
} catch (EOFException eof) {
in.reset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
import org.apache.ratis.proto.RaftProtos.FileChunkProto;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.apache.ratis.thirdparty.com.google.protobuf.UnsafeByteOperations;
import org.apache.ratis.util.FileUtils;
import org.apache.ratis.util.IOUtils;
import org.apache.ratis.util.JavaUtils;

import java.io.Closeable;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Path;
Expand Down Expand Up @@ -57,13 +57,19 @@ public FileChunkReader(FileInfo info, Path relativePath) throws IOException {
final File f = info.getPath().toFile();
if (info.getFileDigest() == null) {
digester = MD5Hash.getDigester();
this.in = new DigestInputStream(new FileInputStream(f), digester);
this.in = new DigestInputStream(FileUtils.newInputStream(f), digester);
} else {
digester = null;
this.in = new FileInputStream(f);
this.in = FileUtils.newInputStream(f);
}
}

static ByteString readFileChunk(int chunkLength, InputStream in) throws IOException {
final byte[] chunkBuffer = new byte[chunkLength];
IOUtils.readFully(in, chunkBuffer, 0, chunkBuffer.length);
return UnsafeByteOperations.unsafeWrap(chunkBuffer);
}

/**
* Read the next chunk.
*
Expand All @@ -74,9 +80,7 @@ public FileChunkReader(FileInfo info, Path relativePath) throws IOException {
public FileChunkProto readFileChunk(int chunkMaxSize) throws IOException {
final long remaining = info.getFileSize() - offset;
final int chunkLength = remaining < chunkMaxSize ? (int) remaining : chunkMaxSize;
final byte[] chunkBuffer = new byte[chunkLength];
IOUtils.readFully(in, chunkBuffer, 0, chunkBuffer.length);
final ByteString data = UnsafeByteOperations.unsafeWrap(chunkBuffer);
final ByteString data = readFileChunk(chunkLength, in);
// whether this chunk is the last chunk of current file
final boolean isDone = offset + chunkLength == info.getFileSize();
final ByteString fileDigest;
Expand Down
Loading

0 comments on commit c4f04c1

Please sign in to comment.