ResourceServlet: Stream large files, bypassing the cache
Sufficiently large static files will thrash the cache. Don't bother even storing them in the cache; stream them directly from their Paths. Instead of using a content-based hash for the ETag, just use Last-Modified based on the modified time of the file. Change-Id: Idc9b5daf9f5da124eb0d6b8b21806866de3d3a6b
This commit is contained in:
		| @@ -17,7 +17,9 @@ package com.google.gerrit.httpd.raw; | |||||||
| import static com.google.common.base.Preconditions.checkNotNull; | import static com.google.common.base.Preconditions.checkNotNull; | ||||||
| import static com.google.common.net.HttpHeaders.CONTENT_ENCODING; | import static com.google.common.net.HttpHeaders.CONTENT_ENCODING; | ||||||
| import static com.google.common.net.HttpHeaders.ETAG; | import static com.google.common.net.HttpHeaders.ETAG; | ||||||
|  | import static com.google.common.net.HttpHeaders.IF_MODIFIED_SINCE; | ||||||
| import static com.google.common.net.HttpHeaders.IF_NONE_MATCH; | import static com.google.common.net.HttpHeaders.IF_NONE_MATCH; | ||||||
|  | import static com.google.common.net.HttpHeaders.LAST_MODIFIED; | ||||||
| import static java.util.concurrent.TimeUnit.DAYS; | import static java.util.concurrent.TimeUnit.DAYS; | ||||||
| import static java.util.concurrent.TimeUnit.MINUTES; | import static java.util.concurrent.TimeUnit.MINUTES; | ||||||
| import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; | import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; | ||||||
| @@ -28,6 +30,7 @@ import com.google.common.base.CharMatcher; | |||||||
| import com.google.common.cache.Cache; | import com.google.common.cache.Cache; | ||||||
| import com.google.common.collect.ImmutableMap; | import com.google.common.collect.ImmutableMap; | ||||||
| import com.google.common.hash.Hashing; | import com.google.common.hash.Hashing; | ||||||
|  | import com.google.gerrit.common.FileUtil; | ||||||
| import com.google.gerrit.common.Nullable; | import com.google.gerrit.common.Nullable; | ||||||
| import com.google.gerrit.httpd.HtmlDomUtil; | import com.google.gerrit.httpd.HtmlDomUtil; | ||||||
| import com.google.gwtexpui.server.CacheHeaders; | import com.google.gwtexpui.server.CacheHeaders; | ||||||
| @@ -36,14 +39,15 @@ import com.google.gwtjsonrpc.server.RPCServletUtils; | |||||||
| import org.slf4j.Logger; | import org.slf4j.Logger; | ||||||
| import org.slf4j.LoggerFactory; | import org.slf4j.LoggerFactory; | ||||||
|  |  | ||||||
| import java.io.FileNotFoundException; |  | ||||||
| import java.io.IOException; | import java.io.IOException; | ||||||
| import java.io.OutputStream; | import java.io.OutputStream; | ||||||
| import java.nio.file.Files; | import java.nio.file.Files; | ||||||
|  | import java.nio.file.NoSuchFileException; | ||||||
| import java.nio.file.Path; | import java.nio.file.Path; | ||||||
| import java.nio.file.attribute.FileTime; | import java.nio.file.attribute.FileTime; | ||||||
| import java.util.concurrent.Callable; | import java.util.concurrent.Callable; | ||||||
| import java.util.concurrent.ExecutionException; | import java.util.concurrent.ExecutionException; | ||||||
|  | import java.util.zip.GZIPOutputStream; | ||||||
|  |  | ||||||
| import javax.servlet.http.HttpServlet; | import javax.servlet.http.HttpServlet; | ||||||
| import javax.servlet.http.HttpServletRequest; | import javax.servlet.http.HttpServletRequest; | ||||||
| @@ -61,6 +65,8 @@ public abstract class ResourceServlet extends HttpServlet { | |||||||
|   private static final Logger log = |   private static final Logger log = | ||||||
|       LoggerFactory.getLogger(ResourceServlet.class); |       LoggerFactory.getLogger(ResourceServlet.class); | ||||||
|  |  | ||||||
|  |   private static final int CACHE_FILE_SIZE_LIMIT_BYTES = 100 << 10; | ||||||
|  |  | ||||||
|   private static final String JS = "application/x-javascript"; |   private static final String JS = "application/x-javascript"; | ||||||
|   private static final ImmutableMap<String, String> MIME_TYPES = |   private static final ImmutableMap<String, String> MIME_TYPES = | ||||||
|       ImmutableMap.<String, String> builder() |       ImmutableMap.<String, String> builder() | ||||||
| @@ -108,20 +114,45 @@ public abstract class ResourceServlet extends HttpServlet { | |||||||
|   @Override |   @Override | ||||||
|   protected void doGet(HttpServletRequest req, HttpServletResponse rsp) |   protected void doGet(HttpServletRequest req, HttpServletResponse rsp) | ||||||
|       throws IOException { |       throws IOException { | ||||||
|     Resource r; |     String name = CharMatcher.is('/').trimFrom(req.getPathInfo()); | ||||||
|  |     if (isUnreasonableName(name)) { | ||||||
|  |       notFound(rsp); | ||||||
|  |       return; | ||||||
|  |     } | ||||||
|  |     Path p = getResourcePath(name); | ||||||
|  |     if (p == null) { | ||||||
|  |       notFound(rsp); | ||||||
|  |       return; | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     Resource r = cache.getIfPresent(p); | ||||||
|  |     if (r == null && maybeStream(p, req, rsp)) { | ||||||
|  |       return; | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     if (r == null) { | ||||||
|  |       Callable<Resource> loader = newLoader(p); | ||||||
|       try { |       try { | ||||||
|       r = getResource(req); |         r = cache.get(p, loader); | ||||||
|  |         if (refresh && r.isStale(p)) { | ||||||
|  |           cache.invalidate(p); | ||||||
|  |           r = cache.get(p, loader); | ||||||
|  |         } | ||||||
|       } catch (ExecutionException e) { |       } catch (ExecutionException e) { | ||||||
|       log.warn(String.format( |         log.warn("Cannot load static resource " + req.getPathInfo(), e); | ||||||
|           "Cannot load static resource %s", |  | ||||||
|           req.getPathInfo()), e); |  | ||||||
|         CacheHeaders.setNotCacheable(rsp); |         CacheHeaders.setNotCacheable(rsp); | ||||||
|         rsp.setStatus(SC_INTERNAL_SERVER_ERROR); |         rsp.setStatus(SC_INTERNAL_SERVER_ERROR); | ||||||
|         return; |         return; | ||||||
|       } |       } | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     if (r == Resource.NOT_FOUND) { | ||||||
|  |       notFound(rsp); | ||||||
|  |       return; | ||||||
|  |     } | ||||||
|  |  | ||||||
|     String e = req.getParameter("e"); |     String e = req.getParameter("e"); | ||||||
|     if (r == Resource.NOT_FOUND || (e != null && !r.etag.equals(e))) { |     if (e != null && !r.etag.equals(e)) { | ||||||
|       CacheHeaders.setNotCacheable(rsp); |       CacheHeaders.setNotCacheable(rsp); | ||||||
|       rsp.setStatus(SC_NOT_FOUND); |       rsp.setStatus(SC_NOT_FOUND); | ||||||
|       return; |       return; | ||||||
| @@ -164,29 +195,61 @@ public abstract class ResourceServlet extends HttpServlet { | |||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private Resource getResource(HttpServletRequest req) |   private static void notFound(HttpServletResponse rsp) { | ||||||
|       throws ExecutionException { |     rsp.setStatus(SC_NOT_FOUND); | ||||||
|     String name = CharMatcher.is('/').trimFrom(req.getPathInfo()); |     CacheHeaders.setNotCacheable(rsp); | ||||||
|     if (isUnreasonableName(name)) { |  | ||||||
|       return Resource.NOT_FOUND; |  | ||||||
|     } |  | ||||||
|     Path p = getResourcePath(name); |  | ||||||
|     if (p == null) { |  | ||||||
|       return Resource.NOT_FOUND; |  | ||||||
|   } |   } | ||||||
|  |  | ||||||
|     Callable<Resource> loader = newLoader(p); |   /** | ||||||
|     Resource r = cache.get(p, loader); |    * Maybe stream a path to the response, depending on the properties of the | ||||||
|     if (r == Resource.NOT_FOUND) { |    * file and cache headers in the request. | ||||||
|       return Resource.NOT_FOUND; |    * | ||||||
|  |    * @param p path to stream | ||||||
|  |    * @param req HTTP request. | ||||||
|  |    * @param rsp HTTP response. | ||||||
|  |    * @return true if the response was written (either the file contents or an | ||||||
|  |    *     error); false if the path is too small to stream and should be cached. | ||||||
|  |    */ | ||||||
|  |   private boolean maybeStream(Path p, HttpServletRequest req, | ||||||
|  |       HttpServletResponse rsp) throws IOException { | ||||||
|  |     try { | ||||||
|  |       if (Files.size(p) < CACHE_FILE_SIZE_LIMIT_BYTES) { | ||||||
|  |         return false; | ||||||
|  |       } | ||||||
|  |     } catch (NoSuchFileException e) { | ||||||
|  |       cache.put(p, Resource.NOT_FOUND); | ||||||
|  |       notFound(rsp); | ||||||
|  |       return true; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     if (refresh && r.isStale(p)) { |     long lastModified = FileUtil.lastModified(p); | ||||||
|       cache.invalidate(p); |     if (req.getDateHeader(IF_MODIFIED_SINCE) >= lastModified) { | ||||||
|       r = cache.get(p, loader); |       rsp.setStatus(SC_NOT_MODIFIED); | ||||||
|  |       return true; | ||||||
|     } |     } | ||||||
|     return r; |  | ||||||
|  |     if (lastModified > 0) { | ||||||
|  |       rsp.setDateHeader(LAST_MODIFIED, lastModified); | ||||||
|     } |     } | ||||||
|  |     if (!CacheHeaders.hasCacheHeader(rsp)) { | ||||||
|  |       CacheHeaders.setCacheable(req, rsp, 15, MINUTES, refresh); | ||||||
|  |     } | ||||||
|  |     rsp.setContentType(contentType(p.toString())); | ||||||
|  |  | ||||||
|  |     OutputStream out = rsp.getOutputStream(); | ||||||
|  |     GZIPOutputStream gz = null; | ||||||
|  |     if (RPCServletUtils.acceptsGzipEncoding(req)) { | ||||||
|  |       rsp.setHeader(CONTENT_ENCODING, "gzip"); | ||||||
|  |       gz = new GZIPOutputStream(out); | ||||||
|  |       out = gz; | ||||||
|  |     } | ||||||
|  |     Files.copy(p, out); | ||||||
|  |     if (gz != null) { | ||||||
|  |       gz.finish(); | ||||||
|  |     } | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |  | ||||||
|   private static boolean isUnreasonableName(String name) { |   private static boolean isUnreasonableName(String name) { | ||||||
|     return name.length() < 1 |     return name.length() < 1 | ||||||
| @@ -206,7 +269,7 @@ public abstract class ResourceServlet extends HttpServlet { | |||||||
|               Files.getLastModifiedTime(p), |               Files.getLastModifiedTime(p), | ||||||
|               contentType(p.toString()), |               contentType(p.toString()), | ||||||
|               Files.readAllBytes(p)); |               Files.readAllBytes(p)); | ||||||
|         } catch (FileNotFoundException e) { |         } catch (NoSuchFileException e) { | ||||||
|           return Resource.NOT_FOUND; |           return Resource.NOT_FOUND; | ||||||
|         } |         } | ||||||
|       } |       } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Dave Borowitz
					Dave Borowitz