zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jongy...@apache.org
Subject zeppelin git commit: [ZEPPELIN-3636] Add timeout for s3 amazon bucket endpoint
Date Fri, 20 Jul 2018 00:56:26 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 1af1d8395 -> a290ccb0b


[ZEPPELIN-3636] Add timeout for s3 amazon bucket endpoint

### What is this PR for?
If there is no connection to amazonaws, app will wait for 2 minutes during setup:
```
INFO [2018-07-13 14:45:19,644] (Helium.java[loadConf]:103) - Add helium local registry /opt/zeppelin/product/helium
INFO [2018-07-13 14:45:19,645] (Helium.java[loadConf]:100) - Add helium online registry https://s3.amazonaws.com/helium-package/helium.json
ERROR [2018-07-13 14:47:27,098] (HeliumOnlineRegistry.java[getAll]:80) - Connect to s3.amazonaws.com:443
[s3.amazonaws.com/54.231.120.10] failed: Connection timed out (Connection timed out)
INFO [2018-07-13 14:47:28,104] ( ContextHandler.java[doStart]:744) - Started o.e.j.w.WebAppContext161479c6
```

Even if Amazon S3 notebook storage wasn't configured Helium goes to zeppelin.notebook.s3.endpoint.

It would be nice if we could set timeout for s3 bucket if we know that there is no connection
to amazonaws.

### What type of PR is it?
Improvement

### What is the Jira issue?
issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN-3636

### How should this be tested?
* Log after setting timeout property to 20000 in zeppelin-site.xml:
```
INFO [2018-07-18 19:35:55,514] ({main} Helium.java[loadConf]:103) - Add helium local registry
/home/egklimov/IdeaProjects/zeppelin/helium
INFO [2018-07-18 19:35:55,514] ({main} Helium.java[loadConf]:100) - Add helium online registry
https://s3.amazonaws.com/helium-package/helium.json
ERROR [2018-07-18 19:36:15,690] ({main} HeliumOnlineRegistry.java[getAll]:91) - Connect to
s3.amazonaws.com:443 [s3.amazonaws.com/192.168.65.17] failed: connect timed out
INFO [2018-07-18 19:36:16,823] ({main} ContextHandler.java[doStart]:744) - Started o.e.j.w.WebAppContext6107227e
```
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation?
Yes, information about Zeppelin Properties updated

Author: egorklimov <klim.electronicmail@gmail.com>

Closes #3082 from egorklimov/ZEPPELIN-3636 and squashes the following commits:

00fd306a1 [egorklimov] Socket timeout added
78d1fdd3c [egorklimov] Code refactored
28286bb03 [egorklimov] Timeout test added
9770a4793 [egorklimov] zeppelin-site.xml.template updated
0c9821610 [egorklimov] Docs updated
7d2c4d178 [egorklimov] ZeppelinConfiguration property zeppelin.notebook.s3.timeout added


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/a290ccb0
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/a290ccb0
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/a290ccb0

Branch: refs/heads/master
Commit: a290ccb0be70f6d11f39d7aa35f24208ae033553
Parents: 1af1d83
Author: egorklimov <klim.electronicmail@gmail.com>
Authored: Thu Jul 19 20:19:03 2018 +0300
Committer: Jongyoul Lee <jongyoul@apache.org>
Committed: Fri Jul 20 09:56:22 2018 +0900

----------------------------------------------------------------------
 conf/zeppelin-site.xml.template                 |  7 ++
 docs/setup/operation/configuration.md           |  6 ++
 .../zeppelin/conf/ZeppelinConfiguration.java    |  5 ++
 .../zeppelin/helium/HeliumOnlineRegistry.java   | 23 ++++++-
 .../helium/HeliumOnlineRegistryTest.java        | 67 ++++++++++++++++++++
 5 files changed, 105 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a290ccb0/conf/zeppelin-site.xml.template
----------------------------------------------------------------------
diff --git a/conf/zeppelin-site.xml.template b/conf/zeppelin-site.xml.template
index 9f6fd6f..3fa5e9b 100755
--- a/conf/zeppelin-site.xml.template
+++ b/conf/zeppelin-site.xml.template
@@ -113,10 +113,17 @@
 </property>
 
 <property>
+  <name>zeppelin.notebook.s3.timeout</name>
+  <value>120000</value>
+  <description>s3 bucket endpoint request timeout in msec.</description>
+</property>
+
+<property>
   <name>zeppelin.notebook.storage</name>
   <value>org.apache.zeppelin.notebook.repo.S3NotebookRepo</value>
   <description>notebook persistence layer implementation</description>
 </property>
+
 -->
 
 <!-- Additionally, encryption is supported for notebook data stored in S3 -->

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a290ccb0/docs/setup/operation/configuration.md
----------------------------------------------------------------------
diff --git a/docs/setup/operation/configuration.md b/docs/setup/operation/configuration.md
index 46426fd..0c4cc21 100644
--- a/docs/setup/operation/configuration.md
+++ b/docs/setup/operation/configuration.md
@@ -204,6 +204,12 @@ If both are defined, then the **environment variables** will take priority.
     <td>Endpoint for the bucket</td>
   </tr>
   <tr>
+    <td>N/A</td>
+    <td><h6 class="properties">zeppelin.notebook.s3.timeout</h6></td>
+    <td>120000</td>
+    <td>Bucket endpoint request timeout in msec</td>
+  </tr>
+  <tr>
     <td><h6 class="properties">ZEPPELIN_NOTEBOOK_S3_KMS_KEY_ID</h6></td>
     <td><h6 class="properties">zeppelin.notebook.s3.kmsKeyID</h6></td>
     <td></td>

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a290ccb0/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
index 0b13e59..9cb30c1 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
@@ -383,6 +383,10 @@ public class ZeppelinConfiguration extends XMLConfiguration {
     return getString(ConfVars.ZEPPELIN_NOTEBOOK_S3_ENDPOINT);
   }
 
+  public String getS3Timeout() {
+    return getString(ConfVars.ZEPPELIN_NOTEBOOK_S3_TIMEOUT);
+  }
+
   public String getS3KMSKeyID() {
     return getString(ConfVars.ZEPPELIN_NOTEBOOK_S3_KMS_KEY_ID);
   }
@@ -751,6 +755,7 @@ public class ZeppelinConfiguration extends XMLConfiguration {
     ZEPPELIN_NOTEBOOK_GCS_STORAGE_DIR("zeppelin.notebook.gcs.dir", ""),
     ZEPPELIN_NOTEBOOK_S3_BUCKET("zeppelin.notebook.s3.bucket", "zeppelin"),
     ZEPPELIN_NOTEBOOK_S3_ENDPOINT("zeppelin.notebook.s3.endpoint", "s3.amazonaws.com"),
+    ZEPPELIN_NOTEBOOK_S3_TIMEOUT("zeppelin.notebook.s3.timeout", "120000"),
     ZEPPELIN_NOTEBOOK_S3_USER("zeppelin.notebook.s3.user", "user"),
     ZEPPELIN_NOTEBOOK_S3_EMP("zeppelin.notebook.s3.encryptionMaterialsProvider", null),
     ZEPPELIN_NOTEBOOK_S3_KMS_KEY_ID("zeppelin.notebook.s3.kmsKeyID", null),

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a290ccb0/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
index 3e511b2..e54687a 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumOnlineRegistry.java
@@ -22,14 +22,22 @@ import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
 import org.apache.zeppelin.util.Util;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.*;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
 import java.net.URI;
 import java.util.LinkedList;
 import java.util.List;
@@ -58,7 +66,6 @@ public class HeliumOnlineRegistry extends HeliumRegistry {
   public HeliumOnlineRegistry(String name, String uri, File registryCacheDir) {
     super(name, uri);
     registryCacheDir.mkdirs();
-
     UUID registryCacheFileUuid = UUID.nameUUIDFromBytes(uri.getBytes());
     this.registryCacheFile = new File(registryCacheDir, registryCacheFileUuid.toString());
 
@@ -73,8 +80,18 @@ public class HeliumOnlineRegistry extends HeliumRegistry {
         .build();
     HttpGet get = new HttpGet(uri());
     HttpResponse response;
-
     try {
+      ZeppelinConfiguration cfg = ZeppelinConfiguration.create();
+      if ((get.getURI().getHost().equals(cfg.getS3Endpoint()))) {
+        if (cfg.getS3Timeout() != null) {
+          int timeout = Integer.valueOf(cfg.getS3Timeout());
+          RequestConfig requestCfg = RequestConfig.custom()
+                  .setConnectTimeout(timeout)
+                  .setSocketTimeout(timeout)
+                  .build();
+          get.setConfig(requestCfg);
+        }
+      }
       response = client.execute(get);
     } catch (Exception e) {
       logger.error(e.getMessage());

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a290ccb0/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumOnlineRegistryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumOnlineRegistryTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumOnlineRegistryTest.java
new file mode 100644
index 0000000..25bfb49
--- /dev/null
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumOnlineRegistryTest.java
@@ -0,0 +1,67 @@
+package org.apache.zeppelin.helium;
+
+import static org.junit.Assert.assertTrue;
+import org.apache.commons.io.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import java.io.File;
+import java.io.IOException;
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
+
+
+public class HeliumOnlineRegistryTest {
+  // ip 192.168.65.17 belongs to private network
+  // request will be ended with connection time out error
+  private static final String IP = "192.168.65.17";
+  private static final String TIMEOUT = "2000";
+
+  private File tmpDir;
+
+  @Before
+  public void setUp() throws Exception {
+    tmpDir = new File(
+            System.getProperty("java.io.tmpdir")
+                    + "/ZeppelinLTest_"
+                    + System.currentTimeMillis()
+    );
+  }
+
+  @After
+  public void tearDown() throws IOException {
+    FileUtils.deleteDirectory(tmpDir);
+  }
+
+  @Test
+  public void zeppelinNotebookS3TimeoutPropertyTest() throws IOException {
+    System.setProperty(
+            ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_S3_TIMEOUT.getVarName(),
+            TIMEOUT
+    );
+    System.setProperty(
+            ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_S3_ENDPOINT.getVarName(),
+            IP
+    );
+    HeliumOnlineRegistry heliumOnlineRegistry = new HeliumOnlineRegistry(
+            "https://" + IP,
+            "https://" + IP,
+            tmpDir
+    );
+
+    long start = System.currentTimeMillis();
+    heliumOnlineRegistry.getAll();
+    long processTime = System.currentTimeMillis() - start;
+
+    long basicTimeout = Long.valueOf(
+            ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_S3_TIMEOUT.getStringValue()
+    );
+    assertTrue(
+            String.format(
+                    "Wrong timeout during connection: expected %s, actual is about %d",
+                    TIMEOUT,
+                    processTime
+            ),
+            basicTimeout > processTime
+    );
+  }
+}


Mime
View raw message