[icedtea-web] RFC: Use timeouts for checking if cache is fresh

Omair Majid omajid at redhat.com
Fri May 2 21:06:54 UTC 2014


Hi,

IcedTea-Web tries to check that the cached content is up-to-date by
connecting to the server and doing HEAD requests. The problem is, some
servers are too slow and checking that the cache is indeed up-to-date
takes a very long time. One particular server was taking about 7 seconds
to respond to each request, just to tell us that the cache was indeed
up-to-date.

The attached patch adds some basic (and I suspect incomplete) timeout
functionality. IcedTea-Web will assume the cache is up-to-date if the
server doesn't reply in a second or so. This significantly speeds up
running cached applications.

The proprietary javaws does something similar, as indicated at:
http://docs.oracle.com/javase/7/docs/technotes/guides/javaws/enhancements6.html

Thoughts?

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681
-------------- next part --------------
diff --git a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
@@ -26,6 +26,7 @@
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
+import java.net.SocketTimeoutException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.security.AccessController;
@@ -35,6 +36,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Pack200;
 import java.util.jar.Pack200.Unpacker;
@@ -42,6 +44,7 @@
 
 import net.sourceforge.jnlp.DownloadOptions;
 import net.sourceforge.jnlp.Version;
+import net.sourceforge.jnlp.config.DeploymentConfiguration;
 import net.sourceforge.jnlp.event.DownloadEvent;
 import net.sourceforge.jnlp.event.DownloadListener;
 import net.sourceforge.jnlp.runtime.JNLPRuntime;
@@ -101,6 +104,8 @@
     // separately locks on (in order of aquire order, ie, sync on prefetch never syncs on lock):
     //   lock, prefetch, this.resources, each resource, listeners
 
+    public static final long NO_TIMEOUT = 0;
+
     /** notified on initialization or download of a resource */
     private static final Object lock = new Object(); // used to lock static structures
 
@@ -147,11 +152,17 @@
     /** whether to download parts before requested */
     private boolean prefetch;
 
+    private long timeOutInMillis = NO_TIMEOUT;
+
     /**
      * Creates a resource tracker that does not prefetch resources.
      */
     public ResourceTracker() {
-        this(false);
+        this(false, Long.valueOf(JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_UPDATE_TIMEOUT)));
+    }
+
+    public ResourceTracker(boolean prefetch) {
+        this(prefetch, Long.valueOf(JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_UPDATE_TIMEOUT)));
     }
 
     /**
@@ -159,7 +170,7 @@
      *
      * @param prefetch whether to download resources before requested.
      */
-    public ResourceTracker(boolean prefetch) {
+    public ResourceTracker(boolean prefetch, long timeoutInMillis) {
         this.prefetch = prefetch;
 
         if (prefetch) {
@@ -168,6 +179,8 @@
                 prefetchTrackers.trimToSize();
             }
         }
+
+        this.timeOutInMillis = timeoutInMillis;
     }
 
     /**
@@ -794,20 +807,41 @@
         try {
             File localFile = CacheUtil.getCacheFile(resource.location, resource.downloadVersion);
 
-            // connect
-            URL finalLocation = findBestUrl(resource);
-
-            if (finalLocation == null) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Attempted to download " + resource.location + ", but failed to connect!");
-                throw new NullPointerException("finalLocation == null"); // Caught below
+            long timeout = timeOutInMillis;
+            if (resource.getUpdatePolicy() == UpdatePolicy.FORCE) {
+                timeout = NO_TIMEOUT;
             }
 
-            resource.setDownloadLocation(finalLocation);
-            URLConnection connection = finalLocation.openConnection(); // this won't change so should be okay unsynchronized
-            connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
+            URL finalLocation = findBestUrl(resource, timeout);
 
-            int size = connection.getContentLength();
-            boolean current = CacheUtil.isCurrent(resource.location, resource.requestVersion, connection) && resource.getUpdatePolicy() != UpdatePolicy.FORCE;
+            boolean current = false;
+            long size = -1;
+            URLConnection connection = null;
+
+            // if timeout or can't connect, use the cache
+            if (finalLocation == null) {
+                current = CacheUtil.isCached(resource.location, resource.requestVersion);
+                if (current) {
+                    size = entry.getContentLength();
+                }
+            }
+
+            if (!current) {
+                final long NO_TIMEOUT = 0;
+                finalLocation = findBestUrl(resource, NO_TIMEOUT);
+                if (finalLocation == null) {
+                    throw new IOException("Unable to connect to URL");
+                }
+
+                resource.setDownloadLocation(finalLocation);
+                connection = finalLocation.openConnection(); // this won't change so should be okay unsynchronized
+                connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
+
+                size = connection.getContentLength();
+
+                current = CacheUtil.isCurrent(resource.location, resource.requestVersion, connection) && resource.getUpdatePolicy() != UpdatePolicy.FORCE;
+            }
+
             if (!current) {
                 if (entry.isCached()) {
                     entry.markForDelete();
@@ -824,7 +858,7 @@
             synchronized (resource) {
                 resource.localFile = localFile;
                 // resource.connection = connection;
-                resource.size = size;
+                resource.size = (int) size;
                 resource.changeStatus(CONNECT | CONNECTING, CONNECTED);
 
                 // check if up-to-date; if so set as downloaded
@@ -870,7 +904,7 @@
      * @throws IOException
      */
     static int getUrlResponseCode(URL url, Map<String, String> requestProperties, String requestMethod) throws IOException {
-        return getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod).result;
+        return getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod, NO_TIMEOUT).result;
     }
 
     private static class CodeWithRedirect {
@@ -895,12 +929,16 @@
      * Connects to the given URL, and grabs a response code and redirecton if
      * the URL uses the HTTP protocol, or returns an arbitrary valid HTTP
      * response code.
+     * 
+     * @param timeoutInMillis the timeout in milliseconds. {@link #NO_TIMEOUT}
+     * means no timeout.
      *
      * @return the response code if HTTP connection and redirection value, or
-     * HttpURLConnection.HTTP_OK and null if not.
+     * HttpURLConnection.HTTP_OK and null if not. In case of timeout, 
+     * {@code null} is returned.
      * @throws IOException
      */
-    static CodeWithRedirect getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, String requestMethod) throws IOException {
+    static CodeWithRedirect getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, String requestMethod, long timeoutInMillis) throws IOException {
         CodeWithRedirect result = new CodeWithRedirect();
         URLConnection connection = url.openConnection();
 
@@ -911,14 +949,18 @@
         if (connection instanceof HttpURLConnection) {
             HttpURLConnection httpConnection = (HttpURLConnection) connection;
             httpConnection.setRequestMethod(requestMethod);
+            httpConnection.setReadTimeout((int)timeoutInMillis);
 
-            int responseCode = httpConnection.getResponseCode();
+            try {
+                int responseCode = httpConnection.getResponseCode();
+                /* Fully consuming current request helps with connection re-use 
+                 * See http://docs.oracle.com/javase/1.5.0/docs/guide/net/http-keepalive.html */
+                HttpUtils.consumeAndCloseConnectionSilently(httpConnection);
 
-            /* Fully consuming current request helps with connection re-use 
-             * See http://docs.oracle.com/javase/1.5.0/docs/guide/net/http-keepalive.html */
-            HttpUtils.consumeAndCloseConnectionSilently(httpConnection);
-
-            result.result = responseCode;
+                result.result = responseCode;
+            } catch (SocketTimeoutException ste) {
+                return null;
+            }
         }
 
         Map<String, List<String>> header = connection.getHeaderFields();
@@ -945,16 +987,18 @@
      * and packing, if possible.
      *
      * @param resource the resource
-     * @return the best URL, or null if all failed to resolve
+     * @param timeoutInMillis the timeout before the server is declared
+     *      non-responsive. To avoid any timeout, use {@link #NO_TIMEOUT}.
+     * @return the best URL, or null if all failed to resolve, or server is too slow
      */
-     URL findBestUrl(Resource resource) {
+     URL findBestUrl(Resource resource, long timeoutInMillis) {
         DownloadOptions options = downloadOptions.get(resource);
         if (options == null) {
             options = new DownloadOptions(false, false);
         }
 
         List<URL> urls = new ResourceUrlCreator(resource, options).getUrls();
-         OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "All possible urls for "
+        OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "All possible urls for "
                  + resource.toString() + " : " + urls);
         
          for (String requestMethod : requestMethods) {
@@ -964,7 +1008,11 @@
                      Map<String, String> requestProperties = new HashMap<String, String>();
                      requestProperties.put("Accept-Encoding", "pack200-gzip, gzip");
 
-                     CodeWithRedirect response = getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod);
+                     CodeWithRedirect response = getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod, timeoutInMillis);
+                     if (response == null) {
+                         return null;
+                     }
+
                      if (response.shouldRedirect()){
                          if (response.URL == null) {
                              OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Although " + resource.toString() + " got redirect " + response.result + " code for " + requestMethod + " request for " + url.toExternalForm() + " the target was null. Not following");
@@ -1180,8 +1228,9 @@
                         }
                     }
                 }
-                if (finished)
+                if (finished) {
                     return true;
+                }
 
                 // wait
                 long waitTime = 0;


More information about the distro-pkg-dev mailing list