[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] [metrics-web/master] Tweak custom graphs and make LRU cache work.



Author: Karsten Loesing <karsten.loesing@xxxxxxx>
Date: Wed, 22 Sep 2010 11:36:17 +0200
Subject: Tweak custom graphs and make LRU cache work.
Commit: 4a42230c0c82b56a6fe9cdee3b249e60c7689ac7

---
 etc/ernie.properties                               |   11 +-
 src/org/torproject/ernie/web/GraphController.java  |  216 +++++++++++---------
 .../ernie/web/NetworkSizeImageServlet.java         |  106 +++++++---
 .../ernie/web/RelayBandwidthImageServlet.java      |  106 +++++++---
 .../ernie/web/RelayPlatformsImageServlet.java      |  107 +++++++----
 .../ernie/web/RelayVersionsImageServlet.java       |  106 +++++++---
 war/WEB-INF/templates/graphs_custom-graph.tpl.jsp  |    3 +-
 7 files changed, 416 insertions(+), 239 deletions(-)

diff --git a/etc/ernie.properties b/etc/ernie.properties
index eec6052..1f8cf92 100644
--- a/etc/ernie.properties
+++ b/etc/ernie.properties
@@ -7,11 +7,14 @@ rserve.host=localhost
 # The Rserve port
 rserve.port=6311
 
-# How many graphs to keep cached
-max.cached.graphs=5000
+# How many graphs to keep cached at most
+max.cache.size=5000
 
-# How many requests between cache clears
-clear.cache.requests=30
+# How many graphs to keep cached at least
+min.cache.size=2500
+
+# Maximum age in seconds of a graph in the cache
+max.cache.age=21600
 
 # Directory to store cached graph. Must be writable by tomcat
 # and rserve processes
diff --git a/src/org/torproject/ernie/web/GraphController.java b/src/org/torproject/ernie/web/GraphController.java
index bacfc2a..88570b3 100644
--- a/src/org/torproject/ernie/web/GraphController.java
+++ b/src/org/torproject/ernie/web/GraphController.java
@@ -1,129 +1,151 @@
 package org.torproject.ernie.web;
 
-import org.torproject.ernie.util.ErnieProperties;
-import org.apache.log4j.Logger;
-import javax.servlet.*;
-import javax.servlet.http.*;
 import java.io.*;
 import java.util.*;
+
 import org.rosuda.REngine.Rserve.*;
 import org.rosuda.REngine.*;
 
+import org.torproject.ernie.util.ErnieProperties;
+
 public class GraphController {
 
-  private static final Logger log;
-  private static final String baseDir;
-  private static final int cacheSize;
-  private final String graphName;
-  private static int cacheClearRequests;
-  private static int requests;
+  /* Singleton instance and getInstance method of this class. */
+  private static GraphController instance = new GraphController();
+  public static GraphController getInstance() {
+    return instance;
+  }
+
+  /* Host and port where Rserve is listening. */
+  private String rserveHost;
+  private int rservePort;
 
-  private static final int rservePort;
-  private static final String rserveHost;
+  /* Some parameters for our cache of graph images. */
+  private String cachedGraphsDirectory;
+  private int maxCacheSize;
+  private int minCacheSize;
+  private long maxCacheAge;
+  private int currentCacheSize;
+  private long oldestGraph;
 
-  static {
-    log = Logger.getLogger(GraphController.class.toString());
+  protected GraphController ()  {
+
+    /* Read properties from property file. */
     ErnieProperties props = new ErnieProperties();
-    cacheSize = props.getInt("max.cached.graphs");
-    baseDir = props.getProperty("cached.graphs.dir");
-    cacheClearRequests = props.getInt("cache.clear.requests");
-    rservePort = props.getInt("rserve.port");
-    rserveHost = props.getProperty("rserve.host");
-    requests = 0;
+    this.cachedGraphsDirectory = props.getProperty("cached.graphs.dir");
+    this.maxCacheSize = props.getInt("max.cache.size");
+    this.minCacheSize = props.getInt("min.cache.size");
+    this.maxCacheAge = (long) props.getInt("max.cache.age");
+    this.rserveHost = props.getProperty("rserve.host");
+    this.rservePort = props.getInt("rserve.port");
 
-    try {
-      /* Create temp graphs directory if it doesn't exist. */
-      File dir = new File(baseDir);
-      if (!dir.exists())  {
-        dir.mkdirs();
-      }
+    /* Clean up cache on startup. */
+    this.cleanUpCache();
+  }
 
-      /* Change directory permissions to allow it to be written to
-       * by Rserve. */
-      Runtime rt = Runtime.getRuntime();
-      rt.exec("chmod 777 " + baseDir).waitFor();
-    } catch (InterruptedException e) {
-    } catch (IOException e) {
-      log.warn("Couldn't create temporary graphs directory. " + e);
+  /* Generate a graph using the given R query that has a placeholder for
+   * the absolute path to the image to be created. */
+  public byte[] generateGraph(String rQuery, String imageFilename) {
+
+    /* Check if we need to clean up the cache first, or we might give
+     * someone an old grpah. */
+    if (this.currentCacheSize > this.maxCacheSize ||
+        (this.currentCacheSize > 0 && System.currentTimeMillis()
+        - this.oldestGraph > this.maxCacheAge * 1000L)) {
+      this.cleanUpCache();
     }
-  }
 
-  public GraphController (String graphName)  {
-    this.graphName = graphName;
-  }
+    /* See if we need to generate this graph. */
+    File imageFile = new File(this.cachedGraphsDirectory + "/"
+        + imageFilename);
+    if (!imageFile.exists()) {
 
-  public void writeOutput(String imagePath, HttpServletRequest request,
-      HttpServletResponse response) throws IOException {
+      /* We do. Update the R query to contain the absolute path to the file
+       * to be generated, create a connection to Rserve, run the R query,
+       * and close the connection. The generated graph will be on disk. */
+      rQuery = String.format(rQuery, imageFile.getAbsolutePath());
+      try {
+        RConnection rc = new RConnection(rserveHost, rservePort);
+        rc.eval(rQuery);
+        rc.close();
+      } catch (RserveException e) {
+        return null;
+      }
 
-    /* Read file from disk and write it to response. */
-    BufferedInputStream input = null;
-    BufferedOutputStream output = null;
-    try {
-      File imageFile = new File(imagePath);
-      /* If there was an error when generating the graph,
-       * set the header to 400 bad request. */
-      if (!imageFile.exists())  {
-        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
-      } else {
-        response.setContentType("image/png");
-        response.setHeader("Content-Length", String.valueOf(
-            imageFile.length()));
-        response.setHeader("Content-Disposition",
-            "inline; filename=\"" + graphName + ".png" + "\"");
-        input = new BufferedInputStream(new FileInputStream(imageFile),
-            1024);
-        output = new BufferedOutputStream(response.getOutputStream(), 1024);
-        byte[] buffer = new byte[1024];
-        int length;
-        while ((length = input.read(buffer)) > 0) {
-            output.write(buffer, 0, length);
-        }
-        requests++;
-        if (requests % cacheClearRequests == 0) {
-          deleteLRUgraph();
-        }
+      /* Check that we really just generated the file */
+      if (!imageFile.exists()) {
+        return null;
       }
+
+      /* Update our graph counter. */
+      this.currentCacheSize++;
     }
-    finally {
-      if (output != null)
-        output.close();
-      if (input != null)
-        input.close();
-    }
-  }
 
-  public void generateGraph(String rquery, String path)  {
+    /* Read the image from disk and write it to a byte array. */
+    byte[] result = null;
     try {
-      File f = new File(path);
-      if (!f.exists())  {
-        RConnection rc = new RConnection(rserveHost, rservePort);
-        rc.eval(rquery);
-        rc.close();
+      BufferedInputStream bis = new BufferedInputStream(
+          new FileInputStream(imageFile), 1024);
+      ByteArrayOutputStream baos = new ByteArrayOutputStream();
+      byte[] buffer = new byte[1024];
+      int length;
+      while ((length = bis.read(buffer)) > 0) {
+        baos.write(buffer, 0, length);
       }
-    } catch (Exception e) {
-      log.warn("Internal Rserve error. Couldn't generate graph: " +
-          e.toString());
+      result = baos.toByteArray();
+    } catch (IOException e) {
+      return null;
     }
+
+    /* Return the graph bytes. */
+    return result;
   }
 
-  /* Caching mechanism to delete the least recently
-   * used graph every X requests.
-   * TODO We're not really deleting the least recently used graphs here,
-   * but a random sample. Not the end of the world, but when we're bored,
-   * let's fix this. */
-  public void deleteLRUgraph()  {
-    File dir = new File(baseDir);
-    List<File> flist = Arrays.asList(dir.listFiles());
-    if (flist.size() > (cacheSize + cacheClearRequests))  {
-      Collections.sort(flist);
-      for (int i = 0; i <= cacheClearRequests; i++) {
-        flist.get(i).delete();
+  /* Clean up graph cache by removing all graphs older than maxCacheAge
+   * and then the oldest graphs until we have minCacheSize graphs left.
+   * Also update currentCacheSize and oldestGraph. */
+  public void cleanUpCache() {
+
+    /* Check if the cache is empty first. */
+    File[] filesInCache = new File(this.cachedGraphsDirectory).
+        listFiles();
+    if (filesInCache.length == 0) {
+      this.currentCacheSize = 0;
+      this.oldestGraph = System.currentTimeMillis();
+      return;
+    }
+
+    /* Sort graphs in cache by the time they were last modified. */
+    List<File> graphsByLastModified = new LinkedList<File>(
+        Arrays.asList(filesInCache));
+    Collections.sort(graphsByLastModified, new Comparator<File>() {
+      public int compare(File a, File b) {
+        return a.lastModified() < b.lastModified() ? -1 :
+            a.lastModified() > b.lastModified() ? 1 : 0;
       }
+    });
+
+    /* Delete the graphs that are either older than maxCacheAge and then
+     * as many graphs as necessary to shrink to minCacheSize graphs. */
+    long cutOffTime = System.currentTimeMillis()
+        - this.maxCacheAge * 1000L;
+    while (!graphsByLastModified.isEmpty()) {
+      File oldestGraphInList = graphsByLastModified.remove(0);
+      if (oldestGraphInList.lastModified() >= cutOffTime ||
+          graphsByLastModified.size() < this.minCacheSize) {
+        break;
+      }
+      oldestGraphInList.delete();
     }
-  }
 
-  public String getBaseDir()  {
-    return this.baseDir;
+    /* Update currentCacheSize and oldestGraph that we need to decide when
+     * we should next clean up the graph cache. */
+    this.currentCacheSize = graphsByLastModified.size();
+    if (!graphsByLastModified.isEmpty()) {
+      this.oldestGraph = graphsByLastModified.get(0).lastModified();
+    } else {
+      this.oldestGraph = System.currentTimeMillis();
+    }
   }
 }
 
diff --git a/src/org/torproject/ernie/web/NetworkSizeImageServlet.java b/src/org/torproject/ernie/web/NetworkSizeImageServlet.java
index 78c24e4..c31da86 100644
--- a/src/org/torproject/ernie/web/NetworkSizeImageServlet.java
+++ b/src/org/torproject/ernie/web/NetworkSizeImageServlet.java
@@ -1,61 +1,99 @@
 package org.torproject.ernie.web;
 
-import java.util.*;
-import java.text.*;
 import java.io.*;
+import java.text.*;
+import java.util.*;
 import javax.servlet.*;
 import javax.servlet.http.*;
-import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.log4j.Logger;
+
+/* TODO This class shares a lot of code with the other *ImageServlet
+ * classes. We should at some point try harder to reuse code. But let's
+ * wait until we know what parameters besides start and end time will be
+ * shared between these classes. We'll likely want to add more parameters
+ * that reduce the code that can be re-used between servlets. */
 
 public class NetworkSizeImageServlet extends HttpServlet {
 
-  private static final Logger log;
-  private final String rquery;
-  private final String graphName;
-  private final GraphController gcontroller;
-  private SimpleDateFormat simpledf;
+  private GraphController graphController;
 
-  static {
-    log = Logger.getLogger(NetworkSizeImageServlet.class);
-  }
+  private SimpleDateFormat dateFormat;
 
   public NetworkSizeImageServlet()  {
-    this.graphName = "networksize";
-    this.gcontroller = new GraphController(graphName);
-    this.rquery = "plot_networksize_line('%s', '%s', '%s')";
-    this.simpledf = new SimpleDateFormat("yyyy-MM-dd");
-    this.simpledf.setTimeZone(TimeZone.getTimeZone("UTC"));
+    this.graphController = GraphController.getInstance();
+    this.dateFormat = new SimpleDateFormat("yyyy-MM-dd");
+    this.dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
 
   public void doGet(HttpServletRequest request,
       HttpServletResponse response) throws IOException,
       ServletException {
 
-    try {
-      String md5file, start, end, path, query;
-
-      start = request.getParameter("start");
-      end = request.getParameter("end");
-
-      /* Validate input */
+    /* Check parameters. */
+    String startParameter = request.getParameter("start");
+    String endParameter = request.getParameter("end");
+    if (startParameter == null && endParameter == null) {
+      /* If no parameters are given, set default date range to the past 30
+       * days. */
+      long now = System.currentTimeMillis();
+      startParameter = dateFormat.format(now
+          - 30L * 24L * 60L * 60L * 1000L);
+      endParameter = dateFormat.format(now);
+    } else if (startParameter != null && endParameter != null) {
+      long startTimestamp = -1L, endTimestamp = -1L;
       try {
-        simpledf.parse(start);
-        simpledf.parse(end);
+        startTimestamp = dateFormat.parse(startParameter).getTime();
+        endTimestamp = dateFormat.parse(endParameter).getTime();
       } catch (ParseException e)  {
         response.sendError(HttpServletResponse.SC_BAD_REQUEST);
         return;
       }
+      /* The parameters are dates. Good. Does end not precede start? */
+      if (startTimestamp > endTimestamp) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* And while we're at it, make sure both parameters lie in this
+       * century. */
+      if (!startParameter.startsWith("20") ||
+          !endParameter.startsWith("20")) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* Looks like sane parameters. Re-format them to get a canonical
+       * version, not something like 2010-1-1, 2010-01-1, etc. */
+      startParameter = dateFormat.format(startTimestamp);
+      endParameter = dateFormat.format(endTimestamp);
+    } else {
+      /* Either none or both of start and end need to be set. */
+      response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+      return;
+    }
 
-      md5file = DigestUtils.md5Hex(graphName + "-" + start + "-" + end);
-      path = gcontroller.getBaseDir() + md5file + ".png";
-
-      query = String.format(rquery, start, end, path);
-      gcontroller.generateGraph(query, path);
-      gcontroller.writeOutput(path, request, response);
+    /* Request graph from graph controller, which either returns it from
+     * its cache or asks Rserve to generate it. */
+    String imageFilename = "networksize-" + startParameter + "-"
+        + endParameter + ".png";
+    String rQuery = "plot_networksize_line('" + startParameter + "', '"
+        + endParameter + "', '%s')";
+    byte[] graphBytes = graphController.generateGraph(rQuery,
+        imageFilename);
 
-    } catch (IOException e) {
-      log.warn(e.toString());
+    /* Make sure that we have a graph to return. */
+    if (graphBytes == null) {
+      response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+      return;
     }
+
+    /* Write graph bytes to response. */
+    BufferedOutputStream output = null;
+    response.setContentType("image/png");
+    response.setHeader("Content-Length",
+        String.valueOf(graphBytes.length));
+    response.setHeader("Content-Disposition",
+        "inline; filename=\"" + imageFilename + "\"");
+    output = new BufferedOutputStream(response.getOutputStream(), 1024);
+    output.write(graphBytes, 0, graphBytes.length);
+    output.close();
   }
 }
+
diff --git a/src/org/torproject/ernie/web/RelayBandwidthImageServlet.java b/src/org/torproject/ernie/web/RelayBandwidthImageServlet.java
index 310ba67..83475a4 100644
--- a/src/org/torproject/ernie/web/RelayBandwidthImageServlet.java
+++ b/src/org/torproject/ernie/web/RelayBandwidthImageServlet.java
@@ -1,61 +1,99 @@
 package org.torproject.ernie.web;
 
-import java.util.*;
-import java.text.*;
 import java.io.*;
+import java.text.*;
+import java.util.*;
 import javax.servlet.*;
 import javax.servlet.http.*;
-import org.apache.log4j.Logger;
-import org.apache.commons.codec.digest.DigestUtils;
+
+/* TODO This class shares a lot of code with the other *ImageServlet
+ * classes. We should at some point try harder to reuse code. But let's
+ * wait until we know what parameters besides start and end time will be
+ * shared between these classes. We'll likely want to add more parameters
+ * that reduce the code that can be re-used between servlets. */
 
 public class RelayBandwidthImageServlet extends HttpServlet {
 
-  private static final Logger log;
-  private final String rquery;
-  private final String graphName;
-  private final GraphController gcontroller;
-  private SimpleDateFormat simpledf;
+  private GraphController graphController;
 
-  static {
-    log = Logger.getLogger(RelayBandwidthImageServlet.class);
-  }
+  private SimpleDateFormat dateFormat;
 
   public RelayBandwidthImageServlet()  {
-    this.graphName = "bandwidth";
-    this.gcontroller = new GraphController(graphName);
-    this.rquery = "plot_bandwidth_line('%s', '%s', '%s')";
-    this.simpledf = new SimpleDateFormat("yyyy-MM-dd");
-    this.simpledf.setTimeZone(TimeZone.getTimeZone("UTC"));
+    this.graphController = GraphController.getInstance();
+    this.dateFormat = new SimpleDateFormat("yyyy-MM-dd");
+    this.dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
 
   public void doGet(HttpServletRequest request,
       HttpServletResponse response) throws IOException,
       ServletException {
 
-    try {
-      String md5file, start, end, path, query;
-
-      start = request.getParameter("start");
-      end = request.getParameter("end");
-
-      /* Validate input */
+    /* Check parameters. */
+    String startParameter = request.getParameter("start");
+    String endParameter = request.getParameter("end");
+    if (startParameter == null && endParameter == null) {
+      /* If no parameters are given, set default date range to the past 30
+       * days. */
+      long now = System.currentTimeMillis();
+      startParameter = dateFormat.format(now
+          - 30L * 24L * 60L * 60L * 1000L);
+      endParameter = dateFormat.format(now);
+    } else if (startParameter != null && endParameter != null) {
+      long startTimestamp = -1L, endTimestamp = -1L;
       try {
-        simpledf.parse(start);
-        simpledf.parse(end);
+        startTimestamp = dateFormat.parse(startParameter).getTime();
+        endTimestamp = dateFormat.parse(endParameter).getTime();
       } catch (ParseException e)  {
         response.sendError(HttpServletResponse.SC_BAD_REQUEST);
         return;
       }
+      /* The parameters are dates. Good. Does end not precede start? */
+      if (startTimestamp > endTimestamp) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* And while we're at it, make sure both parameters lie in this
+       * century. */
+      if (!startParameter.startsWith("20") ||
+          !endParameter.startsWith("20")) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* Looks like sane parameters. Re-format them to get a canonical
+       * version, not something like 2010-1-1, 2010-01-1, etc. */
+      startParameter = dateFormat.format(startTimestamp);
+      endParameter = dateFormat.format(endTimestamp);
+    } else {
+      /* Either none or both of start and end need to be set. */
+      response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+      return;
+    }
 
-      md5file = DigestUtils.md5Hex(graphName + "-" + start + "-" + end);
-      path = gcontroller.getBaseDir() + md5file + ".png";
-
-      query = String.format(rquery, start, end, path);
-      gcontroller.generateGraph(query, path);
-      gcontroller.writeOutput(path, request, response);
+    /* Request graph from graph controller, which either returns it from
+     * its cache or asks Rserve to generate it. */
+    String imageFilename = "bandwidth-" + startParameter + "-"
+        + endParameter + ".png";
+    String rQuery = "plot_bandwidth_line('" + startParameter + "', '"
+        + endParameter + "', '%s')";
+    byte[] graphBytes = graphController.generateGraph(rQuery,
+        imageFilename);
 
-    } catch (IOException e) {
-      log.warn(e.toString());
+    /* Make sure that we have a graph to return. */
+    if (graphBytes == null) {
+      response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+      return;
     }
+
+    /* Write graph bytes to response. */
+    BufferedOutputStream output = null;
+    response.setContentType("image/png");
+    response.setHeader("Content-Length",
+        String.valueOf(graphBytes.length));
+    response.setHeader("Content-Disposition",
+        "inline; filename=\"" + imageFilename + "\"");
+    output = new BufferedOutputStream(response.getOutputStream(), 1024);
+    output.write(graphBytes, 0, graphBytes.length);
+    output.close();
   }
 }
+
diff --git a/src/org/torproject/ernie/web/RelayPlatformsImageServlet.java b/src/org/torproject/ernie/web/RelayPlatformsImageServlet.java
index 7dbced4..ee3efec 100644
--- a/src/org/torproject/ernie/web/RelayPlatformsImageServlet.java
+++ b/src/org/torproject/ernie/web/RelayPlatformsImageServlet.java
@@ -1,62 +1,99 @@
 package org.torproject.ernie.web;
 
-import java.util.*;
-import java.text.*;
 import java.io.*;
+import java.text.*;
+import java.util.*;
 import javax.servlet.*;
 import javax.servlet.http.*;
-import org.apache.log4j.Logger;
-import org.apache.commons.codec.digest.DigestUtils;
+
+/* TODO This class shares a lot of code with the other *ImageServlet
+ * classes. We should at some point try harder to reuse code. But let's
+ * wait until we know what parameters besides start and end time will be
+ * shared between these classes. We'll likely want to add more parameters
+ * that reduce the code that can be re-used between servlets. */
 
 public class RelayPlatformsImageServlet extends HttpServlet {
 
-  private static final Logger log;
-  private final String rquery;
-  private final String graphName;
-  private final GraphController gcontroller;
-  private SimpleDateFormat simpledf;
+  private GraphController graphController;
 
-  static {
-    log = Logger.getLogger(RelayPlatformsImageServlet.class);
-  }
+  private SimpleDateFormat dateFormat;
 
   public RelayPlatformsImageServlet()  {
-    this.graphName = "platforms";
-    this.gcontroller = new GraphController(graphName);
-    this.rquery = "plot_platforms_line('%s', '%s', '%s')";
-    this.simpledf = new SimpleDateFormat("yyyy-MM-dd");
-    this.simpledf.setTimeZone(TimeZone.getTimeZone("UTC"));
+    this.graphController = GraphController.getInstance();
+    this.dateFormat = new SimpleDateFormat("yyyy-MM-dd");
+    this.dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
 
   public void doGet(HttpServletRequest request,
       HttpServletResponse response) throws IOException,
       ServletException {
 
-    try {
-      String md5file, start, end, path, query;
-
-      start = request.getParameter("start");
-      end = request.getParameter("end");
-
-      /* Validate input */
+    /* Check parameters. */
+    String startParameter = request.getParameter("start");
+    String endParameter = request.getParameter("end");
+    if (startParameter == null && endParameter == null) {
+      /* If no parameters are given, set default date range to the past 30
+       * days. */
+      long now = System.currentTimeMillis();
+      startParameter = dateFormat.format(now
+          - 30L * 24L * 60L * 60L * 1000L);
+      endParameter = dateFormat.format(now);
+    } else if (startParameter != null && endParameter != null) {
+      long startTimestamp = -1L, endTimestamp = -1L;
       try {
-        simpledf.parse(start);
-        simpledf.parse(end);
+        startTimestamp = dateFormat.parse(startParameter).getTime();
+        endTimestamp = dateFormat.parse(endParameter).getTime();
       } catch (ParseException e)  {
         response.sendError(HttpServletResponse.SC_BAD_REQUEST);
         return;
       }
+      /* The parameters are dates. Good. Does end not precede start? */
+      if (startTimestamp > endTimestamp) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* And while we're at it, make sure both parameters lie in this
+       * century. */
+      if (!startParameter.startsWith("20") ||
+          !endParameter.startsWith("20")) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* Looks like sane parameters. Re-format them to get a canonical
+       * version, not something like 2010-1-1, 2010-01-1, etc. */
+      startParameter = dateFormat.format(startTimestamp);
+      endParameter = dateFormat.format(endTimestamp);
+    } else {
+      /* Either none or both of start and end need to be set. */
+      response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+      return;
+    }
 
-      md5file = DigestUtils.md5Hex(graphName + "-" + start + "-" + end);
-      path = gcontroller.getBaseDir() + md5file + ".png";
-
-      query = String.format(rquery, start, end, path);
-
-      gcontroller.generateGraph(query, path);
-      gcontroller.writeOutput(path, request, response);
+    /* Request graph from graph controller, which either returns it from
+     * its cache or asks Rserve to generate it. */
+    String imageFilename = "platforms-" + startParameter + "-"
+        + endParameter + ".png";
+    String rQuery = "plot_platforms_line('" + startParameter + "', '"
+        + endParameter + "', '%s')";
+    byte[] graphBytes = graphController.generateGraph(rQuery,
+        imageFilename);
 
-    } catch (IOException e) {
-      log.warn(e.toString());
+    /* Make sure that we have a graph to return. */
+    if (graphBytes == null) {
+      response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+      return;
     }
+
+    /* Write graph bytes to response. */
+    BufferedOutputStream output = null;
+    response.setContentType("image/png");
+    response.setHeader("Content-Length",
+        String.valueOf(graphBytes.length));
+    response.setHeader("Content-Disposition",
+        "inline; filename=\"" + imageFilename + "\"");
+    output = new BufferedOutputStream(response.getOutputStream(), 1024);
+    output.write(graphBytes, 0, graphBytes.length);
+    output.close();
   }
 }
+
diff --git a/src/org/torproject/ernie/web/RelayVersionsImageServlet.java b/src/org/torproject/ernie/web/RelayVersionsImageServlet.java
index 0783648..36c2684 100644
--- a/src/org/torproject/ernie/web/RelayVersionsImageServlet.java
+++ b/src/org/torproject/ernie/web/RelayVersionsImageServlet.java
@@ -1,61 +1,99 @@
 package org.torproject.ernie.web;
 
-import java.util.*;
-import java.text.*;
 import java.io.*;
+import java.text.*;
+import java.util.*;
 import javax.servlet.*;
 import javax.servlet.http.*;
-import org.apache.log4j.Logger;
-import org.apache.commons.codec.digest.DigestUtils;
+
+/* TODO This class shares a lot of code with the other *ImageServlet
+ * classes. We should at some point try harder to reuse code. But let's
+ * wait until we know what parameters besides start and end time will be
+ * shared between these classes. We'll likely want to add more parameters
+ * that reduce the code that can be re-used between servlets. */
 
 public class RelayVersionsImageServlet extends HttpServlet {
 
-  private static final Logger log;
-  private final String rquery;
-  private final String graphName;
-  private final GraphController gcontroller;
-  private SimpleDateFormat simpledf;
+  private GraphController graphController;
 
-  static {
-    log = Logger.getLogger(RelayVersionsImageServlet.class);
-  }
+  private SimpleDateFormat dateFormat;
 
   public RelayVersionsImageServlet()  {
-    this.graphName = "versions";
-    this.gcontroller = new GraphController(graphName);
-    this.rquery = "plot_versions_line('%s', '%s', '%s')";
-    this.simpledf = new SimpleDateFormat("yyyy-MM-dd");
-    this.simpledf.setTimeZone(TimeZone.getTimeZone("UTC"));
+    this.graphController = GraphController.getInstance();
+    this.dateFormat = new SimpleDateFormat("yyyy-MM-dd");
+    this.dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
 
   public void doGet(HttpServletRequest request,
       HttpServletResponse response) throws IOException,
       ServletException {
 
-    try {
-      String md5file, start, end, path, query;
-
-      start = request.getParameter("start");
-      end = request.getParameter("end");
-
-      /* Validate input */
+    /* Check parameters. */
+    String startParameter = request.getParameter("start");
+    String endParameter = request.getParameter("end");
+    if (startParameter == null && endParameter == null) {
+      /* If no parameters are given, set default date range to the past 30
+       * days. */
+      long now = System.currentTimeMillis();
+      startParameter = dateFormat.format(now
+          - 30L * 24L * 60L * 60L * 1000L);
+      endParameter = dateFormat.format(now);
+    } else if (startParameter != null && endParameter != null) {
+      long startTimestamp = -1L, endTimestamp = -1L;
       try {
-        simpledf.parse(start);
-        simpledf.parse(end);
+        startTimestamp = dateFormat.parse(startParameter).getTime();
+        endTimestamp = dateFormat.parse(endParameter).getTime();
       } catch (ParseException e)  {
         response.sendError(HttpServletResponse.SC_BAD_REQUEST);
         return;
       }
+      /* The parameters are dates. Good. Does end not precede start? */
+      if (startTimestamp > endTimestamp) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* And while we're at it, make sure both parameters lie in this
+       * century. */
+      if (!startParameter.startsWith("20") ||
+          !endParameter.startsWith("20")) {
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        return;
+      }
+      /* Looks like sane parameters. Re-format them to get a canonical
+       * version, not something like 2010-1-1, 2010-01-1, etc. */
+      startParameter = dateFormat.format(startTimestamp);
+      endParameter = dateFormat.format(endTimestamp);
+    } else {
+      /* Either none or both of start and end need to be set. */
+      response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+      return;
+    }
 
-      md5file = DigestUtils.md5Hex(graphName + "-" + start + "-" + end);
-      path = gcontroller.getBaseDir() + md5file + ".png";
-
-      query = String.format(rquery, start, end, path);
-      gcontroller.generateGraph(query, path);
-      gcontroller.writeOutput(path, request, response);
+    /* Request graph from graph controller, which either returns it from
+     * its cache or asks Rserve to generate it. */
+    String imageFilename = "versions-" + startParameter + "-"
+        + endParameter + ".png";
+    String rQuery = "plot_versions_line('" + startParameter + "', '"
+        + endParameter + "', '%s')";
+    byte[] graphBytes = graphController.generateGraph(rQuery,
+        imageFilename);
 
-    } catch (IOException e) {
-      log.warn(e.toString());
+    /* Make sure that we have a graph to return. */
+    if (graphBytes == null) {
+      response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+      return;
     }
+
+    /* Write graph bytes to response. */
+    BufferedOutputStream output = null;
+    response.setContentType("image/png");
+    response.setHeader("Content-Length",
+        String.valueOf(graphBytes.length));
+    response.setHeader("Content-Disposition",
+        "inline; filename=\"" + imageFilename + "\"");
+    output = new BufferedOutputStream(response.getOutputStream(), 1024);
+    output.write(graphBytes, 0, graphBytes.length);
+    output.close();
   }
 }
+
diff --git a/war/WEB-INF/templates/graphs_custom-graph.tpl.jsp b/war/WEB-INF/templates/graphs_custom-graph.tpl.jsp
index 4599608..6df8b7b 100644
--- a/war/WEB-INF/templates/graphs_custom-graph.tpl.jsp
+++ b/war/WEB-INF/templates/graphs_custom-graph.tpl.jsp
@@ -92,7 +92,8 @@ for tracking a more specific part of the Tor network.</p>
   <jsp:getProperty name="customgraph" property="graphStart"/> to
   <jsp:getProperty name="customgraph" property="graphEnd"/></strong></p>
   <img src="<jsp:getProperty name="customgraph" property="graphURL"/>"
-       href="<jsp:getProperty name="customgraph" property="graphURL"/>"/>
+       href="<jsp:getProperty name="customgraph" property="graphURL"/>"
+       width=576 height=360 />
   <%}
 }%>
 <div style="clear:both;"></div>
-- 
1.7.1