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 <[email protected]> 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 <[email protected]> Authored: Tue Jan 5 16:16:06 2016 +0900 Committer: Alexander Bezzubov <[email protected]> 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
