From 6a4f8650a8f55400a2d57d85401971e846980b53 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Wed, 16 Nov 2016 15:19:31 -0800 Subject: [PATCH 1/3] More control over http keepalive and sockjs timeouts --- NEWS | 7 +++++++ config/shiny-server-rules.config | 21 +++++++++++++++++++++ lib/main.js | 9 +++++++-- lib/proxy/sockjs.js | 15 +++++++++++++-- lib/router/config-router.js | 15 +++++++++++++++ 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 9161d8da..f8575c16 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,10 @@ +shiny-server 1.5.2 +-------------------------------------------------------------------------------- + +* Add additional configuration directives `http_keepalive_timeout`, + `sockjs_heartbeat_delay`, and `sockjs_disconnect_delay` to allow working + with very slow connections and large SockJS payloads. + shiny-server 1.5.1 -------------------------------------------------------------------------------- diff --git a/config/shiny-server-rules.config b/config/shiny-server-rules.config index a39a5189..1ce38dec 100644 --- a/config/shiny-server-rules.config +++ b/config/shiny-server-rules.config @@ -141,6 +141,27 @@ app_idle_timeout { maxcount 1; } +http_keepalive_timeout { + desc "Defines how long a keepalive connection will sit between HTTP requests/responses before it is closed. Defaults to 45 seconds."; + param Float timeout "The number of seconds to keep a connection alive between requests/responses."; + at $; + maxcount 1; +} + +sockjs_heartbeat_delay { + desc "How often the SockJS server should send heartbeat packets to the server. These are used to prevent proxies and load balancers from closing active SockJS connections. Defaults to 25 seconds."; + param Float delay "The number of seconds to wait between heartbeat packets."; + at $; + maxcount 1; +} + +sockjs_disconnect_delay { + desc "How long the SockJS server should wait between HTTP requests before considering the client to be disconnected."; + param Float delay "The number of seconds to wait before giving up."; + at $; + maxcount 1; +} + simple_scheduler { desc "A basic scheduler which will spawn one single-threaded R worker for each application. If no scheduler is specified, this is the default scheduler."; param Integer [maxRequests] "The maximum number of requests to assign to this scheduler before it should start returning rejecting incoming traffic using a '503 - Service Unavailable' message. Once this threshold is hit, users attempting to initialize a new session will receive 503 errors." 100; diff --git a/lib/main.js b/lib/main.js index c7b56ea1..2d4c3f04 100755 --- a/lib/main.js +++ b/lib/main.js @@ -178,6 +178,8 @@ app.use(connect_util.filterByRegex( )); app.use(shinyProxy.httpListener); +var socketTimeout = 45 * 1000; + // Now create a server and hook everything up. var server = new Server(); server.on('connection', function(socket) { @@ -186,7 +188,7 @@ server.on('connection', function(socket) { // SockJS sends a heartbeat every 25s so as long as we wait significantly // longer than that to timeout, we shouldn't need to worry about closing // active connections. - socket.setTimeout(45 * 1000); + socket.setTimeout(socketTimeout); }); server.on('request', _.bind(app.handle, app)); server.on('error', function(err) { @@ -226,9 +228,12 @@ var loadConfig_p = qutil.serialized(function() { transport.setSocketDir(configRouter.socketDir); // Create SockJS server - sockjsServer = proxy_sockjs.createServer(metarouter, schedulerRegistry); + sockjsServer = proxy_sockjs.createServer(metarouter, schedulerRegistry, + configRouter.sockjsHeartbeatDelay, configRouter.sockjsDisconnectDelay); sockjsHandler = sockjsServer.middleware(); + socketTimeout = configRouter.httpKeepaliveTimeout; + return createLogger_p(configRouter.accessLogSpec) .then(function(logfunc) { requestLogger = logfunc; diff --git a/lib/proxy/sockjs.js b/lib/proxy/sockjs.js index 5a0b4772..f13076d0 100644 --- a/lib/proxy/sockjs.js +++ b/lib/proxy/sockjs.js @@ -22,7 +22,16 @@ var RobustSockJS = require('./robust-sockjs'); var errorcode = require("./errorcode"); exports.createServer = createServer; -function createServer(router, schedulerRegistry) { +function createServer(router, schedulerRegistry, heartbeatDelay, disconnectDelay) { + if (!heartbeatDelay || heartbeatDelay < 0) { + logger.warn("Ignoring invalid SockJS heartbeat delay: " + heartbeatDelay); + heartbeatDelay = 25 * 1000; + } + if (!disconnectDelay || disconnectDelay < 0) { + logger.warn("Ignoring invalid SockJS disconnect delay: " + disconnectDelay); + disconnectDelay = 5 * 1000; + } + // Create a single SockJS server that will serve all applications. We'll use // the connection.url to dispatch among the different worker processes' // websocket ports. Once a connection is established, we simply pipe IO @@ -31,7 +40,9 @@ function createServer(router, schedulerRegistry) { // TODO: make URL configurable sockjs_url: '//d1fxtkz8shb9d2.cloudfront.net/sockjs-0.3.min.js', prefix: '.*/__sockjs__(/[no]=\\w+)?', - log: function() {} + log: function() {}, + heartbeat_delay: heartbeatDelay, + disconnect_delay: disconnectDelay }); var robust = new RobustSockJS(); diff --git a/lib/router/config-router.js b/lib/router/config-router.js index 9aac94ef..b759bbaf 100644 --- a/lib/router/config-router.js +++ b/lib/router/config-router.js @@ -108,6 +108,21 @@ function ConfigRouter(conf, schedulerRegistry) { this.$allowAppOverride = conf.getValues('allow_app_override').enabled; this.$templateDir = conf.getValues('template_dir').dir; + this.httpKeepaliveTimeout = 45 * 1000; + if (conf.getOne('http_keepalive_timeout')) { + this.httpKeepaliveTimeout = conf.getValues('http_keepalive_timeout').timeout * 1000; + } + + this.sockjsHeartbeatDelay = 25 * 1000; + if (conf.getOne('sockjs_heartbeat_delay')) { + this.sockjsHeartbeatDelay = conf.getValues('sockjs_heartbeat_delay').delay * 1000; + } + + this.sockjsDisconnectDelay = 5 * 1000; + if (conf.getOne('sockjs_disconnect_delay')) { + this.sockjsDisconnectDelay = conf.getValues('sockjs_disconnect_delay').delay * 1000; + } + var apps = conf.search("application", true); if (apps && apps.length > 0){ logger.error("The `application` configuration has been deprecated. Please "+ From f480600b36cc905dba8c837b369fe5e0f4be9f23 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Thu, 17 Nov 2016 16:26:50 -0800 Subject: [PATCH 2/3] Code review feedback --- NEWS | 3 ++- config/shiny-server-rules.config | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index f8575c16..47d62d2f 100644 --- a/NEWS +++ b/NEWS @@ -3,7 +3,8 @@ shiny-server 1.5.2 * Add additional configuration directives `http_keepalive_timeout`, `sockjs_heartbeat_delay`, and `sockjs_disconnect_delay` to allow working - with very slow connections and large SockJS payloads. + with very slow connections and large SockJS payloads. (The default values + for these options are the same as in previous versions of Shiny Server.) shiny-server 1.5.1 -------------------------------------------------------------------------------- diff --git a/config/shiny-server-rules.config b/config/shiny-server-rules.config index 1ce38dec..bae68678 100644 --- a/config/shiny-server-rules.config +++ b/config/shiny-server-rules.config @@ -156,7 +156,7 @@ sockjs_heartbeat_delay { } sockjs_disconnect_delay { - desc "How long the SockJS server should wait between HTTP requests before considering the client to be disconnected."; + desc "How long the SockJS server should wait between HTTP requests before considering the client to be disconnected. Defaults to 5 seconds. If this value needs to be adjusted above 10 seconds, it's a good idea to disable websockets using the `disable_websockets` directive, as that transport protocol has an effective 10 second limit built in."; param Float delay "The number of seconds to wait before giving up."; at $; maxcount 1; From c27bb66058637b0eae8ce72a680e9f6634bbb4da Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Thu, 17 Nov 2016 16:43:36 -0800 Subject: [PATCH 3/3] Add detailed comment about socket.setTimeout --- lib/main.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/main.js b/lib/main.js index 2d4c3f04..925fe3e7 100755 --- a/lib/main.js +++ b/lib/main.js @@ -188,6 +188,13 @@ server.on('connection', function(socket) { // SockJS sends a heartbeat every 25s so as long as we wait significantly // longer than that to timeout, we shouldn't need to worry about closing // active connections. + // + // jcheng 11/17/2016: This doesn't work as well as you'd think. The timeout + // timer starts at e.g. the last invocation of write(), not waiting for + // that write to actually complete. In other words, there can be actual + // activity happening over the socket and yet the timeout can be hit. It's + // unclear whether the Node maintainers consider this a bug or not. See + // PR @rstudio/shiny-server#264 for all the gory details. socket.setTimeout(socketTimeout); }); server.on('request', _.bind(app.handle, app));