From 96c430fac7d135cf5c48950f2ec59e68fe0eeed3 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Wed, 1 Jan 2014 23:31:32 +0100 Subject: [PATCH] plugin manager: new `timeout` option When testing plugin manager with real downloads, it could happen that the test run forever. Fortunately, test suite will be interrupted after 20 minutes, but it could be useful not to fail the whole test suite but only warn in that case. By default, plugin manager still wait indefinitely but it can be modified using new `--timeout` option: ```sh bin/plugin --install elasticsearch/kibana --timeout 30s bin/plugin --install elasticsearch/kibana --timeout 1h ``` Closes #4603. Closes #4600. --- docs/reference/modules/plugins.asciidoc | 18 ++++++ .../http/client/HttpDownloadHelper.java | 59 ++++++++----------- .../elasticsearch/plugins/PluginManager.java | 27 +++++++-- .../plugin/PluginManagerTests.java | 32 ++++++---- 4 files changed, 84 insertions(+), 52 deletions(-) diff --git a/docs/reference/modules/plugins.asciidoc b/docs/reference/modules/plugins.asciidoc index 53c817ff17a30..8ea94312b485d 100644 --- a/docs/reference/modules/plugins.asciidoc +++ b/docs/reference/modules/plugins.asciidoc @@ -123,6 +123,24 @@ bin/plugin --install mobz/elasticsearch-head --verbose plugin --remove head --silent ----------------------------------- +[float] +==== Timeout settings + +By default, the `plugin` script will wait indefinitely when downloading before failing. +The timeout parameter can be used to explicitly specify how long it waits. Here is some examples of setting it to +different values: + +[source,shell] +----------------------------------- +# Wait for 30 seconds before failing +bin/plugin --install mobz/elasticsearch-head --timeout 30s + +# Wait for 1 minute before failing +bin/plugin --install mobz/elasticsearch-head --timeout 1m + +# Wait forever (default) +bin/plugin --install mobz/elasticsearch-head --timeout 0 +----------------------------------- [float] [[known-plugins]] diff --git a/src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java b/src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java index 7b1bf09364291..d3a1e537f4b3e 100644 --- a/src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java +++ b/src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java @@ -19,7 +19,10 @@ package org.elasticsearch.common.http.client; +import org.apache.lucene.util.IOUtils; +import org.elasticsearch.ElasticSearchTimeoutException; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.unit.TimeValue; import java.io.*; import java.net.HttpURLConnection; @@ -33,9 +36,8 @@ public class HttpDownloadHelper { private boolean useTimestamp = false; private boolean skipExisting = false; - private long maxTime = 0; - public boolean download(URL source, File dest, @Nullable DownloadProgress progress) throws IOException { + public boolean download(URL source, File dest, @Nullable DownloadProgress progress, TimeValue timeout) throws Exception { if (dest.exists() && skipExisting) { return true; } @@ -55,19 +57,20 @@ public boolean download(URL source, File dest, @Nullable DownloadProgress progre } GetThread getThread = new GetThread(source, dest, hasTimestamp, timestamp, progress); - getThread.setDaemon(true); - getThread.start(); + try { - getThread.join(maxTime * 1000); - } catch (InterruptedException ie) { - // ignore - } + getThread.setDaemon(true); + getThread.start(); + getThread.join(timeout.millis()); - if (getThread.isAlive()) { - String msg = "The GET operation took longer than " + maxTime - + " seconds, stopping it."; + if (getThread.isAlive()) { + throw new ElasticSearchTimeoutException("The GET operation took longer than " + timeout + ", stopping it."); + } + } + catch (InterruptedException ie) { + return false; + } finally { getThread.closeStreams(); - throw new IOException(msg); } return getThread.wasSuccessful(); @@ -329,16 +332,7 @@ private boolean downloadFile() throws FileNotFoundException, IOException { } finished = !isInterrupted(); } finally { - try { - os.close(); - } catch (IOException e) { - // ignore - } - try { - is.close(); - } catch (IOException e) { - // ignore - } + IOUtils.close(os, is); // we have started to (over)write dest, but failed. // Try to delete the garbage we'd otherwise leave @@ -374,20 +368,15 @@ boolean wasSuccessful() throws IOException { * Closes streams, interrupts the download, may delete the * output file. */ - void closeStreams() { + void closeStreams() throws IOException { interrupt(); - try { - os.close(); - } catch (IOException e) { - // ignore - } - try { - is.close(); - } catch (IOException e) { - // ignore - } - if (!success && dest.exists()) { - dest.delete(); + if (success) { + IOUtils.close(is, os); + } else { + IOUtils.closeWhileHandlingException(is, os); + if (dest != null && dest.exists()) { + dest.delete(); + } } } } diff --git a/src/main/java/org/elasticsearch/plugins/PluginManager.java b/src/main/java/org/elasticsearch/plugins/PluginManager.java index 0cc13a1f28f85..55c6e3b999248 100644 --- a/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -21,12 +21,14 @@ import com.google.common.base.Strings; import org.elasticsearch.ElasticSearchIllegalArgumentException; +import org.elasticsearch.ElasticSearchTimeoutException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.http.client.HttpDownloadHelper; import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.Environment; import org.elasticsearch.node.internal.InternalSettingsPreparer; @@ -60,15 +62,20 @@ public enum OutputMode { DEFAULT, SILENT, VERBOSE } + // By default timeout is 0 which means no timeout + public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMillis(0); + private final Environment environment; private String url; private OutputMode outputMode; + private TimeValue timeout; - public PluginManager(Environment environment, String url, OutputMode outputMode) { + public PluginManager(Environment environment, String url, OutputMode outputMode, TimeValue timeout) { this.environment = environment; this.url = url; this.outputMode = outputMode; + this.timeout = timeout; TrustManager[] trustAllCerts = new TrustManager[]{ new X509TrustManager() { @@ -124,9 +131,11 @@ public void downloadAndExtract(String name) throws IOException { URL pluginUrl = new URL(url); log("Trying " + pluginUrl.toExternalForm() + "..."); try { - downloadHelper.download(pluginUrl, pluginFile, progress); + downloadHelper.download(pluginUrl, pluginFile, progress, this.timeout); downloaded = true; - } catch (IOException e) { + } catch (ElasticSearchTimeoutException e) { + throw e; + } catch (Exception e) { // ignore log("Failed: " + ExceptionsHelper.detailedMessage(e)); } @@ -137,9 +146,11 @@ public void downloadAndExtract(String name) throws IOException { for (URL url : pluginHandle.urls()) { log("Trying " + url.toExternalForm() + "..."); try { - downloadHelper.download(url, pluginFile, progress); + downloadHelper.download(url, pluginFile, progress, this.timeout); downloaded = true; break; + } catch (ElasticSearchTimeoutException e) { + throw e; } catch (Exception e) { debug("Failed: " + ExceptionsHelper.detailedMessage(e)); } @@ -308,6 +319,7 @@ public static void main(String[] args) { String url = null; OutputMode outputMode = OutputMode.DEFAULT; String pluginName = null; + TimeValue timeout = DEFAULT_TIMEOUT; int action = ACTION.NONE; if (args.length < 1) { @@ -332,7 +344,9 @@ public static void main(String[] args) { || command.equals("install") || command.equals("-install")) { pluginName = args[++c]; action = ACTION.INSTALL; - + } else if (command.equals("-t") || command.equals("--timeout") + || command.equals("timeout") || command.equals("-timeout")) { + timeout = TimeValue.parseTimeValue(args[++c], DEFAULT_TIMEOUT); } else if (command.equals("-r") || command.equals("--remove") // Deprecated commands || command.equals("remove") || command.equals("-remove")) { @@ -357,7 +371,7 @@ public static void main(String[] args) { if (action > ACTION.NONE) { int exitCode = EXIT_CODE_ERROR; // we fail unless it's reset - PluginManager pluginManager = new PluginManager(initialSettings.v2(), url, outputMode); + PluginManager pluginManager = new PluginManager(initialSettings.v2(), url, outputMode, timeout); switch (action) { case ACTION.INSTALL: try { @@ -413,6 +427,7 @@ private static void displayHelp(String message) { System.out.println("Usage:"); System.out.println(" -u, --url [plugin location] : Set exact URL to download the plugin from"); System.out.println(" -i, --install [plugin name] : Downloads and installs listed plugins [*]"); + System.out.println(" -t, --timeout [duration] : Timeout setting: 30s, 1m, 1h... (infinite by default)"); System.out.println(" -r, --remove [plugin name] : Removes listed plugins"); System.out.println(" -l, --list : List installed plugins"); System.out.println(" -v, --verbose : Prints verbose messages"); diff --git a/src/test/java/org/elasticsearch/plugin/PluginManagerTests.java b/src/test/java/org/elasticsearch/plugin/PluginManagerTests.java index b84052a18776c..b85be454052ae 100644 --- a/src/test/java/org/elasticsearch/plugin/PluginManagerTests.java +++ b/src/test/java/org/elasticsearch/plugin/PluginManagerTests.java @@ -19,11 +19,13 @@ package org.elasticsearch.plugin; import org.elasticsearch.ElasticSearchIllegalArgumentException; +import org.elasticsearch.ElasticSearchTimeoutException; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.Environment; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.node.internal.InternalSettingsPreparer; @@ -123,13 +125,17 @@ public void testSitePluginWithSourceThrows() throws Exception { downloadAndExtract(pluginName, "file://" + url.getFile()); } + /** + * We build a plugin manager instance which wait only for 30 seconds before + * raising an ElasticSearchTimeoutException + */ private static PluginManager pluginManager(String pluginUrl) { Tuple initialSettings = InternalSettingsPreparer.prepareSettings( ImmutableSettings.settingsBuilder().build(), false); if (!initialSettings.v2().pluginsFile().exists()) { FileSystemUtils.mkdirs(initialSettings.v2().pluginsFile()); } - return new PluginManager(initialSettings.v2(), pluginUrl, PluginManager.OutputMode.SILENT); + return new PluginManager(initialSettings.v2(), pluginUrl, PluginManager.OutputMode.SILENT, TimeValue.timeValueSeconds(30)); } private static void downloadAndExtract(String pluginName, String pluginUrl) throws IOException { @@ -208,16 +214,20 @@ public void testInstallSitePlugin() throws IOException { private void singlePluginInstallAndRemove(String pluginShortName, String pluginCoordinates) throws IOException { PluginManager pluginManager = pluginManager(pluginCoordinates); - pluginManager.downloadAndExtract(pluginShortName); - File[] plugins = pluginManager.getListInstalledPlugins(); - assertThat(plugins, notNullValue()); - assertThat(plugins.length, is(1)); - - // We remove it - pluginManager.removePlugin(pluginShortName); - plugins = pluginManager.getListInstalledPlugins(); - assertThat(plugins, notNullValue()); - assertThat(plugins.length, is(0)); + try { + pluginManager.downloadAndExtract(pluginShortName); + File[] plugins = pluginManager.getListInstalledPlugins(); + assertThat(plugins, notNullValue()); + assertThat(plugins.length, is(1)); + + // We remove it + pluginManager.removePlugin(pluginShortName); + plugins = pluginManager.getListInstalledPlugins(); + assertThat(plugins, notNullValue()); + assertThat(plugins.length, is(0)); + } catch (ElasticSearchTimeoutException e) { + logger.warn("--> timeout exception raised while downloading plugin [{}]. Skipping test.", pluginShortName); + } } /**