Skip to content

Commit

Permalink
plugin manager: new timeout option
Browse files Browse the repository at this point in the history
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 elastic#4603.
Closes elastic#4600.
  • Loading branch information
dadoonet authored and brusic committed Jan 19, 2014
1 parent 3608d43 commit 41b1172
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 52 deletions.
18 changes: 18 additions & 0 deletions docs/reference/modules/plugins.asciidoc
Expand Up @@ -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]]
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}
}
}
Expand Down
27 changes: 21 additions & 6 deletions src/main/java/org/elasticsearch/plugins/PluginManager.java
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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));
}
Expand All @@ -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));
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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")) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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");
Expand Down
32 changes: 21 additions & 11 deletions src/test/java/org/elasticsearch/plugin/PluginManagerTests.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Settings, Environment> 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 {
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down

0 comments on commit 41b1172

Please sign in to comment.