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 <zjf...@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 <zjf...@apache.org> Authored: Thu Jan 25 09:50:48 2018 +0800 Committer: Jeff Zhang <zjf...@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()));
