From 2e6299399bd798ad2959a2b80fd3ac2d2a1374df Mon Sep 17 00:00:00 2001 From: gmungoc Date: Mon, 9 Oct 2017 15:31:13 +0100 Subject: [PATCH] JAL-2523 basic sleep and retry (3 times) on 429 response code with Retry-After --- src/jalview/ext/ensembl/EnsemblInfo.java | 5 - src/jalview/ext/ensembl/EnsemblLookup.java | 2 +- src/jalview/ext/ensembl/EnsemblRestClient.java | 121 ++++++++++---------- src/jalview/ext/ensembl/EnsemblSymbol.java | 20 ++-- src/jalview/ext/ensembl/EnsemblXref.java | 15 ++- .../jalview/ext/ensembl/EnsemblRestClientTest.java | 37 ++++-- 6 files changed, 110 insertions(+), 90 deletions(-) diff --git a/src/jalview/ext/ensembl/EnsemblInfo.java b/src/jalview/ext/ensembl/EnsemblInfo.java index 3108194..7668941 100644 --- a/src/jalview/ext/ensembl/EnsemblInfo.java +++ b/src/jalview/ext/ensembl/EnsemblInfo.java @@ -70,11 +70,6 @@ class EnsemblInfo // flag set to true if REST major version is not the one expected boolean restMajorVersionMismatch; - /* - * absolute time to wait till if we overloaded the REST service - */ - long retryAfter; - /** * Constructor given expected REST version number e.g 4.5 or 3.4.3 * diff --git a/src/jalview/ext/ensembl/EnsemblLookup.java b/src/jalview/ext/ensembl/EnsemblLookup.java index eb8f90e..6483401 100644 --- a/src/jalview/ext/ensembl/EnsemblLookup.java +++ b/src/jalview/ext/ensembl/EnsemblLookup.java @@ -134,7 +134,7 @@ public class EnsemblLookup extends EnsemblRestClient { br = getHttpResponse(url, ids); } - return (parseResponse(br)); + return br == null ? null : parseResponse(br); } catch (IOException e) { // ignore diff --git a/src/jalview/ext/ensembl/EnsemblRestClient.java b/src/jalview/ext/ensembl/EnsemblRestClient.java index 81bc560..c06d13e 100644 --- a/src/jalview/ext/ensembl/EnsemblRestClient.java +++ b/src/jalview/ext/ensembl/EnsemblRestClient.java @@ -56,6 +56,12 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher private static final int CONNECT_TIMEOUT_MS = 10 * 1000; // 10 seconds + private static final int MAX_RETRIES = 3; + + private static final int HTTP_OK = 200; + + private static final int HTTP_OVERLOAD = 429; + /* * update these constants when Jalview has been checked / updated for * changes to Ensembl REST API (ref JAL-2105) @@ -205,7 +211,7 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher * @see http://rest.ensembl.org/documentation/info/ping * @return */ - private boolean checkEnsembl() + boolean checkEnsembl() { BufferedReader br = null; try @@ -220,6 +226,10 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher * if ping takes more than 2 seconds to respond, treat as if unavailable */ br = getHttpResponse(ping, null, 2 * 1000); + if (br == null) + { + return false; + } JSONParser jp = new JSONParser(); JSONObject val = (JSONObject) jp.parse(br); String pingString = val.get("ping").toString(); @@ -282,7 +292,7 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher } /** - * Writes the HTTP request and gets the response as a reader. + * Sends the HTTP request and gets the response as a reader * * @param url * @param ids @@ -296,18 +306,25 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher protected BufferedReader getHttpResponse(URL url, List ids, int readTimeout) throws IOException { - // long now = System.currentTimeMillis(); - int maxRetries = 3; - int retriesLeft = maxRetries; + int retriesLeft = MAX_RETRIES; HttpURLConnection connection = null; + int responseCode = 0; + while (retriesLeft > 0) { connection = tryConnection(url, ids, readTimeout); + responseCode = connection.getResponseCode(); + if (responseCode == HTTP_OVERLOAD) // 429 + { + retriesLeft--; + checkRetryAfter(connection); + } + else + { + retriesLeft = 0; + } } - - int responseCode = connection.getResponseCode(); - - if (responseCode != 200) + if (responseCode != HTTP_OK) // 200 { /* * note: a GET request for an invalid id returns an error code e.g. 415 @@ -316,14 +333,12 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher System.err.println("Response code " + responseCode + " for " + url); return null; } - // get content + InputStream response = connection.getInputStream(); // System.out.println(getClass().getName() + " took " // + (System.currentTimeMillis() - now) + "ms to fetch"); - checkRateLimits(connection); - BufferedReader reader = null; reader = new BufferedReader(new InputStreamReader(response, "UTF-8")); return reader; @@ -340,6 +355,7 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher protected HttpURLConnection tryConnection(URL url, List ids, int readTimeout) throws IOException, ProtocolException { + // System.out.println(System.currentTimeMillis() + " " + url); HttpURLConnection connection = (HttpURLConnection) url.openConnection(); /* @@ -368,51 +384,36 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher } /** - * Inspect response headers for any sign of server overload and respect any - * 'retry-after' directive + * Inspects response headers for a 'retry-after' directive, and waits for the + * directed period (if less than 10 seconds) * * @see https://github.com/Ensembl/ensembl-rest/wiki/Rate-Limits * @param connection */ - void checkRateLimits(HttpURLConnection connection) + void checkRetryAfter(HttpURLConnection connection) { - // number of requests allowed per time interval: - String limit = connection.getHeaderField("X-RateLimit-Limit"); - // length of quota time interval in seconds: - // String period = connection.getHeaderField("X-RateLimit-Period"); - // seconds remaining until usage quota is reset: - String reset = connection.getHeaderField("X-RateLimit-Reset"); - // number of requests remaining from quota for current period: - String remaining = connection.getHeaderField("X-RateLimit-Remaining"); - // number of seconds to wait before retrying (if remaining == 0) String retryDelay = connection.getHeaderField("Retry-After"); // to test: // retryDelay = "5"; - EnsemblInfo info = domainData.get(getDomain()); if (retryDelay != null) { - System.err.println("Ensembl REST service rate limit exceeded, wait " - + retryDelay + " seconds before retrying"); try { - info.retryAfter = System.currentTimeMillis() - + (1000 * Integer.valueOf(retryDelay)); - } catch (NumberFormatException e) + int retrySecs = Integer.valueOf(retryDelay); + if (retrySecs > 0 && retrySecs < 10) + { + System.err + .println("Ensembl REST service rate limit exceeded, waiting " + + retryDelay + " seconds before retrying"); + Thread.sleep(1000 * retrySecs); + } + } catch (NumberFormatException | InterruptedException e) { - System.err - .println("Unexpected value for Retry-After: " + retryDelay); + System.err.println("Error handling Retry-After: " + e.getMessage()); } } - else - { - info.retryAfter = 0; - // debug: - // System.out.println(String.format( - // "%s Ensembl requests remaining of %s (reset in %ss)", - // remaining, limit, reset)); - } } /** @@ -430,20 +431,6 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher long now = System.currentTimeMillis(); /* - * check if we are waiting for 'Retry-After' to expire - */ - if (info.retryAfter > now) - { - System.err.println("Still " + (1 + (info.retryAfter - now) / 1000) - + " secs to wait before retrying Ensembl"); - return false; - } - else - { - info.retryAfter = 0; - } - - /* * recheck if Ensembl is up if it was down, or the recheck period has elapsed */ boolean retestAvailability = (now @@ -580,18 +567,36 @@ abstract class EnsemblRestClient extends EnsemblSequenceFetcher { JSONParser jp = new JSONParser(); URL url = null; + BufferedReader br = null; + try { url = new URL( getDomain() + "/info/data?content-type=application/json"); - BufferedReader br = getHttpResponse(url, null); - JSONObject val = (JSONObject) jp.parse(br); - JSONArray versions = (JSONArray) val.get("releases"); - domainData.get(getDomain()).dataVersion = versions.get(0).toString(); + br = getHttpResponse(url, null); + if (br != null) + { + JSONObject val = (JSONObject) jp.parse(br); + JSONArray versions = (JSONArray) val.get("releases"); + domainData.get(getDomain()).dataVersion = versions.get(0) + .toString(); + } } catch (Throwable t) { System.err.println( "Error checking Ensembl data version: " + t.getMessage()); + } finally + { + if (br != null) + { + try + { + br.close(); + } catch (IOException e) + { + // ignore + } + } } } diff --git a/src/jalview/ext/ensembl/EnsemblSymbol.java b/src/jalview/ext/ensembl/EnsemblSymbol.java index 9f86731..e2e38b5 100644 --- a/src/jalview/ext/ensembl/EnsemblSymbol.java +++ b/src/jalview/ext/ensembl/EnsemblSymbol.java @@ -119,19 +119,19 @@ public class EnsemblSymbol extends EnsemblXref { for (String query : queries) { - for (Species taxon : Species.values()) + for (Species taxon : Species.getModelOrganisms()) { - if (taxon.isModelOrganism()) + URL url = getUrl(query, taxon); + if (url != null) { - URL url = getUrl(query, taxon); - if (url != null) + br = getHttpResponse(url, ids); + if (br != null) { - br = getHttpResponse(url, ids); - } - String geneId = parseSymbolResponse(br); - if (geneId != null) - { - result.add(geneId); + String geneId = parseSymbolResponse(br); + if (geneId != null) + { + result.add(geneId); + } } } } diff --git a/src/jalview/ext/ensembl/EnsemblXref.java b/src/jalview/ext/ensembl/EnsemblXref.java index c0b00b1..c002c08 100644 --- a/src/jalview/ext/ensembl/EnsemblXref.java +++ b/src/jalview/ext/ensembl/EnsemblXref.java @@ -124,8 +124,11 @@ class EnsemblXref extends EnsemblRestClient if (url != null) { br = getHttpResponse(url, ids); + if (br != null) + { + result = parseResponse(br); + } } - return (parseResponse(br)); } catch (IOException e) { // ignore @@ -168,16 +171,16 @@ class EnsemblXref extends EnsemblRestClient while (rvals.hasNext()) { JSONObject val = (JSONObject) rvals.next(); - String dbName = val.get("dbname").toString(); - if (dbName.equals(GO_GENE_ONTOLOGY)) + String dbname = val.get("dbname").toString(); + if (GO_GENE_ONTOLOGY.equals(dbname)) { continue; } String id = val.get("primary_id").toString(); - if (dbName != null && id != null) + if (dbname != null && id != null) { - dbName = DBRefUtils.getCanonicalName(dbName); - DBRefEntry dbref = new DBRefEntry(dbName, getXRefVersion(), id); + dbname = DBRefUtils.getCanonicalName(dbname); + DBRefEntry dbref = new DBRefEntry(dbname, getXRefVersion(), id); result.add(dbref); } } diff --git a/test/jalview/ext/ensembl/EnsemblRestClientTest.java b/test/jalview/ext/ensembl/EnsemblRestClientTest.java index 31001da..cc3a3db 100644 --- a/test/jalview/ext/ensembl/EnsemblRestClientTest.java +++ b/test/jalview/ext/ensembl/EnsemblRestClientTest.java @@ -20,21 +20,40 @@ */ package jalview.ext.ensembl; +import static org.testng.Assert.assertTrue; + import jalview.datamodel.AlignmentI; import java.net.MalformedURLException; import java.net.URL; import java.util.List; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; public class EnsemblRestClientTest { + private EnsemblRestClient sf; @Test(suiteName = "live") public void testIsEnsemblAvailable() { - EnsemblRestClient sf = new EnsemblRestClient() + boolean isAvailable = sf.isEnsemblAvailable(); + if (isAvailable) + { + System.out.println("Ensembl is UP!"); + } + else + { + System.err + .println("Ensembl is DOWN or unreachable ******************* BAD!"); + } + } + + @BeforeMethod(alwaysRun = true) + protected void setUp() + { + sf = new EnsemblRestClient() { @Override @@ -74,16 +93,14 @@ public class EnsemblRestClientTest } }; - boolean isAvailable = sf.isEnsemblAvailable(); - if (isAvailable) - { - System.out.println("Ensembl is UP!"); - } - else + } + + @Test(groups = "Network") + public void testCheckEnsembl_overload() + { + for (int i = 0; i < 20; i++) { - System.err - .println("Ensembl is DOWN or unreachable ******************* BAD!"); + assertTrue(sf.checkEnsembl(), "Error on " + (i + 1) + "th ping"); } } - } -- 1.7.10.2