Repository: mesos Updated Branches: refs/heads/master 1e54a761c -> dab0977d2
Reverted commit for HTTP caching of static assets. This reverts commit d0300e1a47d1ba5d6714957fc258ab125fd53ed1. We identified several issues in this implementation and the most important one is described by MESOS-3026. Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/dab0977d Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/dab0977d Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/dab0977d Branch: refs/heads/master Commit: dab0977d2c9649fd9a7235c82cfa5d944ca32214 Parents: 1e54a76 Author: Till Toenshoff <[email protected]> Authored: Fri Jul 10 00:13:06 2015 +0200 Committer: Till Toenshoff <[email protected]> Committed: Fri Jul 10 00:24:52 2015 +0200 ---------------------------------------------------------------------- 3rdparty/libprocess/include/process/http.hpp | 16 ---- 3rdparty/libprocess/src/process.cpp | 62 +------------ 3rdparty/libprocess/src/tests/process_tests.cpp | 94 -------------------- 3 files changed, 1 insertion(+), 171 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/dab0977d/3rdparty/libprocess/include/process/http.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp index 69334b4..72b6d27 100644 --- a/3rdparty/libprocess/include/process/http.hpp +++ b/3rdparty/libprocess/include/process/http.hpp @@ -32,7 +32,6 @@ #include <process/future.hpp> #include <process/owned.hpp> #include <process/pid.hpp> -#include <process/time.hpp> #include <stout/error.hpp> #include <stout/hashmap.hpp> @@ -382,21 +381,6 @@ struct Accepted : Response }; -struct NotModified : Response -{ - explicit NotModified(const Time& lastModified) - { - status = "304 Not Modified"; - - std::string mtime = stringify(RFC1123(lastModified)); - - if (!mtime.empty()) { - headers["Last-Modified"] = mtime; - } - } -}; - - struct TemporaryRedirect : Response { explicit TemporaryRedirect(const std::string& url) http://git-wip-us.apache.org/repos/asf/mesos/blob/dab0977d/3rdparty/libprocess/src/process.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp index ee8002f..d6b0d55 100644 --- a/3rdparty/libprocess/src/process.cpp +++ b/3rdparty/libprocess/src/process.cpp @@ -22,7 +22,6 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <time.h> #include <unistd.h> #include <arpa/inet.h> @@ -95,8 +94,6 @@ #include <stout/thread.hpp> #include <stout/unreachable.hpp> -#include <stout/os/stat.hpp> - #include "config.hpp" #include "decoder.hpp" #include "encoder.hpp" @@ -110,7 +107,6 @@ using namespace process::firewall; using namespace process::metrics::internal; -using process::RFC1123; using process::wait; // Necessary on some OS's to disambiguate. using process::http::Accepted; @@ -118,7 +114,6 @@ using process::http::BadRequest; using process::http::Forbidden; using process::http::InternalServerError; using process::http::NotFound; -using process::http::NotModified; using process::http::OK; using process::http::Request; using process::http::Response; @@ -3028,53 +3023,6 @@ void ProcessBase::visit(const HttpEvent& event) } } - // Get the modification time of the file. - // TODO(arojas): Use Path.mtime() once it is available. - const Try<long> systemMtime = os::stat::mtime(response.path); - const Try<Time> mtime = systemMtime.isSome() - ? Time::create(systemMtime.get()) - : Error(systemMtime.error()); - - bool modified = true; - - // In order to use the HTTP cache the following is needed: - // 1. Request must include the header 'If-Modified-Since' and it - // must be a valid date. - // 2. Modification time from the resource must be determined. - // 3. Both times must be equal. - // However, if any of the conditions are not met, there's no need - // to propagate the error. If a condition is not satisfied, the - // whole file is returned and the appropriate reason is logged. - if (mtime.isSome() && - event.request->headers.contains("If-Modified-Since")) { - tm clientMtime = {}; - - if (strptime( - event.request->headers["If-Modified-Since"].c_str(), - "%a, %d %b %Y %T %Z", - &clientMtime) != NULL) { - const time_t client = std::mktime(&clientMtime); - const time_t server = static_cast<time_t>(mtime.get().secs()); - - if (client != -1 && client == server) { - modified = false; - } else if (client == -1 || server == -1) { - VLOG(1) << "Failed to create 'time_t' through mktime"; - } - } else { - VLOG(1) << "Failed to parse time."; - } - } - - // Provide the Last-Modified header to the client so it can set up - // the cache. - if (mtime.isSome()) { - response.headers["Last-Modified"] = stringify(RFC1123(mtime.get())); - } else { - VLOG(1) << "Failed to get mtime for " << response.path << ": " - << mtime.error(); - } - // TODO(benh): Use "text/plain" for assets that don't have an // extension or we don't have a mapping for? It might be better to // just let the browser guess (or do it's own default). @@ -3084,15 +3032,7 @@ void ProcessBase::visit(const HttpEvent& event) // Enqueue the response with the HttpProxy so that it respects the // order of requests to account for HTTP/1.1 pipelining. - if (modified) { - dispatch(proxy, &HttpProxy::enqueue, response, *event.request); - } else { - dispatch( - proxy, - &HttpProxy::enqueue, - NotModified(mtime.get()), - *event.request); - } + dispatch(proxy, &HttpProxy::enqueue, response, *event.request); } else { VLOG(1) << "Returning '404 Not Found' for '" << event.request->path << "'"; http://git-wip-us.apache.org/repos/asf/mesos/blob/dab0977d/3rdparty/libprocess/src/tests/process_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp index 9a78c91..bc54d29 100644 --- a/3rdparty/libprocess/src/tests/process_tests.cpp +++ b/3rdparty/libprocess/src/tests/process_tests.cpp @@ -1695,100 +1695,6 @@ TEST(ProcessTest, Provide) } -// Requests a static resource via HTTP and validates if proper answers -// are returned for both cache hits and misses. -TEST(ProcessTest, Cache) -{ - const Try<string>& mkdtemp = os::mkdtemp(); - ASSERT_SOME(mkdtemp); - - const string LOREM_IPSUM = - "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do " - "eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad " - "minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip " - "ex ea commodo consequat. Duis aute irure dolor in reprehenderit in " - "voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur " - "sint occaecat cupidatat non proident, sunt in culpa qui officia " - "deserunt mollit anim id est laborum."; - - const string path = path::join(mkdtemp.get(), "lorem.txt"); - ASSERT_SOME(os::write(path, LOREM_IPSUM)); - - FileServer server(path); - PID<FileServer> pid = spawn(server); - - // First request. - Future<http::Response> response = http::get(pid); - - AWAIT_READY(response); - - // First request had no information about the served file, so a 200 - // code is expected as well as the full body. - EXPECT_EQ("200 OK", response.get().status); - ASSERT_TRUE(response.get().headers.contains("Last-Modified")); - - // Request includes proper cache headers. - const std::string mtime = response.get().headers.get("Last-Modified").get(); - - hashmap<std::string, std::string> headers; - headers.put("Cache-Control", "max-age=0"); - headers.put("If-Modified-Since", mtime); - - response = http::get(pid, None(), None(), headers); - - AWAIT_READY(response); - ASSERT_EQ("304 Not Modified", response.get().status); - ASSERT_TRUE(response.get().headers.contains("Last-Modified")); - EXPECT_SOME_EQ(mtime, response.get().headers.get("Last-Modified")); - - static const char HTTP_DATE[] = "%a, %d %b %Y %T %Z"; - - // Use a different modification time for the request. The whole file - // should be served. - tm tmTime = {}; - strptime(mtime.c_str(), HTTP_DATE, &tmTime); - time_t earlierTime = mktime(&tmTime); - ASSERT_NE(-1, earlierTime); - - // Just an arbitrary earlier time than the mtime. In this case, 100 - // seconds. - earlierTime -= 100; - - // Transform the time back to a string. - ASSERT_TRUE(NULL != gmtime_r(&earlierTime, &tmTime)); - static const size_t BUFFER_SIZE = 200; - char buffer[BUFFER_SIZE] = {0}; - ASSERT_NE(0, strftime(buffer, BUFFER_SIZE, HTTP_DATE, &tmTime)); - std::string strTime = buffer; - headers.put("If-Modified-Since", strTime); - - response = http::get(pid, None(), None(), headers); - - AWAIT_READY(response); - - // Request 'If-Modified-Since' header doesn't match the server's - // mtime. Full file expected. - EXPECT_EQ("200 OK", response.get().status); - EXPECT_TRUE(response.get().headers.contains("Last-Modified")); - EXPECT_EQ(LOREM_IPSUM, response.get().body); - - // Finally, test with an unparseable date. - headers.put("If-Modified-Since", "XXX-YYY-ZZZ PPP"); - response = http::get(pid, None(), None(), headers); - - // Request 'If-Modified-Since' header doesn't match the server's - // mtime. Full file expected. - EXPECT_EQ("200 OK", response.get().status); - EXPECT_TRUE(response.get().headers.contains("Last-Modified")); - EXPECT_EQ(LOREM_IPSUM, response.get().body); - - terminate(server); - wait(server); - - ASSERT_SOME(os::rmdir(path)); -} - - int baz(string s) { return 42; } Future<int> bam(string s) { return 42; }
