tbnl-devel

Download

tbnl-devel@common-lisp.net

July 2010

3 participants
1 discussions

[hunchentoot-devel] 'max-threads' behavior for Hunchentoot
by Scott McKay 13 Sep '10

13 Sep '10

Here are some changes I made to implement 'max-threads' semantics for Hunchentoot. I'd like [some version of] this functionality to be installed into Hunchentoot, so that it is a better behaved multi-threaded web server. A few notes: - The function conditionalized out with #+++potentially-faster-way is meant to be a hint as to how we might refuse the connection without invoking the overhead of accepting the over-the-limit connection. It might be slightly faster, but I don't know if I like the idea of constantly closing and reopening the listener. - 'pooled-thread-per-connection-taskmaster' is just a stub right now, although it provides a useful hook for future work. The idea is to use a thread pool, but on my box, thread creation takes about 250 usec, so it's not clear to me that it's worth the effort, particularly since Bordeaux threads doesn't have the necessary functionality to do it correctly. - 'handle-incoming-connection' on 'one-thread-per-connection-taskmaster' should really try to generate an HTTP 503 error, instead of just closing the connection. I tried several things to make this happen, but nothing seemed to work properly. It seems a shame to have to open the client connection, suck in the whole request, etc etc, just to do this. Is there a better way? Is there some sort of "connection refused" we can do at the socket level? Your review comments and any advice on the third issue would be greatly appreciated. Thanks! --Scott Modified: trunk/qres/lisp/libs/hunchentoot/packages.lisp ============================================================================== --- trunk/qres/lisp/libs/hunchentoot/packages.lisp (original) +++ trunk/qres/lisp/libs/hunchentoot/packages.lisp Thu May 27 10:31:21 2010 @@ -192,7 +192,6 @@ "MIME-TYPE" "NEXT-SESSION-ID" "NO-CACHE" - "ONE-THREAD-PER-CONNECTION-TASKMASTER" "PARAMETER" "PARAMETER-ERROR" "POST-PARAMETER" @@ -250,7 +249,6 @@ "SET-COOKIE" "SET-COOKIE*" "SHUTDOWN" - "SINGLE-THREADED-TASKMASTER" #-:hunchentoot-no-ssl "SSL-ACCEPTOR" "SSL-P" "START" @@ -259,7 +257,12 @@ "STOP" "TASKMASTER" "TASKMASTER-ACCEPTOR" - "URL-DECODE" + "SINGLE-THREADED-TASKMASTER" + "ONE-THREAD-PER-CONNECTION-TASKMASTER" + "POOLED-THREAD-PER-CONNECTION-TASKMASTER" + "INCREMENT-TASKMASTER-THREAD-COUNT" + "DECREMENT-TASKMASTER-THREAD-COUNT" + "URL-DECODE" "URL-ENCODE" "USER-AGENT")) Modified: trunk/qres/lisp/libs/hunchentoot/acceptor.lisp ============================================================================== --- trunk/qres/lisp/libs/hunchentoot/acceptor.lisp (original) +++ trunk/qres/lisp/libs/hunchentoot/acceptor.lisp Thu May 27 10:31:21 2010 @@ -86,7 +86,7 @@ reason to change this to NIL.") (input-chunking-p :initarg :input-chunking-p :accessor acceptor-input-chunking-p - :documentation "A generalized boolean denoting + :documentation "A generalized boolean denoting whether the acceptor may use chunked encoding for input, i.e. when accepting request bodies from the client. The default is T and there's usually no reason to change this to NIL.") @@ -117,8 +117,7 @@ process different from the one where START was called.") #-:lispworks (listen-socket :accessor acceptor-listen-socket - :documentation "The socket listening for incoming -connections.") + :documentation "The socket listening for incoming connections.") (acceptor-shutdown-p :initform nil :accessor acceptor-shutdown-p :documentation "A flag that makes the acceptor @@ -349,9 +348,12 @@ ;; the default is to always answer "no" nil) -;; usocket implementation + +;;; usocket implementation #-:lispworks +(progn + (defmethod start-listening ((acceptor acceptor)) (setf (acceptor-listen-socket acceptor) (usocket:socket-listen (or (acceptor-address acceptor) @@ -361,26 +363,61 @@ :element-type '(unsigned-byte 8))) (values)) -#-:lispworks (defmethod accept-connections ((acceptor acceptor)) (usocket:with-server-socket (listener (acceptor-listen-socket acceptor)) (loop - (when (acceptor-shutdown-p acceptor) - (return)) - (when (usocket:wait-for-input listener :timeout +new-connection-wait-time+) - (handler-case - (when-let (client-connection (usocket:socket-accept listener)) - (set-timeouts client-connection - (acceptor-read-timeout acceptor) - (acceptor-write-timeout acceptor)) - (handle-incoming-connection (acceptor-taskmaster acceptor) - client-connection)) - ;; ignore condition - (usocket:connection-aborted-error ())))))) + (when (acceptor-shutdown-p acceptor) + (return)) + (when (usocket:wait-for-input listener :timeout +new-connection-wait-time+) + (handler-case + (let ((taskmaster (acceptor-taskmaster acceptor))) + (when-let (client-connection (usocket:socket-accept listener)) + (set-timeouts client-connection + (acceptor-read-timeout acceptor) + (acceptor-write-timeout acceptor)) + ;; This will bail if the taskmaster has reached its thread limit + (handle-incoming-connection taskmaster client-connection))) + ;; Ignore the error + (usocket:connection-aborted-error ())))))) + +#+++potentially-faster-way +(defmethod accept-connections ((acceptor acceptor)) + (loop + (usocket:with-server-socket (listener (acceptor-listen-socket acceptor)) + (loop named waiter doing + (when (acceptor-shutdown-p acceptor) + (return-from accept-connections)) + (when (usocket:wait-for-input listener :timeout +new-connection-wait-time+) + (handler-case + (let ((taskmaster (acceptor-taskmaster acceptor))) + ;; Optimization to avoid creating the client connection: + ;; if the taskmaster has reached its thread limit, just close + ;; and reopen the listener socket, and don't even call 'accept' + (when (and (taskmaster-max-threads taskmaster) + (> (taskmaster-thread-count taskmaster) (taskmaster-max-threads taskmaster))) + (when-let (handler (taskmaster-too-many-threads-handler taskmaster)) + (funcall handler taskmaster listener)) + (usocket:socket-close listener) ;close the listener + (setq listener nil) + (start-listening acceptor) ;and start up a new one + (return-from waiter)) + (when-let (client-connection (usocket:socket-accept listener)) + (set-timeouts client-connection + (acceptor-read-timeout acceptor) + (acceptor-write-timeout acceptor)) + ;; This will bail if the taskmaster has reached its thread limit + (handle-incoming-connection taskmaster client-connection))) + ;; Ignore the error + (usocket:connection-aborted-error ()))))))) + +) ;#-:lispworks -;; LispWorks implementation + +;;; LispWorks implementation #+:lispworks +(progn + (defmethod start-listening ((acceptor acceptor)) (multiple-value-bind (listener-process startup-condition) (comm:start-up-server :service (acceptor-port acceptor) @@ -398,8 +435,8 @@ ;; is made :function (lambda (handle) (unless (acceptor-shutdown-p acceptor) - (handle-incoming-connection - (acceptor-taskmaster acceptor) handle))) + (let ((taskmaster (acceptor-taskmaster acceptor))) + (handle-incoming-connection taskmaster client-connection)))) ;; wait until the acceptor was successfully started ;; or an error condition is returned :wait t) @@ -409,11 +446,13 @@ (setf (acceptor-process acceptor) listener-process) (values))) -#+:lispworks (defmethod accept-connections ((acceptor acceptor)) (mp:process-unstop (acceptor-process acceptor)) nil) +) ;#+:lispworks + + (defun list-request-dispatcher (request) "The default request dispatcher which selects a request handler based on a list of individual request dispatchers all of which can Modified: trunk/qres/lisp/libs/hunchentoot/taskmaster.lisp ============================================================================== --- trunk/qres/lisp/libs/hunchentoot/taskmaster.lisp (original) +++ trunk/qres/lisp/libs/hunchentoot/taskmaster.lisp Thu May 27 10:31:21 2010 @@ -62,6 +62,21 @@ might terminate all threads that are currently associated with it. This function is called by the acceptor's STOP method.")) +;; Default method +(defmethod taskmaster-max-threads ((taskmaster taskmaster)) + nil) + +;; Default method +(defmethod taskmaster-thread-count ((taskmaster taskmaster)) + 0) + +(defmethod increment-taskmaster-thread-count ((taskmaster taskmaster)) + nil) + +(defmethod decrement-taskmaster-thread-count ((taskmaster taskmaster)) + nil) + + (defclass single-threaded-taskmaster (taskmaster) () (:documentation "A taskmaster that runs synchronously in the thread @@ -80,25 +95,95 @@ ;; in a single-threaded environment we just call PROCESS-CONNECTION (process-connection (taskmaster-acceptor taskmaster) socket)) + (defclass one-thread-per-connection-taskmaster (taskmaster) (#-:lispworks - (acceptor-process :accessor acceptor-process - :documentation "A process that accepts incoming -connections and hands them off to new processes for request -handling.")) + (acceptor-process + :accessor acceptor-process + :documentation + "A process that accepts incoming connections and hands them off to new processes + for request handling.") + (create-thread-function + :initarg :create-thread-function + :initform 'create-taskmaster-thread + :accessor taskmaster-create-thread-function + :documentation + "Function called to create the handler thread; + takes two arguments, the taskmaster and the socket") + ;; Support for bounding the number of threads we'll create + (max-threads + :type (or integer null) + :initarg :max-threads + :initform nil + :accessor taskmaster-max-threads) + (thread-count + :type integer + :initform 0 + :accessor taskmaster-thread-count) + (thread-count-lock + :initform (bt:make-lock "taskmaster-thread-count") + :accessor taskmaster-thread-count-lock) + (worker-thread-name-format + :type (or string null) + :initarg :worker-thread-name-format + :initform "hunchentoot-worker-~A" + :accessor taskmaster-worker-thread-name-format) + (too-many-threads-handler + :initarg :too-many-threads-handler + :initform nil + :accessor taskmaster-too-many-threads-handler + :documentation + "Function called with two arguments, the taskmaster and the socket, + when too many threads reached, just prior to closing the connection")) + (:default-initargs + :too-many-threads-handler 'log-too-many-threads) (:documentation "A taskmaster that starts one thread for listening -to incoming requests and one thread for each incoming connection. +to incoming requests and one new thread for each incoming connection. +If 'max-threads' is supplied, the number of threads is limited to that. This is the default taskmaster implementation for multi-threaded Lisp implementations.")) -;; usocket implementation +(defmethod increment-taskmaster-thread-count ((taskmaster one-thread-per-connection-taskmaster)) + (when (taskmaster-max-threads taskmaster) + (bt:with-lock-held ((taskmaster-thread-count-lock taskmaster)) + (incf (taskmaster-thread-count taskmaster))))) + +(defmethod decrement-taskmaster-thread-count ((taskmaster one-thread-per-connection-taskmaster)) + (when (taskmaster-max-threads taskmaster) + (bt:with-lock-held ((taskmaster-thread-count-lock taskmaster)) + (decf (taskmaster-thread-count taskmaster))))) + +(defun log-too-many-threads (taskmaster socket) + (declare (ignore socket)) + (let* ((acceptor (taskmaster-acceptor taskmaster)) + (logger (and acceptor (acceptor-message-logger acceptor)))) + (when logger + (funcall logger :warning "Can't handle a new connection, too many threads already")))) + + +;;--- If thread creation is too slow, it would be worth finishing this +;;--- For now, it's just a synonym for 'one-thread-per-connection-taskmaster' +(defclass pooled-thread-per-connection-taskmaster (one-thread-per-connection-taskmaster) + ((create-thread-function + :initarg :create-thread-function + :initform 'create-taskmaster-thread + :accessor taskmaster-create-thread-function + :documentation + "Function called to create the handler thread")) + (:documentation "A taskmaster that starts one thread for listening +to incoming requests and then uses a thread pool for each incoming connection. +If 'max-threads' is supplied, the number of threads is limited to that.")) + + +;;; usocket implementation #-:lispworks +(progn + (defmethod shutdown ((taskmaster taskmaster)) taskmaster) -#-:lispworks (defmethod shutdown ((taskmaster one-thread-per-connection-taskmaster)) ;; just wait until the acceptor process has finished, then return (loop @@ -107,16 +192,39 @@ (sleep 1)) taskmaster) -#-:lispworks (defmethod execute-acceptor ((taskmaster one-thread-per-connection-taskmaster)) (setf (acceptor-process taskmaster) - (bt:make-thread (lambda () - (accept-connections (taskmaster-acceptor taskmaster))) - :name (format nil "Hunchentoot listener \(~A:~A)" - (or (acceptor-address (taskmaster-acceptor taskmaster)) "*") - (acceptor-port (taskmaster-acceptor taskmaster)))))) + (bt:make-thread + (lambda () + (accept-connections (taskmaster-acceptor taskmaster))) + :name (format nil "hunchentoot-listener-~A:~A" + (or (acceptor-address (taskmaster-acceptor taskmaster)) "*") + (acceptor-port (taskmaster-acceptor taskmaster)))))) + +(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) + ;; Only take lock if necessary + (if (taskmaster-max-threads taskmaster) + (if (< (taskmaster-thread-count taskmaster) (taskmaster-max-threads taskmaster)) + (progn + (increment-taskmaster-thread-count taskmaster) + (funcall (taskmaster-create-thread-function taskmaster) taskmaster socket)) + (progn + (when-let (handler (taskmaster-too-many-threads-handler taskmaster)) + (funcall handler taskmaster socket)) + ;; Just close the socket, which will effectively abort the request + ;;--- It sure would be nice to be able to generate an HTTP 503 error, + ;;--- but I just can't seem to get that to work properly + (usocket:socket-close socket))) + (funcall (taskmaster-create-thread-function taskmaster) taskmaster socket))) + +(defun create-taskmaster-thread (taskmaster socket) + (bt:make-thread + (lambda () + (multiple-value-prog1 + (process-connection (taskmaster-acceptor taskmaster) socket) + (decrement-taskmaster-thread-count taskmaster))) + :name (format nil (taskmaster-worker-thread-name-format taskmaster) (client-as-string socket)))) -#-:lispworks (defun client-as-string (socket) "A helper function which returns the client's address and port as a string and tries to act robustly in the presence of network problems." @@ -127,15 +235,14 @@ (usocket:vector-quad-to-dotted-quad address) port)))) -#-:lispworks -(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) - (bt:make-thread (lambda () - (process-connection (taskmaster-acceptor taskmaster) socket)) - :name (format nil "Hunchentoot worker \(client: ~A)" (client-as-string socket)))) +) ;#-:lispworks + -;; LispWorks implementation +;;; LispWorks implementation #+:lispworks +(progn + (defmethod shutdown ((taskmaster taskmaster)) (when-let (process (acceptor-process (taskmaster-acceptor taskmaster))) ;; kill the main acceptor process, see LW documentation for @@ -143,20 +250,38 @@ (mp:process-kill process)) taskmaster) -#+:lispworks (defmethod execute-acceptor ((taskmaster one-thread-per-connection-taskmaster)) (accept-connections (taskmaster-acceptor taskmaster))) -#+:lispworks -(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) handle) +(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) (incf *worker-counter*) ;; check if we need to perform a global GC (when (and *cleanup-interval* (zerop (mod *worker-counter* *cleanup-interval*))) (when *cleanup-function* (funcall *cleanup-function*))) - (mp:process-run-function (format nil "Hunchentoot worker \(client: ~{~A:~A~})" - (multiple-value-list - (get-peer-address-and-port handle))) - nil #'process-connection - (taskmaster-acceptor taskmaster) handle)) + (if (taskmaster-max-threads taskmaster) + (if (< (taskmaster-thread-count taskmaster) (taskmaster-max-threads taskmaster)) + (progn + (increment-taskmaster-thread-count taskmaster) + (funcall (taskmaster-create-thread-function taskmaster) taskmaster socket)) + ;; With any luck, we never get this far if we've exceeded the thread count + ;; "Good" implementations of 'accept-connections' won't even accept connection requests + (progn + (when-let (handler (taskmaster-too-many-threads-handler taskmaster)) + (funcall handler taskmaster socket)) + (usocket:socket-close socket))) + (funcall (taskmaster-create-thread-function taskmaster) taskmaster socket))) + +(defun create-taskmaster-thread (taskmaster socket) + (flet ((process (taskmaster sock) + (multiple-value-prog1 + (process-connection (taskmaster-acceptor taskmaster) socket) + (decrement-taskmaster-thread-count taskmaster)))) + (mp:process-run-function (format nil "hunchentoot-worker-{~A:~A~})" + (multiple-value-list + (get-peer-address-and-port socket))) + nil #'process taskmaster socket))) + +) ;#+:lispworks +

6 43