From e88aa51627a727d1883d1d803c9674d92dac643c Mon Sep 17 00:00:00 2001
From: Luke Hoersten <luke@hoersten.org>
Date: Fri, 19 Jun 2026 19:44:08 -0500
Subject: scrypted: noDelay:true on keep-alive sockets — kills 200ms Nagle
 deadlock
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After enabling keep-alive in bccf0fa, fw_recv p95 spiked from ~25ms
to ~230ms on a significant fraction of frames and net_up p95 went
from <15ms to 200+ms simultaneously. Worst frames hit 467ms wall.

Root cause: Node http.Agent's outgoing sockets default to noDelay
false (Nagle ON). When a ~128KB JPEG body doesn't end on an exact
MTU boundary, the sender holds the final partial packet for up to
200ms waiting for either more data or a peer ACK. Firmware-side
TCP_NODELAY only controls what the firmware *sends* (its ACKs and
response packets); it has no effect on what arrives at the firmware
from a Nagling sender.

Setting noDelay: true on the http.Agent makes new sockets in the pool
opt out of Nagle on creation. Plus a belt-and-suspenders setNoDelay
on the request's socket event in case the Agent-level option isn't
honored on this Node version.

The 230ms signature is the classic Nagle + delayed-ACK deadlock:
- Sender: "I have a partial packet; I'll wait for more data or an
  ACK before sending."
- Receiver: "I'll delay this ACK up to 200ms in case I can piggyback
  it on a response."
- Result: 200ms stall on every send that doesn't fill an MTU.
---
 scrypted/scrypted-viewport.ts | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

(limited to 'scrypted')

diff --git a/scrypted/scrypted-viewport.ts b/scrypted/scrypted-viewport.ts
index df6c041..344d58c 100644
--- a/scrypted/scrypted-viewport.ts
+++ b/scrypted/scrypted-viewport.ts
@@ -6,7 +6,7 @@
 // short git hash of the commit that added this constant — if the
 // hash in the log doesn't match the HEAD this file came from, the
 // Scrypted Script editor is still on stale code.
-const SCRIPT_VERSION = "1063a4e";
+const SCRIPT_VERSION = "pending";
 //
 // Architecture
 // ------------
@@ -338,6 +338,16 @@ class ScryptedViewportProvider extends ScryptedDeviceBase
                 maxSockets:       2,
                 maxFreeSockets:   2,
                 timeout:          30_000,
+                // CRITICAL: without this, the keep-alive socket inherits
+                // Nagle ON by default. When a 128KB JPEG body doesn't
+                // end on an MTU boundary, the kernel sits on the final
+                // partial packet for up to 200ms (delayed-ACK window)
+                // waiting for either more data or an ACK. Firmware-side
+                // TCP_NODELAY only affects firmware's sends; the sender
+                // also has to opt out. Without noDelay we measured
+                // fw_recv p95 spiking from ~25ms to ~230ms — the exact
+                // 200ms Nagle+delayed-ACK deadlock signature.
+                noDelay:          true,
             });
             this.agents.set(host, a);
         }
@@ -387,6 +397,12 @@ class ScryptedViewportProvider extends ScryptedDeviceBase
             }
             req.on("timeout", () => req.destroy(new Error("request timeout")));
             req.on("error", reject);
+            // Belt-and-suspenders: in case the Agent's noDelay option
+            // isn't honored on every Node version, force it on the
+            // socket as soon as it's allocated. Without this the
+            // socket inherits Nagle ON and a JPEG body ending mid-MTU
+            // stalls for the 200ms delayed-ACK window.
+            req.on("socket", (s: any) => { try { s.setNoDelay(true); } catch {} });
             if (opts.body != null) req.write(opts.body);
             req.end();
         });
-- 
cgit v1.2.3