p24-infra Docs

w4-socketio-pm2-cluster-remediation

7 min read

W4 (v42-prod) Socket.IO ⇄ PM2-cluster remediation — analysis & phased plan

Issue: radieu/p24-infra#4553 Stack: W4 backend v42-prod on bms-1 — GitLab gitlab.com/pinbox24/p24-back-ts (development) Status: analysis complete; production remediation is human-gated (see §6) Prepared by: infra-task worker on bms-4, 2026-08-12 (read-only clone of p24-back-ts@development, commit a94400a)

⚠️ This document exists because #4553’s originally-prescribed fix (“add socket.io-redis + a sticky-session module”) is partially incorrect and unsafe as written against the code that is actually deployed. Read §2 before writing any diff. Do not naively add socket.io-redis.

1. Context

  • v42-prod runs socket.io@2.2.0 under PM2 cluster mode, 3 instances (re-enabled 2026-07-12 as a live prod test, monitored via #2542 — no crash-loop recurrence observed through 2026-07-29).
  • The original P0 (2026-07-06/07, docs/pinbox24/incident-2026-07-06-w4-pm2-crash-inline-preview.md, PR #3161) was a crash from io.sockets[userSocketId] resolving to undefined on a worker that did not hold that socket. Mitigation at the time: drop to PM2 fork mode, 1 instance.
  • #4553 tracks the underlying architectural gap (sticky sessions + cross-worker socket state), not the crash symptom.

2. What the issue got wrong — the code already has a cross-worker layer

The single most important finding: p24-back-ts already implements a hand-rolled cross-worker broadcast via node-redis-pubsub (NRP, ^4.0.0) in src/globalHelpers/socketHelper/socket.helper.ts:

emitToAllInChannel(channel, data)                    // ~40 call sites across controllers
  → socketDispatchChannelHelper(...)
  → NRP client.emit("onChannelTrigger", <json>)      // publish to Redis
        │  (fan-out to every PM2 worker, incl. the publisher)
        ▼
  every worker's  client.on("onChannelTrigger", ...) → io.to(channel).emit(eventName, data)

Because every worker receives the pub/sub message and runs io.to(channel).emit() against its own locally-connected room members, room/channel broadcasts already reach sockets on all 3 workers. This is functionally the same fan-out that socket.io-redis would provide for the broadcast path.

Consequences for the prescribed fix:

  • Issue step #2 (“add socket.io-redis”) is unsafe as written. Installing the official adapter makes each io.to(room).emit() itself fan out to all workers. The NRP handler already calls io.to(room).emit() on every worker, so with the adapter also installed each of those N calls re-broadcasts to all N workers → N×N duplicate delivery (duplicate chat lines, duplicate notifications). Adopting socket.io-redis therefore requires removing the NRP layer in the same change — a substantially larger refactor than “add a dependency”, and one that alters every one of the ~40 emitToAllInChannel call sites’ delivery semantics. This is an architect decision, not a mechanical patch.
  • The room-broadcast delivery the issue feared is silently failing (“a user’s socket lives on worker A, an emit from worker B never reaches them”) is already handled by NRP for the emitToAllInChannel path. If silent missed-notification reports exist, NRP pub/sub health (Redis connectivity, the REDIS_HOST/REDIS_PASSWORD env the NRP client uses) is the first thing to check — not the absence of socket.io-redis.

3. The genuine residual hazard — unguarded io.sockets[socketId]

The real cross-worker defect is direct socket access by id, which only works when the socket lives in the current worker’s memory. 12 sites:

FileLinesGuarded against undefined?
src/globalHelpers/socketHelper/socket.helper.ts59, 63, 71, 76✅ already wrapped in if (io.sockets[socketId])
src/globalHelpers/socketHelper/socketListener.helper.ts17, 49, 93, 127❌ unguarded
src/globalHelpers/socketHelper/socketInitUser.helper.ts19, 22, 28❌ unguarded (reads socketId from Redis)

In the normal connect flow these ids are local (the same worker that accepted the connection wrote the Redis KV and calls listenToUserPrivateEvent(socket.id, …)), so they resolve. The crash/silent-failure window is:

  • Multi-device / rapid-reconnect races — two connections for one user land on two different workers; redisClient.set(id, {socketId,…}) from worker B is read back by worker A’s socketInitUserHelper, so io.sockets[<B's socketId>] is undefined on worker A.
  • Any future code path that passes a socket id sourced from Redis / another worker into listenToUserPrivateEvent / listenToRoomChatEvents.

socketListener.helper.ts:17 (io.sockets[userSocketId].on(...)) is the exact shape of the original P0 crash.

4. Sticky sessions under PM2 cluster — why it is not an in-repo one-liner

Sticky sessions are needed so that the engine.io long-polling handshake (multiple HTTP requests that must hit the same worker) is not round-robined across PM2 workers. Options, none trivial:

  1. Proxy-level affinity (recommended, but it is an infra change, not a p24-back-ts change). Add ip_hash (or a cookie-hash) upstream in the bms-1 nginx-proxy in front of v42-prod. This is the cleanest fix and keeps PM2 cluster mode, but it touches shared front-proxy config (blast radius across vhosts) and must be done as a coordinated sys-admin change on bms-1.
  2. @socket.io/sticky / socket.io-sticky-session — both require the app to own the Node cluster master (setupMaster/setupWorker). PM2 cluster mode is the master, so these cannot be wired in without ejecting from PM2 cluster mode into an app-managed cluster — a deployment-model change.
  3. transports: ['websocket'] only — drops the polling transport entirely, sidestepping the handshake-stickiness problem. app.ts:137 currently sets transports: ["websocket", "polling"]. Websocket does not need stickiness once upgraded, but removing polling risks clients on networks/proxies that block raw websockets. Requires client-compat verification.

There is no purely in-p24-back-ts code change that delivers real sticky sessions while staying in PM2 cluster mode. This is the core reason the task cannot be fully shipped by a code MR alone.

Phase 1 — crash-proofing (low-risk, in-repo, ship first). Nil-guard the 7 unguarded io.sockets[socketId] sites in socketListener.helper.ts and socketInitUser.helper.ts, matching the pattern socket.helper.ts already uses. This removes the P0 crash class entirely with zero broadcast regression (broadcasts flow through NRP regardless of local socket presence). Ready-to-apply shape:

// socketListener.helper.ts — listenToUserPrivateEvent / listenToRoomChatEvents
const target = io.sockets[userSocketId];
if (!target) { return; }            // cross-worker / stale id — skip, never crash
target.on(channelName, async (data) => { /* … unchanged … */ });
 
// socketInitUser.helper.ts
const s = io.sockets[userData.socketId];
if (s) { s.join(`user_${userData._id}`); /* … */ }

Phase 2 — sticky sessions (infra, coordinated). Add ip_hash upstream affinity for v42-prod in the bms-1 nginx-proxy, OR drop to transports: ['websocket'] after client-compat verification. Do in a deploy window; validate polling→websocket handshakes land on one worker (process.pid logging already present in socket.middleware).

Phase 3 — adapter decision (architect). Decide whether to (a) keep NRP as the cross-worker broadcast layer (it works; document it as the intended design) or (b) replace NRP with socket.io-redis@^5 (the 2.x-compatible line) and delete the NRP onChannelTrigger handler and all socketDispatchChannelHelper indirection in the same change. Never run both (see §2, N×N duplication). Option (a) is lower-risk and is the default recommendation unless NRP is shown to be the source of concrete missed-delivery reports.

6. Why this is human-gated (not shipped autonomously)

The infra-task worker on bms-4 holds the capability to clone/branch/push/merge pinbox24/p24-back-ts and to SSH bms-1 (per docs/w3-w4-stack-operations.md §2). It is not a capability gap that blocks autonomous shipping — it is a production-risk + correctness judgment, which the 2026-08-12 reroute comment explicitly authorized escalating (“If the infra-task worker judges the production risk too high to ship autonomously, escalate to human-action at that point”). Specifically:

  • The prescribed fix is partly wrong (§2); the correct path needs an architect decision between keep-NRP vs replace-with-adapter, and a sticky-session approach decision that is an infra change (front-proxy) or a deployment-model change, not a code MR.
  • Deploy-coupling check (per docs/playbooks/pinbox24-s3v2-master-branch-autodeploy.md): p24-back-ts .gitlab-ci.yml — merging to development runs stage-back-end-deploy (stage: deploy, only: development, no when: → auto) → deploys to stage, not prod. The prod job prod-back-end-deploy is when: manual. So a merge does not auto-deploy prod, but reaching v42-prod (the P0-history container) is a deliberate manual step that needs a human-accepted window and Pinbox24-team coordination given the incident history.
  • There is no readily-scriptable way for a light worker to run the issue’s mandatory test plan (multiple simultaneous sockets across all 3 PM2 workers + cross-worker room-broadcast verification) before a prod deploy.

Recommended next action: apply Phase 1 (crash-proofing) via a dev-team / scoped GitLab session as a standalone MR (safe, no behavior change beyond removing crashes), then take Phases 2–3 as an architect + sys-admin coordinated change in a deploy window.

Graph View