Force-close connections during HTTP/HTTPS server shutdown#468
Force-close connections during HTTP/HTTPS server shutdown#468
Conversation
Previously, HTTP and HTTPS wranglers would close idle connections during shutdown but leave active connections to timeout naturally (up to 3 minutes). This adds a 250ms grace period after which remaining connections are force-destroyed, matching HTTP2 behavior.
|
The main "gate" on addressing this TODO is testability. The unstated (in the code) prerequisite TODO is: Figure out a good way to do unit tests around network functionality. To be clear, the proposed change looks pretty reasonable. I just want to see a test that demonstrates that it does what it says. Thanks! |
Address review feedback requesting testability for the HTTP connection shutdown logic. Add _testing_addSocket() method to TcpWrangler to enable injecting mock sockets for testing. Create TcpWrangler.test.js with tests covering: - Immediate return when no sockets present - Early return when sockets close before grace period - Force-destroy of lingering sockets after grace period - Handling mix of early and late socket closures - Skip of already-destroyed sockets
|
Does the current state sufficiently address your concerns? |
'Fraid not. Check out |
2 participants
Summary
closeIdleConnections(), waits up to 250ms for active connections to drain_prot_closeSocketsWithGracePeriod()helper method to TcpWrangler for use by subclassesThis addresses the TODO comments in HttpWrangler.js and HttpsWrangler.js about tracking connections and forcing them closed after a timeout.
Note: This PR is based on #467 (Allow Node 25) and should be merged after that PR.
Test plan
ubik dev lintpassesubik run-tests --do=buildpasses (unit tests)ubik run-tests --type=integrationpasses