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 2018/01/26 01:03:40 UTC
zeppelin git commit: ZEPPELIN-3190. Should not use singleton for
FileSystemStorage
Repository: zeppelin
Updated Branches:
refs/heads/master 25d42e4e4 -> 8ddaab07c
ZEPPELIN-3190. Should not use singleton for FileSystemStorage
### What is this PR for?
For now, `FileSystemNotebookRepo`, `FileSystemConfigStorage`, `FileSystemRecoveryStorage` use `FileSystemStorage`, but the singleton pattern means that all the notebook, config and recovery need to be stored in the same storage which might not be proper for some users. So this PR is trying to use separate `FileSystemStorage` instance for `FileSystemNotebookRepo`, `FileSystemConfigStorage`, `FileSystemRecoveryStorage`
### What type of PR is it?
[Bug Fix | Improvement]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3190
### How should this be tested?
* Travis 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: Jeff Zhang <zj...@apache.org>
Closes #2746 from zjffdu/ZEPPELIN-3190 and squashes the following commits:
49611c2 [Jeff Zhang] ZEPPELIN-3190. Should not use singleton for FileSystemStorage
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/8ddaab07
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/8ddaab07
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/8ddaab07
Branch: refs/heads/master
Commit: 8ddaab07ca6fb6ff0a8991f55ba53d8122ce07d4
Parents: 25d42e4
Author: Jeff Zhang <zj...@apache.org>
Authored: Thu Jan 25 09:50:48 2018 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Fri Jan 26 09:03:34 2018 +0800
----------------------------------------------------------------------
.../interpreter/InterpreterSettingManager.java | 3 +--
.../recovery/FileSystemRecoveryStorage.java | 4 +++-
.../apache/zeppelin/notebook/FileSystemStorage.java | 16 +++++-----------
.../notebook/repo/FileSystemNotebookRepo.java | 5 +++--
.../zeppelin/storage/FileSystemConfigStorage.java | 8 ++++++--
5 files changed, 18 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8ddaab07/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
index a6583cb..bda1be6 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
@@ -294,7 +294,6 @@ public class InterpreterSettingManager {
}
})) {
String interpreterDirString = interpreterDir.toString();
-
/**
* Register interpreter by the following ordering
* 1. Register it from path {ZEPPELIN_HOME}/interpreter/{interpreter_name}/
@@ -304,7 +303,7 @@ public class InterpreterSettingManager {
*/
if (!registerInterpreterFromPath(interpreterDirString, interpreterJson)) {
if (!registerInterpreterFromResource(cl, interpreterDirString, interpreterJson)) {
- LOGGER.warn("No interpreter-setting.json found in " + interpreterDirPath);
+ LOGGER.warn("No interpreter-setting.json found in " + interpreterDirString);
}
}
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8ddaab07/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/recovery/FileSystemRecoveryStorage.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/recovery/FileSystemRecoveryStorage.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/recovery/FileSystemRecoveryStorage.java
index 5a0c8ad..9b1b6cb 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/recovery/FileSystemRecoveryStorage.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/recovery/FileSystemRecoveryStorage.java
@@ -69,7 +69,9 @@ public class FileSystemRecoveryStorage extends RecoveryStorage {
super(zConf);
this.interpreterSettingManager = interpreterSettingManager;
this.zConf = zConf;
- this.fs = FileSystemStorage.get(zConf);
+ this.fs = new FileSystemStorage(zConf, zConf.getRecoveryDir());
+ LOGGER.info("Creating FileSystem: " + this.fs.getFs().getClass().getName() +
+ " for Zeppelin Recovery.");
this.recoveryDir = this.fs.makeQualified(new Path(zConf.getRecoveryDir()));
LOGGER.info("Using folder {} to store recovery data", recoveryDir);
this.fs.tryMkDir(recoveryDir);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8ddaab07/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/FileSystemStorage.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/FileSystemStorage.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/FileSystemStorage.java
index 75c0bc3..24bab57 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/FileSystemStorage.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/FileSystemStorage.java
@@ -30,18 +30,16 @@ public class FileSystemStorage {
private static Logger LOGGER = LoggerFactory.getLogger(FileSystemStorage.class);
- private static FileSystemStorage instance;
-
private ZeppelinConfiguration zConf;
private Configuration hadoopConf;
private boolean isSecurityEnabled = false;
private FileSystem fs;
- private FileSystemStorage(ZeppelinConfiguration zConf) throws IOException {
+ public FileSystemStorage(ZeppelinConfiguration zConf, String path) throws IOException {
this.zConf = zConf;
this.hadoopConf = new Configuration();
// disable checksum for local file system. because interpreter.json may be updated by
- // no hadoop filesystem api
+ // non-hadoop filesystem api
this.hadoopConf.set("fs.file.impl", RawLocalFileSystem.class.getName());
this.isSecurityEnabled = UserGroupInformation.isSecurityEnabled();
@@ -58,18 +56,14 @@ public class FileSystemStorage {
}
try {
- this.fs = FileSystem.get(new URI(zConf.getNotebookDir()), this.hadoopConf);
- LOGGER.info("Creating FileSystem: " + this.fs.getClass().getCanonicalName());
+ this.fs = FileSystem.get(new URI(path), this.hadoopConf);
} catch (URISyntaxException e) {
throw new IOException(e);
}
}
- public static synchronized FileSystemStorage get(ZeppelinConfiguration zConf) throws IOException {
- if (instance == null) {
- instance = new FileSystemStorage(zConf);
- }
- return instance;
+ public FileSystem getFs() {
+ return fs;
}
public Path makeQualified(Path path) {
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8ddaab07/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java
index d8ec0e5..32bde37 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java
@@ -42,11 +42,12 @@ public class FileSystemNotebookRepo implements NotebookRepo {
private Path notebookDir;
public FileSystemNotebookRepo(ZeppelinConfiguration zConf) throws IOException {
- this.fs = FileSystemStorage.get(zConf);
+ this.fs = new FileSystemStorage(zConf, zConf.getNotebookDir());
+ LOGGER.info("Creating FileSystem: " + this.fs.getFs().getClass().getName() +
+ " for Zeppelin Notebook.");
this.notebookDir = this.fs.makeQualified(new Path(zConf.getNotebookDir()));
LOGGER.info("Using folder {} to store notebook", notebookDir);
this.fs.tryMkDir(notebookDir);
-
}
@Override
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8ddaab07/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
index 2460e4d..4df8163 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/storage/FileSystemConfigStorage.java
@@ -49,8 +49,12 @@ public class FileSystemConfigStorage extends ConfigStorage {
public FileSystemConfigStorage(ZeppelinConfiguration zConf) throws IOException {
super(zConf);
- this.fs = FileSystemStorage.get(zConf);
- this.fs.tryMkDir(new Path(zConf.getConfigFSDir()));
+ this.fs = new FileSystemStorage(zConf, zConf.getConfigFSDir());
+ LOGGER.info("Creating FileSystem: " + this.fs.getFs().getClass().getName() +
+ " for Zeppelin Config");
+ Path configPath = this.fs.makeQualified(new Path(zConf.getConfigFSDir()));
+ this.fs.tryMkDir(configPath);
+ LOGGER.info("Using folder {} to store Zeppelin Config", configPath);
this.interpreterSettingPath = fs.makeQualified(new Path(zConf.getInterpreterSettingPath()));
this.authorizationPath = fs.makeQualified(new Path(zConf.getNotebookAuthorizationPath()));
this.credentialPath = fs.makeQualified(new Path(zConf.getCredentialsPath()));