From f6cdd76d3ce94b587a41e04ed3153606e9c149aa Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Mon, 17 Dec 2018 13:49:33 +0000 Subject: use portal in storage webserver --- .../src/tests/frameworkimpl/status/statustest.cpp | 86 ++++---- .../frameworkimpl/status/statuswebserver.cpp | 242 +++++---------------- .../storage/frameworkimpl/status/statuswebserver.h | 29 +-- .../generic/status/statusreporter.cpp | 12 - .../generic/status/statusreporter.h | 7 +- 5 files changed, 115 insertions(+), 261 deletions(-) diff --git a/storage/src/tests/frameworkimpl/status/statustest.cpp b/storage/src/tests/frameworkimpl/status/statustest.cpp index 3ee9924648d..b047a4000cf 100644 --- a/storage/src/tests/frameworkimpl/status/statustest.cpp +++ b/storage/src/tests/frameworkimpl/status/statustest.cpp @@ -8,6 +8,29 @@ #include #include #include +#include +#include +#include + +vespalib::string fetch(int port, const vespalib::string &path) { + auto crypto = vespalib::CryptoEngine::get_default(); + auto socket = vespalib::SocketSpec::from_port(port).client_address().connect(); + CPPUNIT_ASSERT(socket.valid()); + auto conn = vespalib::SyncCryptoSocket::create(*crypto, std::move(socket), false); + vespalib::string http_req = vespalib::make_string("GET %s HTTP/1.1\r\n" + "Host: localhost:%d\r\n" + "\r\n", path.c_str(), port); + CPPUNIT_ASSERT_EQUAL(conn->write(http_req.data(), http_req.size()), ssize_t(http_req.size())); + char buf[1024]; + vespalib::string result; + ssize_t res = conn->read(buf, sizeof(buf)); + while (res > 0) { + result.append(vespalib::stringref(buf, res)); + res = conn->read(buf, sizeof(buf)); + } + CPPUNIT_ASSERT_EQUAL(res, ssize_t(0)); + return result; +} namespace storage { @@ -27,7 +50,6 @@ struct StatusTest : public CppUnit::TestFixture { CPPUNIT_TEST(testHtmlStatus); CPPUNIT_TEST(testXmlStatus); CPPUNIT_TEST(test404); - CPPUNIT_TEST(requireThatServerSpecIsConstructedCorrectly); CPPUNIT_TEST_SUITE_END(); }; @@ -102,14 +124,12 @@ StatusTest::testIndexStatusPage() "barid", "Bar impl", "

info

")); StatusWebServer webServer(_node->getComponentRegister(), _node->getComponentRegister(), - "raw:httpport -1"); - std::ostringstream ss; - framework::HttpUrlPath path(""); - webServer.handlePage(path, ss); + "raw:httpport 0"); + auto actual = fetch(webServer.getListenPort(), "/"); std::string expected( "HTTP\\/1.1 200 OK\r\n" - "Connection: Close\r\n" - "Content-type: text\\/html\r\n" + "Connection: close\r\n" + "Content-Type: text\\/html\r\n" "\r\n" "\n" "\n" @@ -123,7 +143,7 @@ StatusTest::testIndexStatusPage() "<\\/body>\n" "<\\/html>\n" ); - CPPUNIT_ASSERT_MATCH_REGEX(expected, ss.str()); + CPPUNIT_ASSERT_MATCH_REGEX(expected, actual); } void @@ -134,14 +154,12 @@ StatusTest::testHtmlStatus() "fooid", "Foo impl", "

info

", "")); StatusWebServer webServer(_node->getComponentRegister(), _node->getComponentRegister(), - "raw:httpport -1"); - std::ostringstream ost; - framework::HttpUrlPath path("/fooid?unusedParam"); - webServer.handlePage(path, ost); + "raw:httpport 0"); + auto actual = fetch(webServer.getListenPort(), "/fooid?unusedParam"); std::string expected( "HTTP/1.1 200 OK\r\n" - "Connection: Close\r\n" - "Content-type: text/html\r\n" + "Connection: close\r\n" + "Content-Type: text/html\r\n" "\r\n" "\n" "\n" @@ -152,7 +170,7 @@ StatusTest::testHtmlStatus() "

info

\n" "\n" ); - CPPUNIT_ASSERT_EQUAL(expected, ost.str()); + CPPUNIT_ASSERT_EQUAL(expected, std::string(actual)); } void @@ -163,21 +181,19 @@ StatusTest::testXmlStatus() "fooid", "Foo impl")); StatusWebServer webServer(_node->getComponentRegister(), _node->getComponentRegister(), - "raw:httpport -1"); - std::ostringstream ost; - framework::HttpUrlPath path("/fooid?unusedParam"); - webServer.handlePage(path, ost); + "raw:httpport 0"); + auto actual = fetch(webServer.getListenPort(), "/fooid?unusedParam"); std::string expected( "HTTP/1.1 200 OK\r\n" - "Connection: Close\r\n" - "Content-type: application/xml\r\n" + "Connection: close\r\n" + "Content-Type: application/xml\r\n" "\r\n" "\n" "\n" "content\n" "" ); - CPPUNIT_ASSERT_EQUAL(expected, ost.str()); + CPPUNIT_ASSERT_EQUAL(expected, std::string(actual)); } void @@ -185,30 +201,14 @@ StatusTest::test404() { StatusWebServer webServer(_node->getComponentRegister(), _node->getComponentRegister(), - "raw:httpport -1"); - std::ostringstream ost; - framework::HttpUrlPath path("/fooid?unusedParam"); - webServer.handlePage(path, ost); + "raw:httpport 0"); + auto actual = fetch(webServer.getListenPort(), "/fooid?unusedParam"); std::string expected( - "HTTP/1.1 404 Not found\r\n" - "Connection: Close\r\n" - "Content-type: text/html\r\n" + "HTTP/1.1 404 Not Found\r\n" + "Connection: close\r\n" "\r\n" - "404 Not found\r\n" - "

404 Not found

\r\n" - "

\r\n" - "\r\n" ); - CPPUNIT_ASSERT_EQUAL_ESCAPED(expected, ost.str()); -} - -void -StatusTest::requireThatServerSpecIsConstructedCorrectly() -{ - CPPUNIT_ASSERT_EQUAL(vespalib::string("requesthost:10"), - StatusWebServer::getServerSpec("requesthost:10", "serverhost:20")); - CPPUNIT_ASSERT_EQUAL(vespalib::string("serverhost:20"), - StatusWebServer::getServerSpec("", "serverhost:20")); + CPPUNIT_ASSERT_EQUAL_ESCAPED(expected, std::string(actual)); } } // storage diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp index c7d8bf24e82..cf38eff4dfe 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include LOG_SETUP(".status"); @@ -22,16 +24,10 @@ StatusWebServer::StatusWebServer( _port(0), _httpServer(), _configFetcher(configUri.getContext()), - _queuedRequests(), - _component(std::make_unique(componentRegister, "Status")), - _thread() + _component(std::make_unique(componentRegister, "Status")) { _configFetcher.subscribe(configUri.getConfigId(), this); _configFetcher.start(); - framework::MilliSecTime maxProcessingTime(60 * 60 * 1000); - framework::MilliSecTime maxWaitTime(10 * 1000); - _thread = _component->startThread(*this, maxProcessingTime, maxWaitTime); - } StatusWebServer::~StatusWebServer() @@ -44,11 +40,6 @@ StatusWebServer::~StatusWebServer() } // Delete http server to ensure that no more incoming requests reach us. _httpServer.reset(0); - - // Stop internal thread such that we don't process anymore web requests - if (_thread.get() != 0) { - _thread->interruptAndJoin(&_workerMonitor); - } } void StatusWebServer::configure(std::unique_ptr config) @@ -65,37 +56,6 @@ void StatusWebServer::configure(std::unique_ptr= 0) { server.reset(new WebServer(*this, newPort)); - server->SetKeepAlive(false); - - bool started = false; - switch (server->Start()) { - case FASTLIB_SUCCESS: - started = true; - break; - case FASTLIB_HTTPSERVER_BADLISTEN: - LOG(warning, "Listen failed on port %u", newPort); - break; - case FASTLIB_HTTPSERVER_NEWTHREADFAILED: - LOG(warning, "Failed starting thread for status server on port %u", newPort); - break; - case FASTLIB_HTTPSERVER_ALREADYSTARTED: - LOG(warning, "Failed starting status server on port %u (already started?)", newPort); - break; - default: - LOG(warning, "Failed starting status server on port %u (unknown reason)", newPort); - break; - } - if (!started) { - std::ostringstream ost; - ost << "Failed to start status HTTP server using port " << newPort << "."; - if (_httpServer) { - ost << " Status server still running on port " << _port << " instead of suggested port " << newPort; - } - LOG(fatal, "%s.", ost.str().c_str()); - _component->requestShutdown(ost.str()); - _httpServer.reset(); - return; - } // Now that we know config update went well, update internal state _port = server->getListenPort(); LOG(config, "Status pages now available on port %u", _port); @@ -112,45 +72,47 @@ void StatusWebServer::configure(std::unique_ptrbind("/", *this)) { } +StatusWebServer::WebServer::~WebServer() +{ + _root.reset(); + _executor.shutdown().sync(); +} namespace { - /** Utility class for printing HTTP errors. */ - struct HttpErrorWriter { - std::ostream& _out; - HttpErrorWriter(std::ostream& out, vespalib::stringref error) - : _out(out) - { - _out << "HTTP/1.1 " << error << "\r\n" - "Connection: Close\r\n" - "Content-type: text/html\r\n\r\n" - "" << error << "\r\n" - "

" << error << "

\r\n" - "

"; - } +struct HandleGetTask : vespalib::Executor::Task { + vespalib::Portal::GetRequest request; + std::function fun; + HandleGetTask(vespalib::Portal::GetRequest request_in, + std::function fun_in) + : request(std::move(request_in)), fun(fun_in) {} + void run() override { fun(std::move(request)); } +}; - template - HttpErrorWriter& operator<<(const T& t) { - _out << t; - return *this; - } +} - ~HttpErrorWriter() { - _out << "

\r\n" - "\r\n"; - } - }; + +void +StatusWebServer::WebServer::get(vespalib::Portal::GetRequest request) +{ + auto fun = [this](vespalib::Portal::GetRequest req) + { + handle_get(std::move(req)); + }; + _executor.execute(std::make_unique(std::move(request), std::move(fun))); } void -StatusWebServer::WebServer::onGetRequest(const string & tmpurl, const string &serverSpec, Fast_HTTPConnection& conn) +StatusWebServer::WebServer::handle_get(vespalib::Portal::GetRequest request) { + const vespalib::string &tmpurl = request.get_uri(); Fast_URL urlCodec; int bufLength = tmpurl.length() * 2 + 10; char * encodedUrl = new char[bufLength]; @@ -160,65 +122,11 @@ StatusWebServer::WebServer::onGetRequest(const string & tmpurl, const string &se urlCodec.decode(encodedUrl, decodedUrl, bufLength); delete [] encodedUrl; - string url = decodedUrl; + vespalib::string url = decodedUrl; LOG(debug, "Status got get request '%s'", url.c_str()); - framework::HttpUrlPath urlpath(url.c_str(), - StatusWebServer::getServerSpec(serverSpec, getServerSpec())); - std::string link(urlpath.getPath()); - if (link.size() > 0 && link[0] == '/') link = link.substr(1); - // Only allow crucial components not locking to answer directly. - // (We want deadlockdetector status page to be available during a - // deadlock - if (link == "" || link == "deadlockdetector") { - std::ostringstream ost; - _status.handlePage(urlpath, ost); - conn.Output(ost.str().c_str()); - } else { - // Route other status requests that can possibly deadlock to a - // worker thread. - vespalib::MonitorGuard monitor(_status._workerMonitor); - _status._queuedRequests.emplace_back(std::make_shared(url.c_str(), urlpath.getServerSpec())); - HttpRequest* req = _status._queuedRequests.back().get(); - framework::SecondTime timeout(urlpath.get("timeout", 30u)); - framework::SecondTime timeoutTime(_status._component->getClock().getTimeInSeconds() + timeout); - monitor.signal(); - while (true) { - monitor.wait(100); - bool done = false; - if (req->_result.get()) { - conn.Output(req->_result->c_str()); - LOG(debug, "Finished status request for '%s'", req->_url.c_str()); - done = true; - } else { - if (_status._component->getClock().getTimeInSeconds() > timeoutTime) - { - std::ostringstream ost; - { - HttpErrorWriter writer(ost, "500 Internal Server Error"); - writer << "Request " << url.c_str() << " timed out " - << "after " << timeout << " seconds."; - } - LOG(debug, "HTTP status request failed: %s. %zu requests queued", - ost.str().c_str(), _status._queuedRequests.size() - 1); - conn.Output(ost.str().c_str()); - done = true; - } - } - if (done) { - for (std::list::iterator it - = _status._queuedRequests.begin(); - it != _status._queuedRequests.end(); ++it) - { - if (it->get() == req) { - _status._queuedRequests.erase(it); - break; - } - } - break; - } - } - } + framework::HttpUrlPath urlpath(url.c_str(), request.get_host()); + _status.handlePage(urlpath, std::move(request)); } namespace { @@ -236,8 +144,15 @@ namespace { }; } + +int +StatusWebServer::getListenPort() const +{ + return _httpServer ? _httpServer->getListenPort() : -1; +} + void -StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, std::ostream& out) +StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, vespalib::Portal::GetRequest request) { vespalib::string link(urlpath.getPath()); if (link.size() > 0 && link[0] == '/') link = link.substr(1); @@ -245,28 +160,23 @@ StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, std::ostream& size_t slashPos = link.find('/'); if (slashPos != std::string::npos) link = link.substr(0, slashPos); - bool pageExisted = false; if ( ! link.empty()) { const framework::StatusReporter *reporter = _reporterMap.getStatusReporter(link); if (reporter != nullptr) { try { - pageExisted = reporter->reportHttpHeader(out, urlpath); - if (pageExisted) { - pageExisted = reporter->reportStatus(out, urlpath); + std::ostringstream content; + auto content_type = reporter->getReportContentType(urlpath); + if (reporter->reportStatus(content, urlpath)) { + request.respond_with_content(content_type, content.str()); + } else { + request.respond_with_error(404, "Not Found"); } } catch (std::exception &e) { - HttpErrorWriter writer(out, "500 Internal Server Error"); - writer << "

" << e.what() << "

"; - pageExisted = true; - } - if (pageExisted) { - LOG(spam, "Status finished request"); - return; + request.respond_with_error(500, "Internal Server Error"); } + } else { + request.respond_with_error(404, "Not Found"); } - } - if (!pageExisted && link.size() > 0) { - HttpErrorWriter writer(out, "404 Not found"); } else { IndexPageReporter indexRep; indexRep << "

Binary version of Vespa: " @@ -278,52 +188,12 @@ StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, std::ostream& << reporter->getName() << "
\n"; } } - indexRep.reportHttpHeader(out, urlpath); - indexRep.reportStatus(out, urlpath); + std::ostringstream content; + auto content_type = indexRep.getReportContentType(urlpath); + indexRep.reportStatus(content, urlpath); + request.respond_with_content(content_type, content.str()); } LOG(spam, "Status finished request"); } -vespalib::string -StatusWebServer::getServerSpec(const vespalib::string &specFromRequest, - const vespalib::string &specFromServer) -{ - if (specFromRequest.empty()) { - // This is a fallback in case the request spec is not given (HTTP 1.0 header) - return specFromServer; - } - return specFromRequest; -} - -void -StatusWebServer::run(framework::ThreadHandle& thread) -{ - while (!thread.interrupted()) { - HttpRequest::SP request; - { - vespalib::MonitorGuard monitor(_workerMonitor); - for (const HttpRequest::SP & cur : _queuedRequests) { - if ( ! cur->_result ) { - request = cur; - break; - } - } - if (!request.get()) { - monitor.wait(10 * 1000); - thread.registerTick(framework::WAIT_CYCLE); - continue; - } - } - framework::HttpUrlPath urlpath(request->_url, request->_serverSpec); - std::ostringstream ost; - handlePage(urlpath, ost); - // If the same request is still in front of the queue - // (it hasn't timed out), add the result to it. - vespalib::MonitorGuard monitor(_workerMonitor); - request->_result.reset(new vespalib::string(ost.str())); - monitor.signal(); - thread.registerTick(framework::PROCESS_CYCLE); - } -} - } // storage diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h index fe716b4212c..756a895afb8 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h @@ -14,7 +14,8 @@ #include #include #include -#include +#include +#include #include namespace storage { @@ -30,16 +31,21 @@ namespace framework { class StatusWebServer : private config::IFetcherCallback, private framework::Runnable { - class WebServer : public Fast_HTTPServer { + class WebServer : public vespalib::Portal::GetHandler { StatusWebServer& _status; - vespalib::string _serverSpec; + vespalib::Portal::SP _server; + vespalib::ThreadStackExecutor _executor; + vespalib::Portal::Token::UP _root; public: WebServer(StatusWebServer&, uint16_t port); + ~WebServer(); + + void get(vespalib::Portal::GetRequest request) override; + void handle_get(vespalib::Portal::GetRequest request); - void onGetRequest(const string & url, const string & serverSpec, Fast_HTTPConnection& conn) override; - const vespalib::string &getServerSpec() const { - return _serverSpec; + int getListenPort() const { + return _server->listen_port(); } }; struct HttpRequest { @@ -61,9 +67,7 @@ class StatusWebServer : private config::IFetcherCallback _httpServer; config::ConfigFetcher _configFetcher; - std::list _queuedRequests; std::unique_ptr _component; - std::unique_ptr _thread; public: StatusWebServer(const StatusWebServer &) = delete; @@ -72,14 +76,11 @@ public: framework::StatusReporterMap&, const config::ConfigUri & configUri); ~StatusWebServer() override; - - void handlePage(const framework::HttpUrlPath&, std::ostream& out); - static vespalib::string getServerSpec(const vespalib::string &requestSpec, - const vespalib::string &serverSpec); + int getListenPort() const; + void handlePage(const framework::HttpUrlPath&, vespalib::Portal::GetRequest request); private: void configure(std::unique_ptr config) override; - void getPage(const char* url, Fast_HTTPConnection& conn); - void run(framework::ThreadHandle&) override; + void run(framework::ThreadHandle&) override {} }; } diff --git a/storageframework/src/vespa/storageframework/generic/status/statusreporter.cpp b/storageframework/src/vespa/storageframework/generic/status/statusreporter.cpp index b17c70d132e..c7d025b060f 100644 --- a/storageframework/src/vespa/storageframework/generic/status/statusreporter.cpp +++ b/storageframework/src/vespa/storageframework/generic/status/statusreporter.cpp @@ -15,17 +15,5 @@ StatusReporter::~StatusReporter() { } -bool -StatusReporter::reportHttpHeader(std::ostream& out, - const HttpUrlPath& path) const -{ - vespalib::string contentType(getReportContentType(path)); - if (contentType == "") return false; - out << "HTTP/1.1 200 OK\r\n" - "Connection: Close\r\n" - "Content-type: " << contentType << "\r\n\r\n"; - return true; -} - } // framework } // storage diff --git a/storageframework/src/vespa/storageframework/generic/status/statusreporter.h b/storageframework/src/vespa/storageframework/generic/status/statusreporter.h index 64e7c7b62e7..8d58467a221 100644 --- a/storageframework/src/vespa/storageframework/generic/status/statusreporter.h +++ b/storageframework/src/vespa/storageframework/generic/status/statusreporter.h @@ -40,12 +40,7 @@ struct StatusReporter virtual bool isValidStatusRequest() const { return true; } /** - * Overwrite to get full control of header writes. - * Returning false means the page specified by path does not exist. - */ - virtual bool reportHttpHeader(std::ostream&, const HttpUrlPath&) const; - /** - * Called by default writeHttpHeader implementation to get content type. + * Called to get content type. * An empty string indicates page not found. */ virtual vespalib::string getReportContentType(const HttpUrlPath&) const = 0; -- cgit v1.2.3