[tor-commits] [metrics-lib/master] Use timeouts in URLConnection instead of own timeout thread.

karsten at torproject.org karsten at torproject.org
Mon Jan 23 18:43:20 UTC 2012


commit 8c14bac87e3623bf7d8c1e04f9edc84c5a2a64c0
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date:   Mon Jan 23 19:40:26 2012 +0100

    Use timeouts in URLConnection instead of own timeout thread.
---
 .../torproject/descriptor/DescriptorRequest.java   |   10 +++-
 .../descriptor/RelayDescriptorDownloader.java      |   12 ++++-
 .../descriptor/example/ConsensusHealthChecker.java |   10 +++--
 .../example/MetricsRelayDescriptorAggregator.java  |    9 ++--
 .../descriptor/impl/DescriptorRequestImpl.java     |   11 ++++-
 .../descriptor/impl/DirectoryDownloader.java       |   44 ++++---------------
 .../descriptor/impl/DownloadCoordinatorImpl.java   |   15 ++++--
 .../impl/RelayDescriptorDownloaderImpl.java        |   30 ++++++++++----
 8 files changed, 77 insertions(+), 64 deletions(-)

diff --git a/src/org/torproject/descriptor/DescriptorRequest.java b/src/org/torproject/descriptor/DescriptorRequest.java
index f68332f..422d133 100644
--- a/src/org/torproject/descriptor/DescriptorRequest.java
+++ b/src/org/torproject/descriptor/DescriptorRequest.java
@@ -29,12 +29,16 @@ public interface DescriptorRequest {
   /* Return the time in millis when this request ended. */
   public long getRequestEnd();
 
-  /* Return whether this request ended, because the request timeout
+  /* Return whether this request ended, because the connect timeout has
    * expired. */
-  public boolean requestTimeoutHasExpired();
+  public boolean connectTimeoutHasExpired();
+
+  /* Return whether this request ended, because the read timeout has
+   * expired. */
+  public boolean readTimeoutHasExpired();
 
   /* Return whether this request ended, because the global timeout for all
-   * requests expired. */
+   * requests has expired. */
   public boolean globalTimeoutHasExpired();
 
   /* Return the descriptors contained in the reply. */
diff --git a/src/org/torproject/descriptor/RelayDescriptorDownloader.java b/src/org/torproject/descriptor/RelayDescriptorDownloader.java
index 28d6ea1..754c644 100644
--- a/src/org/torproject/descriptor/RelayDescriptorDownloader.java
+++ b/src/org/torproject/descriptor/RelayDescriptorDownloader.java
@@ -74,11 +74,17 @@ public interface RelayDescriptorDownloader {
    * descriptors. */
   public void setExcludeExtraInfoDescriptors(Set<String> identifiers);
 
-  /* Define a request timeout for a single request.  If a timeout expires,
+  /* Define a connect timeout for a single request.  If a timeout expires,
    * no further requests will be sent to the directory authority or
-   * mirror.  Setting this value to 0 disables the request timeout.
+   * mirror.  Setting this value to 0 disables the connect timeout.
    * Default value is 1 minute (60 * 1000). */
-  public void setRequestTimeout(long requestTimeoutMillis);
+  public void setConnectTimeout(long connectTimeoutMillis);
+
+  /* Define a read timeout for a single request.  If a timeout expires,
+   * no further requests will be sent to the directory authority or
+   * mirror.  Setting this value to 0 disables the read timeout.
+   * Default value is 1 minute (60 * 1000). */
+  public void setReadTimeout(long readTimeoutMillis);
 
   /* Define a global timeout for all requests.  Once this timeout expires,
    * all running requests are aborted and no further requests are made.
diff --git a/src/org/torproject/descriptor/example/ConsensusHealthChecker.java b/src/org/torproject/descriptor/example/ConsensusHealthChecker.java
index 178f164..b8cc12b 100644
--- a/src/org/torproject/descriptor/example/ConsensusHealthChecker.java
+++ b/src/org/torproject/descriptor/example/ConsensusHealthChecker.java
@@ -38,9 +38,10 @@ public class ConsensusHealthChecker {
     downloader.setIncludeCurrentConsensusFromAllDirectoryAuthorities();
     downloader.setIncludeCurrentReferencedVotes();
 
-    /* Set a request timeout of 1 minute and a global timeout of 10
-     * minutes to avoid being blocked forever by a slow download. */
-    downloader.setRequestTimeout(60L * 1000L);
+    /* Set connect and read timeouts of 1 minute each and a global timeout
+     * of 10 minutes to avoid being blocked forever by a slow download. */
+    downloader.setConnectTimeout(60L * 1000L);
+    downloader.setReadTimeout(60L * 1000L);
     downloader.setGlobalTimeout(10L * 60L * 1000L);
 
     /* Run the previously configured downloads and iterate over the
@@ -60,7 +61,8 @@ public class ConsensusHealthChecker {
             + "more consensuses and/or votes and cannot make a good "
             + "statement about the consensus health.  Exiting.");
         return;
-      } else if (request.requestTimeoutHasExpired()) {
+      } else if (request.connectTimeoutHasExpired() ||
+          request.readTimeoutHasExpired()) {
         System.out.println("The request to directory authority "
             + request.getDirectoryNickname() + " to download the "
             + "descriptor(s) at " + request.getRequestUrl() + " has "
diff --git a/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java b/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java
index 0d9da8d..bfaaa22 100644
--- a/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java
+++ b/src/org/torproject/descriptor/example/MetricsRelayDescriptorAggregator.java
@@ -99,10 +99,11 @@ public class MetricsRelayDescriptorAggregator {
     downloader.setExcludeExtraInfoDescriptors(
         knownExtraInfoDescriptorIdentifiers.keySet());
 
-    /* Set a request timeout of 2 minutes and a global timeout of 1 hour
-     * to avoid being blocked forever by a slow download, but also to
-     * avoid giving up too quickly. */
-    downloader.setRequestTimeout(2L * 60L * 1000L);
+    /* Set connect and read timeouts of 2 minutes each and a global
+     * timeout of 1 hour to avoid being blocked forever by a slow
+     * download, but also to avoid giving up too quickly. */
+    downloader.setConnectTimeout(2L * 60L * 1000L);
+    downloader.setReadTimeout(2L * 60L * 1000L);
     downloader.setGlobalTimeout(60L * 60L * 1000L);
 
     /* Download descriptors and process them. */
diff --git a/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java b/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java
index 6c44a79..e5c2ff5 100644
--- a/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java
+++ b/src/org/torproject/descriptor/impl/DescriptorRequestImpl.java
@@ -70,9 +70,14 @@ public class DescriptorRequestImpl implements DescriptorRequest {
     return this.requestEnd;
   }
 
-  private boolean requestTimeoutHasExpired;
-  public boolean requestTimeoutHasExpired() {
-    return this.requestTimeoutHasExpired;
+  private boolean connectTimeoutHasExpired;
+  public boolean connectTimeoutHasExpired() {
+    return this.connectTimeoutHasExpired;
+  }
+
+  private boolean readTimeoutHasExpired;
+  public boolean readTimeoutHasExpired() {
+    return this.readTimeoutHasExpired;
   }
 
   private boolean globalTimeoutHasExpired;
diff --git a/src/org/torproject/descriptor/impl/DirectoryDownloader.java b/src/org/torproject/descriptor/impl/DirectoryDownloader.java
index 7b4311c..7e9d19c 100644
--- a/src/org/torproject/descriptor/impl/DirectoryDownloader.java
+++ b/src/org/torproject/descriptor/impl/DirectoryDownloader.java
@@ -27,9 +27,14 @@ public class DirectoryDownloader implements Runnable {
     this.downloadCoordinator = downloadCoordinator;
   }
 
-  private long requestTimeout;
-  protected void setRequestTimeout(long requestTimeout) {
-    this.requestTimeout = requestTimeout;
+  private long connectTimeout;
+  protected void setConnectTimeout(long connectTimeout) {
+    this.connectTimeout = connectTimeout;
+  }
+
+  private long readTimeout;
+  protected void setReadTimeout(long readTimeout) {
+    this.readTimeout = readTimeout;
   }
 
   public void run() {
@@ -41,13 +46,12 @@ public class DirectoryDownloader implements Runnable {
         String url = "http://" + this.ipPort
             + request.getRequestedResource();
         request.setRequestStart(System.currentTimeMillis());
-        Thread timeoutThread = new Thread(new RequestTimeout(
-            this.requestTimeout));
-        timeoutThread.start();
         try {
           URL u = new URL(url);
           HttpURLConnection huc =
               (HttpURLConnection) u.openConnection();
+          huc.setConnectTimeout((int) this.connectTimeout);
+          huc.setReadTimeout((int) this.readTimeout);
           huc.setRequestMethod("GET");
           huc.connect();
           int responseCode = huc.getResponseCode();
@@ -74,39 +78,11 @@ public class DirectoryDownloader implements Runnable {
            * problems, e.g., refused connections. */
           keepRunning = false;
         }
-        /* TODO How do we find out if we were interrupted, and by who?
-         * Set the request or global timeout flag in the response. */
-        timeoutThread.interrupt();
         this.downloadCoordinator.deliverResponse(request);
       } else {
         keepRunning = false;
       }
     } while (keepRunning);
   }
-
-  /* Interrupt a download request if it takes longer than a given time. */
-  /* TODO Also look at URLConnection.setConnectTimeout() and
-   * URLConnection.setReadTimeout() instead of implementing this
-   * ourselves. */
-  private static class RequestTimeout implements Runnable {
-    private long timeoutMillis;
-    private Thread downloaderThread;
-    private RequestTimeout(long timeoutMillis) {
-      this.downloaderThread = Thread.currentThread();
-      this.timeoutMillis = timeoutMillis;
-    }
-    public void run() {
-      long started = System.currentTimeMillis(), sleep;
-      while ((sleep = started + this.timeoutMillis
-          - System.currentTimeMillis()) > 0L) {
-        try {
-          Thread.sleep(sleep);
-        } catch (InterruptedException e) {
-          return;
-        }
-      }
-      this.downloaderThread.interrupt();
-    }
-  }
 }
 
diff --git a/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java b/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java
index e91fd8b..03e7997 100644
--- a/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java
+++ b/src/org/torproject/descriptor/impl/DownloadCoordinatorImpl.java
@@ -27,7 +27,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator {
   private SortedMap<String, DirectoryDownloader> directoryMirrors;
   private boolean downloadConsensusFromAllAuthorities;
   private boolean includeCurrentReferencedVotes;
-  private long requestTimeoutMillis;
+  private long connectTimeoutMillis;
+  private long readTimeoutMillis;
   private long globalTimeoutMillis;
 
   protected DownloadCoordinatorImpl(
@@ -36,7 +37,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator {
       boolean downloadConsensus,
       boolean downloadConsensusFromAllAuthorities,
       Set<String> downloadVotes, boolean includeCurrentReferencedVotes,
-      long requestTimeoutMillis, long globalTimeoutMillis) {
+      long connectTimeoutMillis, long readTimeoutMillis,
+      long globalTimeoutMillis) {
     this.directoryAuthorities = directoryAuthorities;
     this.directoryMirrors = directoryMirrors;
     this.missingConsensus = downloadConsensus;
@@ -44,7 +46,8 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator {
         downloadConsensusFromAllAuthorities;
     this.missingVotes = downloadVotes;
     this.includeCurrentReferencedVotes = includeCurrentReferencedVotes;
-    this.requestTimeoutMillis = requestTimeoutMillis;
+    this.connectTimeoutMillis = connectTimeoutMillis;
+    this.readTimeoutMillis = readTimeoutMillis;
     this.globalTimeoutMillis = globalTimeoutMillis;
     if (this.directoryMirrors.isEmpty() &&
         this.directoryAuthorities.isEmpty()) {
@@ -59,13 +62,15 @@ public class DownloadCoordinatorImpl implements DownloadCoordinator {
       for (DirectoryDownloader directoryMirror :
           this.directoryMirrors.values()) {
         directoryMirror.setDownloadCoordinator(this);
-        directoryMirror.setRequestTimeout(this.requestTimeoutMillis);
+        directoryMirror.setConnectTimeout(this.connectTimeoutMillis);
+        directoryMirror.setReadTimeout(this.readTimeoutMillis);
         new Thread(directoryMirror).start();
       }
       for (DirectoryDownloader directoryAuthority :
           this.directoryAuthorities.values()) {
         directoryAuthority.setDownloadCoordinator(this);
-        directoryAuthority.setRequestTimeout(this.requestTimeoutMillis);
+        directoryAuthority.setConnectTimeout(this.connectTimeoutMillis);
+        directoryAuthority.setReadTimeout(this.readTimeoutMillis);
         new Thread(directoryAuthority).start();
       }
     }
diff --git a/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java b/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java
index 7cdcc0c..0888d64 100644
--- a/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java
+++ b/src/org/torproject/descriptor/impl/RelayDescriptorDownloaderImpl.java
@@ -193,18 +193,32 @@ public class RelayDescriptorDownloaderImpl
         + "descriptors is not implemented yet.");
   }
 
-  private long requestTimeoutMillis = 60L * 1000L;
-  public void setRequestTimeout(long requestTimeoutMillis) {
+  private long readTimeoutMillis = 60L * 1000L;
+  public void setReadTimeout(long readTimeoutMillis) {
     if (this.hasStartedDownloading) {
       throw new IllegalStateException("Reconfiguration is not permitted "
           + "after starting to download.");
     }
-    if (requestTimeoutMillis < 0L) {
-      throw new IllegalArgumentException("Request timeout value "
-          + String.valueOf(requestTimeoutMillis) + " may not be "
+    if (readTimeoutMillis < 0L) {
+      throw new IllegalArgumentException("Read timeout value "
+          + String.valueOf(readTimeoutMillis) + " may not be "
           + "negative.");
     }
-    this.requestTimeoutMillis = requestTimeoutMillis;
+    this.readTimeoutMillis = readTimeoutMillis;
+  }
+
+  private long connectTimeoutMillis = 60L * 1000L;
+  public void setConnectTimeout(long connectTimeoutMillis) {
+    if (this.hasStartedDownloading) {
+      throw new IllegalStateException("Reconfiguration is not permitted "
+          + "after starting to download.");
+    }
+    if (connectTimeoutMillis < 0L) {
+      throw new IllegalArgumentException("Connect timeout value "
+          + String.valueOf(connectTimeoutMillis) + " may not be "
+          + "negative.");
+    }
+    this.connectTimeoutMillis = connectTimeoutMillis;
   }
 
   private long globalTimeoutMillis = 60L * 60L * 1000L;
@@ -231,8 +245,8 @@ public class RelayDescriptorDownloaderImpl
         new DownloadCoordinatorImpl(this.directoryAuthorities,
         this.directoryMirrors, this.downloadConsensus,
         this.downloadConsensusFromAllAuthorities, this.downloadVotes,
-        this.includeCurrentReferencedVotes, this.requestTimeoutMillis,
-        this.globalTimeoutMillis);
+        this.includeCurrentReferencedVotes, this.connectTimeoutMillis,
+        this.readTimeoutMillis, this.globalTimeoutMillis);
     Iterator<DescriptorRequest> descriptorQueue = downloadCoordinator.
         getDescriptorQueue();
     return descriptorQueue;



More information about the tor-commits mailing list