Repository: zeppelin
Updated Branches:
  refs/heads/master a6d6b4224 -> bb5d95fca


[ZEPPELIN-3467] two-step, atomic configuration file

### What is this PR for?
This PR continues the discussion and implementation in 
[PR-2978](https://github.com/apache/zeppelin/pull/2978) which had to be closed 
earlier.

This PR prevents the total loss of configuration information that happens when 
zeppelin attempts to write out modified configuration information into an 
already full file-system. The approach taken here is to first write the 
modified information to a temporary file (in the same directory), and then to 
rename the successfully written file to its eventual name.

The rename operation is not attempted if there is an error while writing the 
temporary file, thus leaving the pre-existing configuration file untouched.

### What type of PR is it?
[Improvement]

### Todos
* [ ] - Task

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-3467

### How should this be tested?
CI Pass

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Sanjay Dasgupta <sanjay.dasgu...@gmail.com>

Closes #3000 from sanjaydasgupta/zepp-3467-atomic-config-file-writes and 
squashes the following commits:

4d3c23b94 [Sanjay Dasgupta] In response to Felix Cheung's comment 
https://github.com/apache/zeppelin/pull/3000#pullrequestreview-134911520
e12e9b371 [Sanjay Dasgupta] Taking care of Jeff Zhang's comment from 
https://github.com/apache/zeppelin/pull/3000#pullrequestreview-125418152
57170e58a [Sanjay Dasgupta] zepp-3467-atomic-config-file-writes: Initial updates


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/bb5d95fc
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/bb5d95fc
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/bb5d95fc

Branch: refs/heads/master
Commit: bb5d95fca0a518c038c65dfed7a27550fc3968d9
Parents: a6d6b42
Author: Sanjay Dasgupta <sanjay.dasgu...@gmail.com>
Authored: Fri Jul 6 15:16:42 2018 +0530
Committer: Jongyoul Lee <jongy...@apache.org>
Committed: Wed Jul 11 10:47:58 2018 +0900

----------------------------------------------------------------------
 .../zeppelin/storage/LocalConfigStorage.java    | 40 ++++++++++++++++----
 1 file changed, 32 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/bb5d95fc/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
----------------------------------------------------------------------
diff --git 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
index 464d6ce..b91ded4 100644
--- 
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
+++ 
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/LocalConfigStorage.java
@@ -28,7 +28,11 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
-
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.FileSystems;
+import java.nio.file.FileSystem;
+import java.nio.file.StandardCopyOption;
 
 /**
  * Storing config in local file system
@@ -51,7 +55,7 @@ public class LocalConfigStorage extends ConfigStorage {
   @Override
   public void save(InterpreterInfoSaving settingInfos) throws IOException {
     LOGGER.info("Save Interpreter Setting to " + 
interpreterSettingPath.getAbsolutePath());
-    writeToFile(settingInfos.toJson(), interpreterSettingPath);
+    atomicWriteToFile(settingInfos.toJson(), interpreterSettingPath);
   }
 
   @Override
@@ -68,7 +72,7 @@ public class LocalConfigStorage extends ConfigStorage {
   @Override
   public void save(NotebookAuthorizationInfoSaving authorizationInfoSaving) 
throws IOException {
     LOGGER.info("Save notebook authorization to file: " + authorizationPath);
-    writeToFile(authorizationInfoSaving.toJson(), authorizationPath);
+    atomicWriteToFile(authorizationInfoSaving.toJson(), authorizationPath);
   }
 
   @Override
@@ -95,17 +99,37 @@ public class LocalConfigStorage extends ConfigStorage {
   @Override
   public void saveCredentials(String credentials) throws IOException {
     LOGGER.info("Save Credentials to file: " + credentialPath);
-    writeToFile(credentials, credentialPath);
+    atomicWriteToFile(credentials, credentialPath);
   }
 
   private String readFromFile(File file) throws IOException {
     return IOUtils.toString(new FileInputStream(file));
   }
 
-  private void writeToFile(String content, File file) throws IOException {
-    FileOutputStream out = new FileOutputStream(file);
-    IOUtils.write(content, out);
+  private void atomicWriteToFile(String content, File file) throws IOException 
{
+    File directory = file.getParentFile();
+    File tempFile = File.createTempFile(file.getName(), null, directory);
+    FileOutputStream out = new FileOutputStream(tempFile);
+    try {
+      IOUtils.write(content, out);
+    } catch (IOException iox) {
+      if (!tempFile.delete()) {
+        tempFile.deleteOnExit();
+      }
+      throw iox;
+    }
     out.close();
+    FileSystem defaultFileSystem = FileSystems.getDefault();
+    Path tempFilePath = defaultFileSystem.getPath(tempFile.getCanonicalPath());
+    Path destinationFilePath = 
defaultFileSystem.getPath(file.getCanonicalPath());
+    try {
+      Files.move(tempFilePath, destinationFilePath, 
StandardCopyOption.ATOMIC_MOVE);
+    } catch (IOException iox) {
+      if (!tempFile.delete()) {
+        tempFile.deleteOnExit();
+      }
+      throw iox;
+    }
   }
 
-}
+}
\ No newline at end of file

Reply via email to