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