Skip to content

RATIS-2260. AtomicFileOutputStream should truncate temp file #1237

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 2 commits into from
Mar 17, 2025
Merged

RATIS-2260. AtomicFileOutputStream should truncate temp file #1237

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5cca470
RATIS-2260. AtomicFileOutputStream should verify temp file does not e…
adoroszlai Mar 17, 2025
42aa34c
truncate existing
adoroszlai Mar 17, 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: 1 addition & 1 deletion ratis-common/src/main/java/org/apache/ratis/util/AtomicFileOutputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public AtomicFileOutputStream(File outFile) throws IOException {
}

public AtomicFileOutputStream(File outFile, File tmpFile) throws IOException {
super(FileUtils.newOutputStreamForceAtClose(tmpFile, StandardOpenOption.CREATE, StandardOpenOption.WRITE));
super(FileUtils.newOutputStreamForceAtClose(tmpFile, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE));
Copy link
Contributor

@szetszwo szetszwo Mar 17, 2025

Choose a reason for hiding this comment

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

Let's pass a createNew parameter. Usually, we don't care about overwriting tmp file since it is just tmp. On the other hand, we don't want to throw an exception when server starting because of a leftover tmp file. The leftover tmp file may be due to killing the previous run.

Copy link
Contributor Author

@adoroszlai adoroszlai Mar 17, 2025

Choose a reason for hiding this comment

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

If we allow the file to exist, at least we should truncate it. Otherwise leftover content may be included in the final file.

Copy link
Contributor

@szetszwo szetszwo Mar 17, 2025

Choose a reason for hiding this comment

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

You are right -- we need TRUNCATE_EXISTING.

  0: 0123456789
  0: aaaaaa6789
  0: bbb

  static void main(String[] args) throws Exception {
    final File f = new File("a.txt");
    try(OutputStream out = newOutputStreamForceAtClose(f, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) {
      out.write("0123456789".getBytes(StandardCharsets.UTF_8));
    }
    dump(f);
    try(OutputStream out = newOutputStreamForceAtClose(f, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) {
      out.write("aaaaaa".getBytes(StandardCharsets.UTF_8));
    }
    dump(f);
    try(OutputStream out = newOutputStreamForceAtClose(f, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) {
      out.write("bbb".getBytes(StandardCharsets.UTF_8));
    }
    dump(f);
  }

  static void dump(File f) throws Exception {
    try(BufferedReader in = new BufferedReader(new InputStreamReader(
        newInputStream(f.toPath()), StandardCharsets.UTF_8))) {
      String line;
      for(int i = 0; (line = in.readLine()) != null; i++) {
        System.out.printf("%3d: %s%n", i, line);
      }
    }
  }

this.outFile = outFile.getAbsoluteFile();
this.tmpFile = tmpFile.getAbsoluteFile();
}
Expand Down
Loading