From b2bf67f7e5d44091a86a6f3e9c60ff2b65aea53a Mon Sep 17 00:00:00 2001 From: mwilliams Date: Wed, 24 Jul 2013 11:30:23 -0700 Subject: [PATCH] Fix pagelet timeouts Push the remaining time through to the pagelet's HttpRequestHandler, and fix HttpRequestHandler::handleRequest to use the timeout we pass in (the latter was an inadvertent omission from my previous timeout diff). --- hphp/runtime/base/thread_info.cpp | 12 +++++- hphp/runtime/base/types.h | 1 + hphp/runtime/ext/ext_server.cpp | 10 +++-- hphp/runtime/server/http_request_handler.cpp | 2 +- hphp/runtime/server/pagelet_server.cpp | 41 ++++++++++++++++---- hphp/runtime/server/pagelet_server.h | 3 +- 6 files changed, 55 insertions(+), 14 deletions(-) diff --git a/hphp/runtime/base/thread_info.cpp b/hphp/runtime/base/thread_info.cpp index faa45a72a..8c31bcabe 100644 --- a/hphp/runtime/base/thread_info.cpp +++ b/hphp/runtime/base/thread_info.cpp @@ -207,12 +207,22 @@ void RequestInjectionData::setTimeout(int seconds) { #endif } +int RequestInjectionData::getRemainingTime() const { + if (m_hasTimer) { + itimerspec ts; + if (!timer_gettime(m_timer_id, &ts)) { + int remaining = ts.it_value.tv_sec; + return remaining > 1 ? remaining : 1; + } + } + return m_timeoutSeconds; +} + void RequestInjectionData::resetTimer(int seconds /* = -1 */) { auto data = &ThreadInfo::s_threadInfo->m_reqInjectionData; if (seconds <= 0) seconds = data->getTimeout(); data->setTimeout(seconds); data->clearTimedOutFlag(); - } void RequestInjectionData::reset() { diff --git a/hphp/runtime/base/types.h b/hphp/runtime/base/types.h index 8ea98c937..78ced2117 100644 --- a/hphp/runtime/base/types.h +++ b/hphp/runtime/base/types.h @@ -243,6 +243,7 @@ public: public: int getTimeout() const { return m_timeoutSeconds; } void setTimeout(int seconds); + int getRemainingTime() const; void resetTimer(int seconds = -1); void setSurprisePage(void* page); void onTimeout(); diff --git a/hphp/runtime/ext/ext_server.cpp b/hphp/runtime/ext/ext_server.cpp index 315de378b..eeaa32d2b 100644 --- a/hphp/runtime/ext/ext_server.cpp +++ b/hphp/runtime/ext/ext_server.cpp @@ -119,15 +119,18 @@ Resource f_pagelet_server_task_start(CStrRef url, CArrRef files /* = null_array */) { String remote_host; Transport *transport = g_context->getTransport(); + int timeout = ThreadInfo::s_threadInfo->m_reqInjectionData.getRemainingTime(); if (transport) { remote_host = transport->getRemoteHost(); if (!headers.exists(s_Host) && RuntimeOption::SandboxMode) { Array tmp = headers; tmp.set(s_Host, transport->getHeader("Host")); - return PageletServer::TaskStart(url, tmp, remote_host, post_data, files); + return PageletServer::TaskStart(url, tmp, remote_host, + post_data, files, timeout); } } - return PageletServer::TaskStart(url, headers, remote_host, post_data); + return PageletServer::TaskStart(url, headers, remote_host, + post_data, files, timeout); } int64_t f_pagelet_server_task_status(CResRef task) { @@ -135,7 +138,8 @@ int64_t f_pagelet_server_task_status(CResRef task) { } String f_pagelet_server_task_result(CResRef task, VRefParam headers, - VRefParam code, int64_t timeout_ms /* = 0 */) { + VRefParam code, + int64_t timeout_ms /* = 0 */) { Array rheaders; int rcode; String response = PageletServer::TaskResult(task, rheaders, rcode, diff --git a/hphp/runtime/server/http_request_handler.cpp b/hphp/runtime/server/http_request_handler.cpp index 40a52afef..b0586a18a 100644 --- a/hphp/runtime/server/http_request_handler.cpp +++ b/hphp/runtime/server/http_request_handler.cpp @@ -136,7 +136,7 @@ void HttpRequestHandler::handleRequest(Transport *transport) { // don't serve the request if it's been sitting in queue for longer than our // allowed request timeout. int requestTimeoutSeconds = - vhost->getRequestTimeoutSeconds(RuntimeOption::RequestTimeoutSeconds); + vhost->getRequestTimeoutSeconds(getDefaultTimeout()); if (requestTimeoutSeconds > 0) { timespec now; Timer::GetMonotonicTime(now); diff --git a/hphp/runtime/server/pagelet_server.cpp b/hphp/runtime/server/pagelet_server.cpp index f1f3f837f..07d2384fe 100644 --- a/hphp/runtime/server/pagelet_server.cpp +++ b/hphp/runtime/server/pagelet_server.cpp @@ -38,8 +38,11 @@ class PageletTransport : public Transport, public Synchronizable { public: PageletTransport(CStrRef url, CArrRef headers, CStrRef postData, CStrRef remoteHost, const set &rfc1867UploadedFiles, - CArrRef files) - : m_refCount(0), m_done(false), m_code(0) { + CArrRef files, int timeoutSeconds) + : m_refCount(0), + m_timeoutSeconds(timeoutSeconds), + m_done(false), + m_code(0) { Timer::GetMonotonicTime(m_queueTime); m_threadType = ThreadType::PageletThread; @@ -211,9 +214,11 @@ public: } } - timespec getStartTimer() const { return m_queueTime; } + const timespec& getStartTimer() const { return m_queueTime; } + int getTimeoutSeconds() const { return m_timeoutSeconds; } private: int m_refCount; + int m_timeoutSeconds; string m_url; HeaderMap m_requestHeaders; @@ -233,13 +238,31 @@ private: /////////////////////////////////////////////////////////////////////////////// +static int64_t to_ms(const timespec& ts) { + return ts.tv_sec * 1000 + (ts.tv_nsec / 1000000); +} + struct PageletWorker : JobQueueWorker { virtual void doJob(PageletTransport *job) { try { job->onRequestStart(job->getStartTimer()); - HttpRequestHandler(0).handleRequest(job); + int timeout = job->getTimeoutSeconds(); + if (timeout > 0) { + timespec ts; + Timer::GetMonotonicTime(ts); + int64_t delta_ms = + to_ms(job->getStartTimer()) + timeout * 1000 - to_ms(ts); + if (delta_ms > 500) { + timeout = (delta_ms + 500) / 1000; + } else { + timeout = 1; + } + } else { + timeout = 0; + } + HttpRequestHandler(timeout).handleRequest(job); job->decRefCount(); } catch (...) { Logger::Error("HttpRequestHandler leaked exceptions"); @@ -256,9 +279,9 @@ public: PageletTask(CStrRef url, CArrRef headers, CStrRef post_data, CStrRef remote_host, const std::set &rfc1867UploadedFiles, - CArrRef files) { + CArrRef files, int timeoutSeconds) { m_job = new PageletTransport(url, headers, remote_host, post_data, - rfc1867UploadedFiles, files); + rfc1867UploadedFiles, files, timeoutSeconds); m_job->incRefCount(); } @@ -318,7 +341,8 @@ void PageletServer::Stop() { Resource PageletServer::TaskStart(CStrRef url, CArrRef headers, CStrRef remote_host, CStrRef post_data /* = null_string */, - CArrRef files /* = null_array */) { + CArrRef files /* = null_array */, + int timeoutSeconds /* = -1 */) { { Lock l(s_dispatchMutex); if (!s_dispatcher) { @@ -331,7 +355,8 @@ Resource PageletServer::TaskStart(CStrRef url, CArrRef headers, } } PageletTask *task = NEWOBJ(PageletTask)(url, headers, remote_host, post_data, - get_uploaded_files(), files); + get_uploaded_files(), files, + timeoutSeconds); Resource ret(task); PageletTransport *job = task->getJob(); Lock l(s_dispatchMutex); diff --git a/hphp/runtime/server/pagelet_server.h b/hphp/runtime/server/pagelet_server.h index 721766bdb..15ca855b9 100644 --- a/hphp/runtime/server/pagelet_server.h +++ b/hphp/runtime/server/pagelet_server.h @@ -35,7 +35,8 @@ public: static Resource TaskStart(CStrRef url, CArrRef headers, CStrRef remote_host, CStrRef post_data = null_string, - CArrRef files = null_array); + CArrRef files = null_array, + int timeoutSeconds = -1); /** * Query if a task is finished. This is non-blocking and can be called as