You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2019/08/29 10:13:30 UTC
[zeppelin] branch master updated: [ZEPPELIN-4305]
LocalStorageConfig.atomicWriteToFile throws exception
This is an automated email from the ASF dual-hosted git repository.
zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 30ce442 [ZEPPELIN-4305] LocalStorageConfig.atomicWriteToFile throws exception
30ce442 is described below
commit 30ce442a768c0717f3424088426faaf3cf95f18c
Author: Alex Ott <al...@gmail.com>
AuthorDate: Mon Aug 19 18:47:05 2019 +0200
[ZEPPELIN-4305] LocalStorageConfig.atomicWriteToFile throws exception
### What is this PR for?
The hotfix that was made several weeks ago changed behavior of LocalStorageConfig.atomicWriteToFile, that started to atomically move files to destination. But this works without errors only when temporary directory and destination are on the same disk. When they are on different disks, atomic move isn't possible, so the exception is thrown.
This PR fixes this by performing non-atomic move to temp file on destination file system, and then atomically rename it to destination file.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
ZEPPELIN-4305
### Questions:
* Does the licenses files need update?
No
* Is there breaking changes for older versions?
No
* Does this needs documentation?
No
Author: Alex Ott <al...@gmail.com>
Closes #3428 from alexott/ZEPPELIN-4305 and squashes the following commits:
90e9c59f6 [Alex Ott] added unit test, and fix resource leak
f36508056 [Alex Ott] instead moving 2 times, just extract temp file into dest dir
293af1536 [Alex Ott] further improvements after code review
349188a44 [Alex Ott] [ZEPPELIN-4305] LocalStorageConfig.atomicWriteToFile throws exception
---
.../zeppelin/storage/LocalConfigStorage.java | 24 ++++++-----
.../zeppelin/storage/LocalConfigStorageTest.java | 50 ++++++++++++++++++++++
2 files changed, 63 insertions(+), 11 deletions(-)
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 b92182d..1bdb13b 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
@@ -17,6 +17,7 @@
package org.apache.zeppelin.storage;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.io.IOUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.interpreter.InterpreterInfoSaving;
@@ -103,14 +104,19 @@ public class LocalConfigStorage extends ConfigStorage {
atomicWriteToFile(credentials, credentialPath);
}
- private String readFromFile(File file) throws IOException {
- return IOUtils.toString(new FileInputStream(file));
+ @VisibleForTesting
+ static String readFromFile(File file) throws IOException {
+ try (FileInputStream is = new FileInputStream(file)) {
+ return IOUtils.toString(is);
+ }
}
- private void atomicWriteToFile(String content, File file) throws IOException {
- File tempFile = Files.createTempFile(file.getName(), null).toFile();
- FileOutputStream out = new FileOutputStream(tempFile);
- try {
+ @VisibleForTesting
+ static void atomicWriteToFile(String content, File file) throws IOException {
+ FileSystem defaultFileSystem = FileSystems.getDefault();
+ Path destinationFilePath = defaultFileSystem.getPath(file.getCanonicalPath());
+ File tempFile = Files.createTempFile(destinationFilePath.getParent(), file.getName(), null).toFile();
+ try (FileOutputStream out = new FileOutputStream(tempFile)) {
IOUtils.write(content, out);
} catch (IOException iox) {
if (!tempFile.delete()) {
@@ -118,13 +124,9 @@ public class LocalConfigStorage extends ConfigStorage {
}
throw iox;
}
- out.close();
- FileSystem defaultFileSystem = FileSystems.getDefault();
- Path destinationFilePath = defaultFileSystem.getPath(file.getCanonicalPath());
try {
file.getParentFile().mkdirs();
- Files.move(tempFile.toPath(), destinationFilePath,
- StandardCopyOption.ATOMIC_MOVE);
+ Files.move(tempFile.toPath(), destinationFilePath, StandardCopyOption.ATOMIC_MOVE);
} catch (IOException iox) {
if (!tempFile.delete()) {
tempFile.deleteOnExit();
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/storage/LocalConfigStorageTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/storage/LocalConfigStorageTest.java
new file mode 100644
index 0000000..2006a69
--- /dev/null
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/storage/LocalConfigStorageTest.java
@@ -0,0 +1,50 @@
+package org.apache.zeppelin.storage;
+
+import org.apache.commons.io.IOUtils;
+import org.junit.Test;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.junit.Assert.*;
+
+public class LocalConfigStorageTest {
+ public static final String TEST_STRING = "this is a test!";
+
+ @Test
+ public void testWritingAtomically() throws IOException {
+ final Path destination = Files.createTempFile("test-", "file");
+ final File destinationFile = destination.toFile();
+ try {
+ LocalConfigStorage.atomicWriteToFile(TEST_STRING, destinationFile);
+ try (InputStream is = Files.newInputStream(destination)) {
+ String read = IOUtils.toString(is);
+ assertEquals(TEST_STRING, read);
+ }
+ } finally {
+ Files.deleteIfExists(destination);
+ }
+ }
+
+ @Test
+ public void testReading() throws IOException {
+ final Path destination = Files.createTempFile("test-", "file");
+ final File destinationFile = destination.toFile();
+
+ try {
+ try (BufferedWriter writer = Files.newBufferedWriter(destination)) {
+ writer.write(TEST_STRING);
+ }
+ String read = LocalConfigStorage.readFromFile(destinationFile);
+ assertEquals(TEST_STRING, read);
+ } finally {
+ Files.deleteIfExists(destination);
+ }
+ }
+
+
+}
\ No newline at end of file