Skip to content

Commit 39c3239

Browse files
committed
Release connections fix
1 parent 8e2a569 commit 39c3239

File tree

2 files changed

+19
-32
lines changed

2 files changed

+19
-32
lines changed

agent/src/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,9 @@
2222
import com.cloud.utils.exception.CloudRuntimeException;
2323
import org.apache.commons.collections.MapUtils;
2424
import org.apache.commons.httpclient.HttpClient;
25-
import org.apache.commons.httpclient.HttpMethod;
26-
import org.apache.commons.httpclient.HttpMethodRetryHandler;
2725
import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
28-
import org.apache.commons.httpclient.NoHttpResponseException;
26+
import org.apache.commons.httpclient.HttpStatus;
2927
import org.apache.commons.httpclient.methods.GetMethod;
30-
import org.apache.commons.httpclient.params.HttpMethodParams;
3128
import org.apache.commons.io.IOUtils;
3229
import org.apache.log4j.Logger;
3330

@@ -43,16 +40,15 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl {
4340

4441
protected HttpClient client;
4542
private static final MultiThreadedHttpConnectionManager s_httpClientManager = new MultiThreadedHttpConnectionManager();
46-
protected HttpMethodRetryHandler myretryhandler;
4743
public static final Logger s_logger = Logger.getLogger(HttpDirectTemplateDownloader.class.getName());
4844
protected GetMethod request;
4945
protected Map<String, String> reqHeaders = new HashMap<>();
5046

5147
public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map<String, String> headers) {
5248
super(url, destPoolPath, templateId, checksum);
5349
s_httpClientManager.getParams().setConnectionTimeout(5000);
50+
s_httpClientManager.getParams().setSoTimeout(5000);
5451
client = new HttpClient(s_httpClientManager);
55-
myretryhandler = createRetryTwiceHandler();
5652
request = createRequest(url, headers);
5753
String downloadDir = getDirectDownloadTempPath(templateId);
5854
createTemporaryDirectoryAndFile(downloadDir);
@@ -66,7 +62,6 @@ protected void createTemporaryDirectoryAndFile(String downloadDir) {
6662

6763
protected GetMethod createRequest(String downloadUrl, Map<String, String> headers) {
6864
GetMethod request = new GetMethod(downloadUrl);
69-
request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
7065
request.setFollowRedirects(true);
7166
if (MapUtils.isNotEmpty(headers)) {
7267
for (String key : headers.keySet()) {
@@ -77,37 +72,20 @@ protected GetMethod createRequest(String downloadUrl, Map<String, String> header
7772
return request;
7873
}
7974

80-
protected HttpMethodRetryHandler createRetryTwiceHandler() {
81-
return new HttpMethodRetryHandler() {
82-
@Override
83-
public boolean retryMethod(final HttpMethod method, final IOException exception, int executionCount) {
84-
if (executionCount >= 2) {
85-
// Do not retry if over max retry count
86-
return false;
87-
}
88-
if (exception instanceof NoHttpResponseException) {
89-
// Retry if the server dropped connection on us
90-
return true;
91-
}
92-
if (!method.isRequestSent()) {
93-
// Retry if the request has not been sent fully or
94-
// if it's OK to retry methods that have been sent
95-
return true;
96-
}
97-
// otherwise do not retry
98-
return false;
99-
}
100-
};
101-
}
102-
10375
@Override
10476
public boolean downloadTemplate() {
10577
try {
106-
client.executeMethod(request);
78+
int status = client.executeMethod(request);
79+
if (status != HttpStatus.SC_OK) {
80+
s_logger.warn("Not able to download template, status code: " + status);
81+
return false;
82+
}
83+
return performDownload();
10784
} catch (IOException e) {
10885
throw new CloudRuntimeException("Error on HTTP request: " + e.getMessage());
86+
} finally {
87+
request.releaseConnection();
10988
}
110-
return performDownload();
11189
}
11290

11391
protected boolean performDownload() {

utils/src/main/java/com/cloud/utils/UriUtils.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,8 @@ public static List<String> getMetalinkChecksums(String url) {
359359
}
360360
} catch (IOException e) {
361361
e.printStackTrace();
362+
} finally {
363+
getMethod.releaseConnection();
362364
}
363365
return null;
364366
}
@@ -425,6 +427,8 @@ protected static boolean checkUrlExistenceMetalink(String url) {
425427
}
426428
} catch (IOException e) {
427429
s_logger.warn(e.getMessage());
430+
} finally {
431+
getMethod.releaseConnection();
428432
}
429433
return false;
430434
}
@@ -441,6 +445,7 @@ public static List<String> getMetalinkUrls(String metalinkUrl) {
441445
status = httpClient.executeMethod(getMethod);
442446
} catch (IOException e) {
443447
s_logger.error("Error retrieving urls form metalink: " + metalinkUrl);
448+
getMethod.releaseConnection();
444449
return null;
445450
}
446451
try {
@@ -454,6 +459,8 @@ public static List<String> getMetalinkUrls(String metalinkUrl) {
454459
}
455460
} catch (IOException e) {
456461
s_logger.warn(e.getMessage());
462+
} finally {
463+
getMethod.releaseConnection();
457464
}
458465
return urls;
459466
}
@@ -474,6 +481,8 @@ public static void checkUrlExistence(String url) {
474481
throw new IllegalArgumentException("Cannot reach URL: " + url + " due to: " + hte.getMessage());
475482
} catch (IOException ioe) {
476483
throw new IllegalArgumentException("Cannot reach URL: " + url + " due to: " + ioe.getMessage());
484+
} finally {
485+
httphead.releaseConnection();
477486
}
478487
}
479488
}

0 commit comments

Comments
 (0)