[zeppelin] branch master updated: [ZEPPELIN-4356] Zeppelin should stop/die/etc when can't create/access notebook repo

[pr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

prabhjyotsingh 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 20bcd4f  [ZEPPELIN-4356] Zeppelin should stop/die/etc when can't create/access notebook repo
20bcd4f is described below

commit 20bcd4f32352acb10df0b6077393040585521afa
Author: Bhavik Patel <bh...@gmail.com>
AuthorDate: Mon Sep 30 20:37:01 2019 +0530

    [ZEPPELIN-4356] Zeppelin should stop/die/etc when can't create/access notebook repo
    
    ### What is this PR for?
    Currently when Zeppelin can't create/access the notebook repo, Zeppelin stays up but returns MultiException; Instead it would be better to stop/die/etc.
    
    ### What type of PR is it?
    [Bug Fix | Improvement]
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-4356
    
    ### How should this be tested?
    CI 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: Bhavik Patel <bh...@gmail.com>
    
    Closes #3468 from bhavikpatel9977/ZEPPELIN-4356 and squashes the following commits:
    
    4680a48d1 [Bhavik Patel] addressed review comments
    e87e6bb23 [Bhavik Patel] Zeppelin should stop/die/etc when can't create/access notebook repo
    
    Change-Id: Ib58bb79024d3e3dafd772d5f8e2c0a41664479a0
---
 .../java/org/apache/zeppelin/server/ErrorData.java | 49 +++++++++++++++++
 .../zeppelin/server/ImmediateErrorHandlerImpl.java | 62 ++++++++++++++++++++++
 .../org/apache/zeppelin/server/ZeppelinServer.java | 16 ++++--
 .../zeppelin/notebook/repo/NotebookRepoSync.java   |  1 -
 4 files changed, 124 insertions(+), 4 deletions(-)

diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ErrorData.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ErrorData.java
new file mode 100644
index 0000000..f467c4e
--- /dev/null
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ErrorData.java
@@ -0,0 +1,49 @@
+/*
+ * 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.server;
+
+import org.glassfish.hk2.api.ActiveDescriptor;
+
+/**
+ * ErrorData to store Descriptor and exception
+ */
+public class ErrorData {
+  private final ActiveDescriptor<?> descriptor;
+  private final Throwable th;
+
+  public ErrorData(ActiveDescriptor<?> descriptor, Throwable th) {
+    this.descriptor = descriptor;
+    this.th = th;
+  }
+
+  public ActiveDescriptor<?> getDescriptor() {
+    return descriptor;
+  }
+
+  public Throwable getThrowable() {
+    return th;
+  }
+
+  @Override
+  public String toString() {
+    return "ErrorData{" +
+            "descriptor=" + descriptor +
+            ", th=" + th +
+            '}';
+  }
+}
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ImmediateErrorHandlerImpl.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ImmediateErrorHandlerImpl.java
new file mode 100644
index 0000000..b21695f
--- /dev/null
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ImmediateErrorHandlerImpl.java
@@ -0,0 +1,62 @@
+/*
+ * 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.server;
+
+import org.glassfish.hk2.api.ActiveDescriptor;
+import org.glassfish.hk2.utilities.ImmediateErrorHandler;
+import javax.inject.Singleton;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * ImmediateErrorHandlerImpl to catch exception
+ */
+@Singleton
+public class ImmediateErrorHandlerImpl implements ImmediateErrorHandler {
+  private final List<ErrorData> constructionErrors = new LinkedList<ErrorData>();
+  private final List<ErrorData> destructionErrors = new LinkedList<ErrorData>();
+
+  @Override
+  public void postConstructFailed(ActiveDescriptor<?> immediateService, Throwable exception) {
+    synchronized (this) {
+      constructionErrors.add(new ErrorData(immediateService, exception));
+      this.notifyAll();
+    }
+  }
+
+  @Override
+  public void preDestroyFailed(ActiveDescriptor<?> immediateService, Throwable exception) {
+    synchronized (this) {
+      destructionErrors.add(new ErrorData(immediateService, exception));
+      this.notifyAll();
+    }
+  }
+
+  List<ErrorData> waitForAtLeastOneConstructionError(long waitTime) throws InterruptedException {
+    synchronized (this) {
+      while (constructionErrors.size() <= 0 && waitTime > 0) {
+        long currentTime = System.currentTimeMillis();
+        wait(waitTime);
+        long elapsedTime = System.currentTimeMillis() - currentTime;
+        waitTime -= elapsedTime;
+      }
+      return new LinkedList<ErrorData>(constructionErrors);
+    }
+  }
+
+}
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
index 2aaad7a..e8bef11 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
@@ -19,6 +19,7 @@ package org.apache.zeppelin.server;
 import java.io.File;
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
+import java.util.List;
 import java.util.EnumSet;
 import java.util.Objects;
 import java.util.stream.Stream;
@@ -43,7 +44,6 @@ import org.apache.zeppelin.interpreter.InterpreterFactory;
 import org.apache.zeppelin.interpreter.InterpreterOutput;
 import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.interpreter.InterpreterSettingManager;
-import org.apache.zeppelin.interpreter.recovery.NullRecoveryStorage;
 import org.apache.zeppelin.interpreter.recovery.RecoveryStorage;
 import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcessListener;
 import org.apache.zeppelin.notebook.NoteEventListener;
@@ -84,6 +84,7 @@ import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.eclipse.jetty.util.thread.ThreadPool;
 import org.eclipse.jetty.webapp.WebAppContext;
 import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
+import org.glassfish.hk2.api.Immediate;
 import org.glassfish.hk2.api.ServiceLocator;
 import org.glassfish.hk2.api.ServiceLocatorFactory;
 import org.glassfish.hk2.utilities.ServiceLocatorUtilities;
@@ -123,6 +124,11 @@ public class ZeppelinServer extends ResourceConfig {
 
     sharedServiceLocator = ServiceLocatorFactory.getInstance().create("shared-locator");
     ServiceLocatorUtilities.enableImmediateScope(sharedServiceLocator);
+    ServiceLocatorUtilities.addClasses(sharedServiceLocator,
+      NotebookRepoSync.class,
+      ImmediateErrorHandlerImpl.class);
+    ImmediateErrorHandlerImpl handler = sharedServiceLocator.getService(ImmediateErrorHandlerImpl.class);
+
     ServiceLocatorUtilities.bind(
         sharedServiceLocator,
         new AbstractBinder() {
@@ -135,7 +141,7 @@ public class ZeppelinServer extends ResourceConfig {
                     conf.getCredentialsEncryptKey());
 
             bindAsContract(InterpreterFactory.class).in(Singleton.class);
-            bindAsContract(NotebookRepoSync.class).to(NotebookRepo.class).in(Singleton.class);
+            bindAsContract(NotebookRepoSync.class).to(NotebookRepo.class).in(Immediate.class);
             bind(LuceneSearch.class).to(SearchService.class).in(Singleton.class);
             bindAsContract(Helium.class).in(Singleton.class);
             bind(conf).to(ZeppelinConfiguration.class);
@@ -148,7 +154,7 @@ public class ZeppelinServer extends ResourceConfig {
             bindAsContract(AuthorizationService.class).to(Singleton.class);
             // TODO(jl): Will make it more beautiful
             if (!StringUtils.isBlank(conf.getShiroPath())) {
-              bind(ShiroAuthenticationService.class).to(AuthenticationService.class).in(Singleton.class);
+              bind(ShiroAuthenticationService.class).to(AuthenticationService.class).in(Immediate.class);
             } else {
               // TODO(jl): Will be added more type
               bind(NoAuthenticationService.class).to(AuthenticationService.class).in(Singleton.class);
@@ -244,6 +250,10 @@ public class ZeppelinServer extends ResourceConfig {
     LOG.info("Starting zeppelin server");
     try {
       jettyWebServer.start(); // Instantiates ZeppelinServer
+      List<ErrorData> errorData = handler.waitForAtLeastOneConstructionError(5 * 1000);
+      if(errorData.size() > 0 && errorData.get(0).getThrowable() != null) {
+        throw new Exception(errorData.get(0).getThrowable());
+      }
       if (conf.getJettyName() != null) {
         org.eclipse.jetty.http.HttpGenerator.setJettyVersion(conf.getJettyName());
       }
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 9a5e829..df28ac6 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
@@ -26,7 +26,6 @@ import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.NotebookAuthorization;
 import org.apache.zeppelin.notebook.OldNoteInfo;
 import org.apache.zeppelin.notebook.Paragraph;
-import org.apache.zeppelin.notebook.repo.zeppelinhub.security.Authentication;
 import org.apache.zeppelin.plugin.PluginManager;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.apache.zeppelin.util.Util;