-
-
Notifications
You must be signed in to change notification settings - Fork 185
Optimize broadcast handling and debounce WS session reloads #222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
2dad67c
8fc4e3d
dfc4189
a5443bf
3b2cc5d
74468af
28d011c
4d7d2bd
67e498d
14b3847
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,130 @@ | ||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @file Deferred broadcast queue that decouples WebSocket notifications from SQLite transactions. Enqueue type+ref pairs inside a transaction, then flush after commit — reducing lock hold time and deduplicating redundant broadcasts. | ||||||||||||||||||||||||
| * @author Son Nguyen <hoangson091104@gmail.com> | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const { stmts } = require("../db"); | ||||||||||||||||||||||||
| const { broadcast } = require("../websocket"); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Pending broadcast items. Each item is { type, ref } where ref contains | ||||||||||||||||||||||||
| * just enough info to load the full payload on flush (sessionId / agentId / inline data). | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| let pending = []; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Enqueue a broadcast to be sent later (outside the current transaction). | ||||||||||||||||||||||||
| * Call this inside `db.transaction()` instead of `broadcast()` directly. | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * @param {string} type - WebSocket message type ("session_updated", "agent_updated", etc.) | ||||||||||||||||||||||||
| * @param {object} ref - Reference to locate the data on flush. | ||||||||||||||||||||||||
| * For session/agent types: { sessionId } or { agentId } | ||||||||||||||||||||||||
| * For inline data (e.g. "new_event"): { data } — the full payload, no DB read needed | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function enqueue(type, ref) { | ||||||||||||||||||||||||
| pending.push({ type, ref }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Flush all pending broadcasts: deduplicate, load data from DB, | ||||||||||||||||||||||||
| * and call the real `broadcast()`. Should be called outside any transaction | ||||||||||||||||||||||||
| * (typically via `setImmediate` after `processEvent` returns). | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function flush() { | ||||||||||||||||||||||||
| if (pending.length === 0) return; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const items = pending; | ||||||||||||||||||||||||
| pending = []; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Deduplicate: for DB-backed types, keep only the last entry per (type, id) | ||||||||||||||||||||||||
| // but preserve the original insertion order. Inline-data types ("new_event") | ||||||||||||||||||||||||
| // are never deduped. | ||||||||||||||||||||||||
| // Two-pass approach: first pass records the last index for each dedup key; | ||||||||||||||||||||||||
| // second pass filters to only those indices (plus keyless inline items), | ||||||||||||||||||||||||
| // preserving the relative order items were enqueued in. | ||||||||||||||||||||||||
| const seen = new Map(); // key → last index | ||||||||||||||||||||||||
| for (let i = 0; i < items.length; i++) { | ||||||||||||||||||||||||
| const key = dedupeKey(items[i]); | ||||||||||||||||||||||||
| if (key) seen.set(key, i); // overwrite — keeps the last occurrence | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| const kept = new Set(seen.values()); // indices of dedup-able items to keep | ||||||||||||||||||||||||
| const deduped = []; | ||||||||||||||||||||||||
| for (let i = 0; i < items.length; i++) { | ||||||||||||||||||||||||
| const key = dedupeKey(items[i]); | ||||||||||||||||||||||||
| if (!key || kept.has(i)) { | ||||||||||||||||||||||||
| deduped.push(items[i]); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Send broadcasts (DB reads happen here, outside the transaction). | ||||||||||||||||||||||||
| // Each item is dispatched independently so a single failure doesn't | ||||||||||||||||||||||||
| // block the rest of the queue or crash the process. | ||||||||||||||||||||||||
| for (const item of deduped) { | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| const data = loadData(item); | ||||||||||||||||||||||||
| if (data !== undefined && data !== null) { | ||||||||||||||||||||||||
| broadcast(item.type, data); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||
| console.error("[broadcast-queue] flush error for %s:", item.type, err?.message || err); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Clear all pending items without sending. Useful for error recovery. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function clear() { | ||||||||||||||||||||||||
| pending = []; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Number of pending items (for diagnostics / testing). | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function size() { | ||||||||||||||||||||||||
| return pending.length; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||
| // Internal helpers | ||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Build a dedup key for DB-backed types. Returns null for inline-data types. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function dedupeKey(item) { | ||||||||||||||||||||||||
| const { type, ref } = item; | ||||||||||||||||||||||||
| if (!ref) return null; // defensive: null/undefined ref — treat as non-dedup-able | ||||||||||||||||||||||||
| if (ref.data !== undefined) return null; // inline payload, don't dedup | ||||||||||||||||||||||||
| if (ref.sessionId) return `${type}:session:${ref.sessionId}`; | ||||||||||||||||||||||||
| if (ref.agentId) return `${type}:agent:${ref.agentId}`; | ||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Load the full payload for a queued broadcast item. | ||||||||||||||||||||||||
| * DB-backed types fetch fresh data; inline types return ref.data directly. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function loadData(item) { | ||||||||||||||||||||||||
| const { type, ref } = item; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Defensive: null/undefined ref means nothing to load | ||||||||||||||||||||||||
| if (!ref) return null; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Inline payload — no DB read needed | ||||||||||||||||||||||||
| if (ref.data !== undefined) return ref.data; | ||||||||||||||||||||||||
|
Comment on lines
+108
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, if
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Added a defensive guard at the top of loadData():
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Added a defensive guard at the top of loadData(): if (!ref) return null; — a null/undefined ref returns null immediately, preventing TypeError on ref.data access. |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Session types | ||||||||||||||||||||||||
| if (ref.sessionId) { | ||||||||||||||||||||||||
| return stmts.getSession.get(ref.sessionId) || null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Agent types | ||||||||||||||||||||||||
| if (ref.agentId) { | ||||||||||||||||||||||||
| return stmts.getAgent.get(ref.agentId) || null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| module.exports = { enqueue, flush, clear, size }; | ||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
refis null or undefined, accessingref.datawill throw aTypeErrorand crash the execution. Adding a defensive guard at the beginning ofdedupeKeyprevents potential crashes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Added a defensive guard at the top of dedupeKey():
if (!ref) return null;— a null/undefined ref is treated as non-dedup-able (same as inline data), preventing TypeError onref.dataaccess.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Added a defensive guard at the top of dedupeKey(): if (!ref) return null; — a null/undefined ref is treated as non-dedup-able (same as inline data), preventing TypeError on ref.data access.