You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by bz...@apache.org on 2016/01/05 10:10:05 UTC
incubator-zeppelin git commit: ZEPPELIN-543: Storage initialization
failure recovery
Repository: incubator-zeppelin
Updated Branches:
refs/heads/master 496dd22c2 -> 3123d3de8
ZEPPELIN-543: Storage initialization failure recovery
### What is this PR for?
Handling edge case failures when initializing notebook storage class(es).
### What type of PR is it?
Improvement
### Todos
* [x] - fix repo initialization
* [x] - add different config tests
### Is there a relevant Jira issue?
[ZEPPELIN-543](https://issues.apache.org/jira/browse/ZEPPELIN-543)
### How should this be tested?
There're various test cases (mostly included as unit tests).
One of the test scenarios:
1. in `conf/zeppelin-env.sh` add the following line
```
export ZEPPELIN_NOTEBOOK_STORAGE="org.apache.zeppelin.notebook.repo.VFSNotebookRepo,org.apache.zeppelin.notebook.repo.DummyNotebookRepo"
```
2. Start Zeppelin
3. Previously Zeppelin would fail to start, now you would see that only first storage class is initialized and second just didn't initialize.
### 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: Khalid Huseynov <kh...@nflabs.com>
Closes #582 from khalidhuseynov/storage-init-failure-recovery and squashes the following commits:
334ec41 [Khalid Huseynov] Merge branch 'master' into storage-init-failure-recovery
812cf1c [Khalid Huseynov] lowercase 3 methods
322a29f [Khalid Huseynov] addressing comments
42f298e [Khalid Huseynov] add tests for different configs
60a9caa [Khalid Huseynov] remove throwing exception, handle gracefully
Project: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/commit/3123d3de
Tree: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/tree/3123d3de
Diff: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/diff/3123d3de
Branch: refs/heads/master
Commit: 3123d3de85bc3e28223f0a60ff598ea478e1f593
Parents: 496dd22
Author: Khalid Huseynov <kh...@nflabs.com>
Authored: Tue Jan 5 16:16:06 2016 +0900
Committer: Alexander Bezzubov <bz...@apache.org>
Committed: Tue Jan 5 18:09:51 2016 +0900
----------------------------------------------------------------------
.../notebook/repo/NotebookRepoSync.java | 58 +++++--
.../NotebookRepoSyncInitializationTest.java | 159 +++++++++++++++++++
2 files changed, 207 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/3123d3de/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
index 6f964f2..caab185 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
@@ -19,6 +19,7 @@ package org.apache.zeppelin.notebook.repo;
import java.io.IOException;
import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
@@ -42,6 +43,9 @@ public class NotebookRepoSync implements NotebookRepo {
private static final String pushKey = "pushNoteIDs";
private static final String pullKey = "pullNoteIDs";
+ private static ZeppelinConfiguration config;
+ private static final String defaultStorage = "org.apache.zeppelin.notebook.repo.VFSNotebookRepo";
+
private List<NotebookRepo> repos = new ArrayList<NotebookRepo>();
/**
@@ -49,26 +53,60 @@ public class NotebookRepoSync implements NotebookRepo {
* @param (conf)
* @throws - Exception
*/
- public NotebookRepoSync(ZeppelinConfiguration conf) throws Exception {
+ @SuppressWarnings("static-access")
+ public NotebookRepoSync(ZeppelinConfiguration conf) {
+ config = conf;
String allStorageClassNames = conf.getString(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE).trim();
if (allStorageClassNames.isEmpty()) {
- throw new IOException("Empty ZEPPELIN_NOTEBOOK_STORAGE conf parameter");
+ allStorageClassNames = defaultStorage;
+ LOG.warn("Empty ZEPPELIN_NOTEBOOK_STORAGE conf parameter, using default {}", defaultStorage);
}
String[] storageClassNames = allStorageClassNames.split(",");
if (storageClassNames.length > getMaxRepoNum()) {
- throw new IOException("Unsupported number of storage classes (" +
- storageClassNames.length + ") in ZEPPELIN_NOTEBOOK_STORAGE");
+ LOG.warn("Unsupported number {} of storage classes in ZEPPELIN_NOTEBOOK_STORAGE : {}\n" +
+ "first {} will be used", storageClassNames.length, allStorageClassNames, getMaxRepoNum());
}
- for (int i = 0; i < storageClassNames.length; i++) {
+ for (int i = 0; i < Math.min(storageClassNames.length, getMaxRepoNum()); i++) {
@SuppressWarnings("static-access")
- Class<?> notebookStorageClass = getClass().forName(storageClassNames[i].trim());
+ Class<?> notebookStorageClass;
+ try {
+ notebookStorageClass = getClass().forName(storageClassNames[i].trim());
+ Constructor<?> constructor = notebookStorageClass.getConstructor(
+ ZeppelinConfiguration.class);
+ repos.add((NotebookRepo) constructor.newInstance(conf));
+ } catch (ClassNotFoundException | NoSuchMethodException | SecurityException |
+ InstantiationException | IllegalAccessException | IllegalArgumentException |
+ InvocationTargetException e) {
+ LOG.warn("Failed to initialize {} notebook storage class {}", storageClassNames[i], e);
+ }
+ }
+ // couldn't initialize any storage, use default
+ if (getRepoCount() == 0) {
+ LOG.info("No storages could be initialized, using default {} storage", defaultStorage);
+ initializeDefaultStorage(conf);
+ }
+ if (getRepoCount() > 1) {
+ try {
+ sync(0, 1);
+ } catch (IOException e) {
+ LOG.warn("Failed to sync with secondary storage on start {}", e);
+ }
+ }
+ }
+
+ @SuppressWarnings("static-access")
+ private void initializeDefaultStorage(ZeppelinConfiguration conf) {
+ Class<?> notebookStorageClass;
+ try {
+ notebookStorageClass = getClass().forName(defaultStorage);
Constructor<?> constructor = notebookStorageClass.getConstructor(
ZeppelinConfiguration.class);
repos.add((NotebookRepo) constructor.newInstance(conf));
- }
- if (getRepoCount() > 1) {
- sync(0, 1);
+ } catch (ClassNotFoundException | NoSuchMethodException | SecurityException |
+ InstantiationException | IllegalAccessException | IllegalArgumentException |
+ InvocationTargetException e) {
+ LOG.warn("Failed to initialize {} notebook storage class {}", defaultStorage, e);
}
}
@@ -184,7 +222,7 @@ public class NotebookRepoSync implements NotebookRepo {
return maxRepoNum;
}
- private NotebookRepo getRepo(int repoIndex) throws IOException {
+ NotebookRepo getRepo(int repoIndex) throws IOException {
if (repoIndex < 0 || repoIndex >= getRepoCount()) {
throw new IOException("Storage repo index is out of range");
}
http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/3123d3de/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncInitializationTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncInitializationTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncInitializationTest.java
new file mode 100644
index 0000000..53c052a
--- /dev/null
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncInitializationTest.java
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.notebook.repo;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
+import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
+import org.apache.zeppelin.notebook.repo.mock.VFSNotebookRepoMock;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NotebookRepoSyncInitializationTest {
+ private static final Logger LOG = LoggerFactory.getLogger(NotebookRepoSyncInitializationTest.class);
+ private String validFirstStorageClass = "org.apache.zeppelin.notebook.repo.VFSNotebookRepo";
+ private String validSecondStorageClass = "org.apache.zeppelin.notebook.repo.mock.VFSNotebookRepoMock";
+ private String invalidStorageClass = "org.apache.zeppelin.notebook.repo.DummyNotebookRepo";
+ private String validOneStorageConf = validFirstStorageClass;
+ private String validTwoStorageConf = validFirstStorageClass + "," + validSecondStorageClass;
+ private String invalidTwoStorageConf = validFirstStorageClass + "," + invalidStorageClass;
+ private String unsupportedStorageConf = validFirstStorageClass + "," + validSecondStorageClass + "," + validSecondStorageClass;
+ private String emptyStorageConf = "";
+
+ @Before
+ public void setUp(){
+ //setup routine
+ }
+
+ @After
+ public void tearDown() {
+ //tear-down routine
+ }
+
+ @Test
+ public void validInitOneStorageTest() throws IOException {
+ // no need to initialize folder due to one storage
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), validOneStorageConf);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check proper initialization of one storage
+ assertEquals(notebookRepoSync.getRepoCount(), 1);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ }
+
+ @Test
+ public void validInitTwoStorageTest() throws IOException {
+ // initialize folders for each storage
+ String zpath = System.getProperty("java.io.tmpdir") + "/ZeppelinLTest_" + System.currentTimeMillis();
+ File mainZepDir = new File(zpath);
+ mainZepDir.mkdirs();
+ new File(mainZepDir, "conf").mkdirs();
+ String mainNotePath = zpath+"/notebook";
+ String secNotePath = mainNotePath + "_secondary";
+ File mainNotebookDir = new File(mainNotePath);
+ File secNotebookDir = new File(secNotePath);
+ mainNotebookDir.mkdirs();
+ secNotebookDir.mkdirs();
+
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), mainZepDir.getAbsolutePath());
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(), mainNotebookDir.getAbsolutePath());
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), validTwoStorageConf);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check that both initialized
+ assertEquals(notebookRepoSync.getRepoCount(), 2);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ assertTrue(notebookRepoSync.getRepo(1) instanceof VFSNotebookRepoMock);
+ }
+
+ @Test
+ public void invalidInitTwoStorageTest() throws IOException {
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), invalidTwoStorageConf);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check that second didn't initialize
+ LOG.info(" " + notebookRepoSync.getRepoCount());
+ assertEquals(notebookRepoSync.getRepoCount(), 1);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ }
+
+ @Test
+ public void initUnsupportedNumberStoragesTest() throws IOException {
+ // initialize folders for each storage, currently for 2 only
+ String zpath = System.getProperty("java.io.tmpdir") + "/ZeppelinLTest_" + System.currentTimeMillis();
+ File mainZepDir = new File(zpath);
+ mainZepDir.mkdirs();
+ new File(mainZepDir, "conf").mkdirs();
+ String mainNotePath = zpath+"/notebook";
+ String secNotePath = mainNotePath + "_secondary";
+ File mainNotebookDir = new File(mainNotePath);
+ File secNotebookDir = new File(secNotePath);
+ mainNotebookDir.mkdirs();
+ secNotebookDir.mkdirs();
+
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_HOME.getVarName(), mainZepDir.getAbsolutePath());
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(), mainNotebookDir.getAbsolutePath());
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), unsupportedStorageConf);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check that first two storages initialized instead of three
+ assertEquals(notebookRepoSync.getRepoCount(), 2);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ assertTrue(notebookRepoSync.getRepo(1) instanceof VFSNotebookRepoMock);
+ }
+
+ @Test
+ public void initEmptyStorageTest() throws IOException {
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), emptyStorageConf);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check initialization of one default storage
+ assertEquals(notebookRepoSync.getRepoCount(), 1);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ }
+
+ @Test
+ public void initOneDummyStorageTest() throws IOException {
+ // set confs
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_STORAGE.getVarName(), invalidStorageClass);
+ ZeppelinConfiguration conf = ZeppelinConfiguration.create();
+ // create repo
+ NotebookRepoSync notebookRepoSync = new NotebookRepoSync(conf);
+ // check initialization of one default storage instead of invalid one
+ assertEquals(notebookRepoSync.getRepoCount(), 1);
+ assertTrue(notebookRepoSync.getRepo(0) instanceof VFSNotebookRepo);
+ }
+}
\ No newline at end of file