Skip to content

Commit

Permalink
Introduce flags to disable force per instance or globally
Browse files Browse the repository at this point in the history
The goal here is to:
* be able to selectively choose performance over integrity in some very
  specific cases
* be able to globally disable AtomicFileWriter integrity, and basically
  revert to previous behaviour by setting a system property.
  This flag is meant as an emergency one in case something goes very
  wron on some production system.
  • Loading branch information
batmat committed Jan 4, 2018
1 parent f555cb6 commit d692b09
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
36 changes: 35 additions & 1 deletion core/src/main/java/hudson/util/AtomicFileWriter.java
Expand Up @@ -23,6 +23,8 @@
*/
package hudson.util;

import jenkins.util.SystemProperties;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.io.File;
Expand Down Expand Up @@ -52,6 +54,15 @@ public class AtomicFileWriter extends Writer {

private static final Logger LOGGER = Logger.getLogger(AtomicFileWriter.class.getName());

private static /* final */ boolean LOSE_INTEGRITY = SystemProperties.getBoolean(
AtomicFileWriter.class.getName() + ".LOSE_DATA_INTEGRITY_FOR_PERFORMANCE");

static {
if (LOSE_INTEGRITY) {
LOGGER.log(Level.SEVERE, "LOSE_DATA_INTEGRITY_FOR_PERFORMANCE flag used, YOU RISK LOSING DATA.");
}
}

private final Writer core;
private final Path tmpPath;
private final Path destPath;
Expand Down Expand Up @@ -94,6 +105,21 @@ private static Path toPath(@Nonnull File file) throws IOException {
* @param charset File charset to write.
*/
public AtomicFileWriter(@Nonnull Path destinationPath, @Nonnull Charset charset) throws IOException {
// See FileChannelWriter docs to understand why we do not cause a force() call on flush() from AtomicFileWriter.
this(destinationPath, charset, true, false);
}

/**
* <strong>DO NOT USE THIS METHOD, OR YOU WILL LOSE DATA INTEGRITY.</strong>
*
* @param destinationPath the destination path where to write the content when committed.
* @param charset File charset to write.
* @param integrityOnFlush do not force writing to disk when flushing
* @param integrityOnClose do not force writing to disk when closing
* @deprecated use {@link AtomicFileWriter#AtomicFileWriter(Path, Charset)}
*/
@Deprecated
public AtomicFileWriter(@Nonnull Path destinationPath, @Nonnull Charset charset, boolean integrityOnFlush, boolean integrityOnClose) throws IOException {
if (charset == null) { // be extra-defensive if people don't care
throw new IllegalArgumentException("charset is null");
}
Expand All @@ -117,7 +143,15 @@ public AtomicFileWriter(@Nonnull Path destinationPath, @Nonnull Charset charset)
throw new IOException("Failed to create a temporary file in "+ dir,e);
}

core = new FileChannelWriter(tmpPath, charset, false, StandardOpenOption.WRITE);
if (LOSE_INTEGRITY) {
// We should log it in WARN, but as this code is called often, this would create huge logs in prod system
// Hence why a similar log is logged once above in a static block.
LOGGER.log(Level.FINE, "WARNING: YOU SET A FLAG THAT COULD LEAD TO DATA LOSS.");
integrityOnFlush = false;
integrityOnClose = false;
}

core = new FileChannelWriter(tmpPath, charset, integrityOnFlush, integrityOnClose, StandardOpenOption.WRITE);
}

@Override
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/hudson/util/FileChannelWriter.java
Expand Up @@ -46,16 +46,22 @@ public class FileChannelWriter extends Writer {
*/
private boolean forceOnFlush;

/**
* See forceOnFlush. You probably never want to set forceOnClose to false.
*/
private boolean forceOnClose;

/**
* @param filePath the path of the file to write to.
* @param charset the charset to use when writing characters.
* @param forceOnFlush set to true if you want {@link FileChannel#force(boolean)} to be called on {@link #flush()}.
* @param options the options for opening the file.
* @throws IOException if something went wrong.
*/
FileChannelWriter(Path filePath, Charset charset, boolean forceOnFlush, OpenOption... options) throws IOException {
FileChannelWriter(Path filePath, Charset charset, boolean forceOnFlush, boolean forceOnClose, OpenOption... options) throws IOException {
this.charset = charset;
this.forceOnFlush = forceOnFlush;
this.forceOnClose = forceOnClose;
channel = FileChannel.open(filePath, options);
}

Expand All @@ -79,7 +85,9 @@ public void flush() throws IOException {
@Override
public void close() throws IOException {
if(channel.isOpen()) {
channel.force(true);
if (forceOnClose) {
channel.force(true);
}
channel.close();
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/hudson/util/FileChannelWriterTest.java
Expand Up @@ -27,7 +27,7 @@ public class FileChannelWriterTest {
@Before
public void setUp() throws Exception {
file = temporaryFolder.newFile();
writer = new FileChannelWriter(file.toPath(), StandardCharsets.UTF_8, true, StandardOpenOption.WRITE);
writer = new FileChannelWriter(file.toPath(), StandardCharsets.UTF_8, true, true, StandardOpenOption.WRITE);
}

@Test
Expand Down

0 comments on commit d692b09

Please sign in to comment.