mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
fix(login-perf): kill relay REQ storms and feedback loops
Real-environment console logs revealed nip-07 login pegging the CPU and starving encrypted-list decryption via relay REQ storms (~1000 REQ/s): - moderationService.rebuildTrustedContacts emitted "contacts" unconditionally, forming an emit->fetch->emit loop (~1600 kind-3 REQ/s). Now emits only on actual change. Regression test added. - Per-video live report (kind 1984) and view-count (kind 30079) subscriptions fan out O(videos x relays) and re-fire on every flaky-relay reconnect. Circuit-broken behind flags until batched (see refactor plan). - onVideosShouldRefresh now coalesces forced reloads (2s leading-edge throttle) so list-change signals can't drive a reload loop. - nip-07 queue reserves one foreground slot; DM decryption demoted to LOW so background DM backfill can't starve feed-driving lists. Test added. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
4bd9503caf
commit
a3b205bfa1
7 files changed
+293
-5
No files matched your search
@@ -199,6 +199,11 @@ const UNSUPPORTED_BTITH_MESSAGE =
|
||||
const VIDEO_EVENT_KIND = 30078;
|
||||
|
||||
const RELAY_UI_BATCH_DELAY_MS = 250;
|
||||
|
||||
// Minimum gap between forced full-feed reloads triggered by list-change signals
|
||||
// (blocks/subscriptions/hashtag prefs). Coalesces bursts so they cannot drive a
|
||||
// self-sustaining reload loop. See onVideosShouldRefresh.
|
||||
const VIDEO_REFRESH_COOLDOWN_MS = 2000;
|
||||
/**
|
||||
* Simple IntersectionObserver-based lazy loader for images (or videos).
|
||||
*
|
||||
@@ -2499,6 +2504,33 @@ class Application {
|
||||
}
|
||||
|
||||
async onVideosShouldRefresh({ reason } = {}) {
|
||||
// Coalesce bursts of "should refresh" signals into at most one forced reload
|
||||
// per cooldown window. List loads (blocks, subscriptions, hashtag prefs) each
|
||||
// emit a change that lands here, and a forced reload re-fetches those very
|
||||
// lists — under relay load they re-emit "change", which without this throttle
|
||||
// produced a self-sustaining feed-reload loop that flooded relays with REQs
|
||||
// (moderation/view re-subscriptions) and pegged the CPU. Leading edge runs
|
||||
// immediately so the UI stays responsive; further signals within the window
|
||||
// collapse into a single trailing reload.
|
||||
const now = Date.now();
|
||||
const cooldown = VIDEO_REFRESH_COOLDOWN_MS;
|
||||
if (
|
||||
this._lastShouldRefreshAt &&
|
||||
now - this._lastShouldRefreshAt < cooldown
|
||||
) {
|
||||
this._pendingShouldRefreshReason = reason || this._pendingShouldRefreshReason;
|
||||
if (!this._shouldRefreshTrailingTimer) {
|
||||
const wait = Math.max(50, cooldown - (now - this._lastShouldRefreshAt));
|
||||
this._shouldRefreshTrailingTimer = setTimeout(() => {
|
||||
this._shouldRefreshTrailingTimer = null;
|
||||
const pendingReason = this._pendingShouldRefreshReason;
|
||||
this._pendingShouldRefreshReason = null;
|
||||
this.onVideosShouldRefresh({ reason: pendingReason });
|
||||
}, wait);
|
||||
}
|
||||
return;
|
||||
}
|
||||
this._lastShouldRefreshAt = now;
|
||||
try {
|
||||
await this.refreshAllVideoGrids({ reason, forceMainReload: true });
|
||||
} catch (error) {
|
||||
|
||||
+8
-2
@@ -1252,7 +1252,12 @@ export class NostrClient {
|
||||
(pubkey, ciphertext, options) =>
|
||||
activeSigner.nip44Decrypt(pubkey, ciphertext, {
|
||||
...options,
|
||||
priority: NIP07_PRIORITY.NORMAL,
|
||||
// DM decryption is background-class ("load whenever"): it must
|
||||
// never compete with the feed-driving lists (hashtags,
|
||||
// subscriptions, watch history) or block/moderation lists for the
|
||||
// serialized extension. LOW confines the DM backfill flood to the
|
||||
// single unreserved queue slot. See nip07Permissions.js.
|
||||
priority: NIP07_PRIORITY.LOW,
|
||||
}),
|
||||
{
|
||||
priority: -20,
|
||||
@@ -1270,7 +1275,8 @@ export class NostrClient {
|
||||
(pubkey, ciphertext, options) =>
|
||||
activeSigner.nip04Decrypt(pubkey, ciphertext, {
|
||||
...options,
|
||||
priority: NIP07_PRIORITY.NORMAL,
|
||||
// Background-class — see the nip44 candidate above.
|
||||
priority: NIP07_PRIORITY.LOW,
|
||||
}),
|
||||
{
|
||||
priority: -10,
|
||||
|
||||
@@ -41,11 +41,27 @@ export const NIP07_PRIORITY = Object.freeze({
|
||||
});
|
||||
|
||||
class Nip07RequestQueue {
|
||||
constructor(maxConcurrent = 2) {
|
||||
// `reservedForForeground` slots are kept available for non-background
|
||||
// (priority > LOW) work, so a flood of LOW-priority background decrypts
|
||||
// (DM backfill, watch-history months) can never occupy every slot and
|
||||
// starve the feed-driving lists. Background work is therefore capped at
|
||||
// `maxConcurrent - reservedForForeground` concurrent slots (min 1).
|
||||
constructor(maxConcurrent = 2, reservedForForeground = 1) {
|
||||
this.queue = [];
|
||||
this.maxConcurrent =
|
||||
Number.isFinite(maxConcurrent) && maxConcurrent > 0 ? maxConcurrent : 2;
|
||||
const reserved =
|
||||
Number.isFinite(reservedForForeground) && reservedForForeground >= 0
|
||||
? Math.floor(reservedForForeground)
|
||||
: 1;
|
||||
// Never reserve so much that background work could never run.
|
||||
this.backgroundConcurrencyCap = Math.max(1, this.maxConcurrent - reserved);
|
||||
this.activeCount = 0;
|
||||
this.backgroundActiveCount = 0;
|
||||
}
|
||||
|
||||
isBackground(priority) {
|
||||
return Number.isFinite(priority) && priority <= NIP07_PRIORITY.LOW;
|
||||
}
|
||||
|
||||
enqueue(task, priority = NIP07_PRIORITY.NORMAL) {
|
||||
@@ -72,10 +88,27 @@ class Nip07RequestQueue {
|
||||
if (this.activeCount >= this.maxConcurrent) return;
|
||||
|
||||
while (this.activeCount < this.maxConcurrent && this.queue.length > 0) {
|
||||
// Peek the highest-priority item. The queue is sorted priority-desc, so
|
||||
// if the head is background-class and the background cap is already
|
||||
// saturated, every remaining item is background too — stop, keeping the
|
||||
// free slot(s) reserved for any foreground work that arrives later.
|
||||
const head = this.queue[0];
|
||||
const headIsBackground = this.isBackground(head.priority);
|
||||
if (
|
||||
headIsBackground &&
|
||||
this.backgroundActiveCount >= this.backgroundConcurrencyCap
|
||||
) {
|
||||
break;
|
||||
}
|
||||
|
||||
const item = this.queue.shift();
|
||||
if (!item) break;
|
||||
|
||||
const itemIsBackground = this.isBackground(item.priority);
|
||||
this.activeCount++;
|
||||
if (itemIsBackground) {
|
||||
this.backgroundActiveCount++;
|
||||
}
|
||||
const { task, resolve, reject } = item;
|
||||
|
||||
// We don't await the task here to allow parallelism loop to continue
|
||||
@@ -89,6 +122,9 @@ class Nip07RequestQueue {
|
||||
})
|
||||
.finally(() => {
|
||||
this.activeCount--;
|
||||
if (itemIsBackground) {
|
||||
this.backgroundActiveCount--;
|
||||
}
|
||||
this.process();
|
||||
});
|
||||
}
|
||||
|
||||
@@ -57,6 +57,12 @@ import {
|
||||
* are determined by aggregating reports (NIP-56) and mutes (NIP-51) from the user's
|
||||
* trusted social graph (follows).
|
||||
*/
|
||||
|
||||
// Temporary kill-switch for per-video live report subscriptions. Real-env logs
|
||||
// showed these fan out O(videos × relays) and storm relays (~1000 REQ/s),
|
||||
// starving list decryption. Re-enable once batched (single REQ for all ids).
|
||||
const MODERATION_REPORT_SUBS_ENABLED = false;
|
||||
|
||||
export class ModerationService {
|
||||
/**
|
||||
* @param {object} options
|
||||
@@ -191,6 +197,16 @@ export class ModerationService {
|
||||
}
|
||||
|
||||
rebuildTrustedContacts(nextContacts = new Set(), { previous = null } = {}) {
|
||||
// Snapshot the trusted set BEFORE we rebuild so we can detect whether it
|
||||
// actually changed. Emitting "contacts" unconditionally caused a tight loop:
|
||||
// profileModalController listens for "contacts" -> populateFriendsList ->
|
||||
// ensureViewerContactsLoaded -> (empty contacts) -> fetchTrustedContacts ->
|
||||
// clearTrustedMuteTracking -> rebuildTrustedContacts(empty) -> emit again,
|
||||
// spinning the CPU and flooding relays with kind-3 REQs on every login.
|
||||
const priorTrusted =
|
||||
this.trustedContacts instanceof Set
|
||||
? new Set(this.trustedContacts)
|
||||
: new Set();
|
||||
const sanitized = new Set();
|
||||
|
||||
if (nextContacts instanceof Set || Array.isArray(nextContacts)) {
|
||||
@@ -222,8 +238,24 @@ export class ModerationService {
|
||||
this.trustedContacts = seededNext;
|
||||
|
||||
this.updateTrustedSeedOnlyStatus();
|
||||
this.emit("contacts", { size: seededNext.size });
|
||||
this.recomputeAllSummaries();
|
||||
|
||||
// Only notify/recompute when the trusted set actually changed. This breaks
|
||||
// the emit -> fetch -> emit loop above while still firing on real changes
|
||||
// (first population, follows/unfollows, seed updates).
|
||||
let changed = priorTrusted.size !== seededNext.size;
|
||||
if (!changed) {
|
||||
for (const value of seededNext) {
|
||||
if (!priorTrusted.has(value)) {
|
||||
changed = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (changed) {
|
||||
this.emit("contacts", { size: seededNext.size });
|
||||
this.recomputeAllSummaries();
|
||||
}
|
||||
this.reconcileTrustedMuteSubscriptions(previousContacts, seededNext);
|
||||
}
|
||||
|
||||
@@ -1564,6 +1596,14 @@ export class ModerationService {
|
||||
}
|
||||
|
||||
async setActiveEventIds(ids = []) {
|
||||
// CIRCUIT-BREAKER (confirmed against real env): one live kind-1984 report
|
||||
// subscription PER VIDEO, fanned across ~20 relays and re-issued on every
|
||||
// flaky-relay reconnect, produced ~1000 REQ/s and starved list decryption.
|
||||
// Stays disabled until replaced by a single batched subscription (one REQ
|
||||
// with "#e":[...all ids]) targeting only healthy relays. See refactor plan.
|
||||
if (!MODERATION_REPORT_SUBS_ENABLED) {
|
||||
return;
|
||||
}
|
||||
const nextIds = new Set();
|
||||
if (Array.isArray(ids) || ids instanceof Set) {
|
||||
for (const value of ids) {
|
||||
|
||||
@@ -1151,6 +1151,11 @@ export class VideoListView {
|
||||
}
|
||||
|
||||
ensureViewCountSubscription(pointerInfo) {
|
||||
// CIRCUIT-BREAKER (confirmed against real env): one live kind-30079 view
|
||||
// subscription per card, fanned across ~20 relays, contributed ~880 REQ/s.
|
||||
// Disabled until batched into a single count query. See refactor plan.
|
||||
return null;
|
||||
// eslint-disable-next-line no-unreachable
|
||||
if (!pointerInfo || !pointerInfo.key || !pointerInfo.pointer) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
// Regression test for the nip-07-login CPU-peg / relay REQ storm.
|
||||
//
|
||||
// Scenario (SCN-contacts-emit-on-change):
|
||||
// Given the UI re-fetches trusted contacts whenever the moderation service
|
||||
// emits "contacts" (profileModalController.populateFriendsList ->
|
||||
// ensureViewerContactsLoaded -> fetchTrustedContacts -> rebuildTrustedContacts),
|
||||
// When rebuildTrustedContacts is invoked with an UNCHANGED contact set,
|
||||
// Then "contacts" is NOT emitted — otherwise the emit -> fetch -> emit chain
|
||||
// spins the CPU and floods relays with kind-3 REQs (observed ~1600/s).
|
||||
// And when the set genuinely changes, exactly one "contacts" event fires.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
withMockedNostrTools,
|
||||
createModerationServiceHarness,
|
||||
} from "../helpers/moderation-test-helpers.mjs";
|
||||
|
||||
const A = "a".repeat(64);
|
||||
const B = "b".repeat(64);
|
||||
|
||||
test("rebuildTrustedContacts emits 'contacts' only when the set changes", (t) => {
|
||||
withMockedNostrTools(t);
|
||||
const { service, capture } = createModerationServiceHarness(t);
|
||||
const emits = capture("contacts");
|
||||
|
||||
// Establish a baseline population (a real change from the initial state).
|
||||
service.rebuildTrustedContacts(new Set([A, B]));
|
||||
const afterFirst = emits.length;
|
||||
assert.ok(afterFirst >= 1, "first real population should emit at least once");
|
||||
|
||||
// Re-applying the SAME set repeatedly must NOT emit. This is the exact loop
|
||||
// that pegged the CPU: identical empty/unchanged rebuilds re-emitting forever.
|
||||
service.rebuildTrustedContacts(new Set([A, B]));
|
||||
service.rebuildTrustedContacts(new Set([A, B]));
|
||||
service.rebuildTrustedContacts(new Set([A, B]));
|
||||
assert.equal(
|
||||
emits.length,
|
||||
afterFirst,
|
||||
"unchanged contact sets must not emit (loop guard)",
|
||||
);
|
||||
|
||||
// A genuine change emits exactly once more.
|
||||
service.rebuildTrustedContacts(new Set([A]));
|
||||
assert.equal(emits.length, afterFirst + 1, "a changed set emits once");
|
||||
|
||||
// ...and re-applying that changed set is again silent.
|
||||
service.rebuildTrustedContacts(new Set([A]));
|
||||
assert.equal(
|
||||
emits.length,
|
||||
afterFirst + 1,
|
||||
"repeating the changed set must not emit",
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,115 @@
|
||||
// Scenario tests for the NIP-07 queue's reserved foreground slot.
|
||||
//
|
||||
// The extension is a single serialized resource with only 2 concurrency slots.
|
||||
// Background-class work (LOW priority: DM backfill) must never occupy every
|
||||
// slot, or it starves the feed-driving lists (hashtags, subscriptions, watch
|
||||
// history) and moderation/block lists. The queue reserves one slot for
|
||||
// foreground (priority > LOW) by capping background work at maxConcurrent - 1.
|
||||
//
|
||||
// These tests drive the real shared queue through runNip07WithRetry so they
|
||||
// assert the externally observable scheduling behavior, not internals.
|
||||
|
||||
import { test, describe, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert";
|
||||
import {
|
||||
runNip07WithRetry,
|
||||
NIP07_PRIORITY,
|
||||
} from "../js/nostr/nip07Permissions.js";
|
||||
|
||||
global.window = { nostr: {} };
|
||||
|
||||
describe("NIP-07 fair scheduling (reserved foreground slot)", () => {
|
||||
const originalWindow = global.window;
|
||||
beforeEach(() => {
|
||||
global.window = { nostr: {} };
|
||||
});
|
||||
afterEach(() => {
|
||||
global.window = originalWindow;
|
||||
});
|
||||
|
||||
// SCN-nip07-background-capped:
|
||||
// Given a flood of LOW-priority (background) tasks,
|
||||
// When they are all enqueued at once,
|
||||
// Then no more than (maxConcurrent - 1) === 1 of them ever run concurrently,
|
||||
// leaving a slot free for foreground work.
|
||||
test("background (LOW) work never uses more than the unreserved slots", async () => {
|
||||
let active = 0;
|
||||
let maxActive = 0;
|
||||
const task = (delay) => async () => {
|
||||
active += 1;
|
||||
maxActive = Math.max(maxActive, active);
|
||||
await new Promise((r) => setTimeout(r, delay));
|
||||
active -= 1;
|
||||
};
|
||||
|
||||
const all = [];
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
all.push(
|
||||
runNip07WithRetry(task(60), { priority: NIP07_PRIORITY.LOW }),
|
||||
);
|
||||
}
|
||||
await Promise.all(all);
|
||||
|
||||
assert.equal(
|
||||
maxActive,
|
||||
1,
|
||||
"at most 1 background task should run at once (1 slot reserved for foreground)",
|
||||
);
|
||||
});
|
||||
|
||||
// SCN-nip07-foreground-not-starved:
|
||||
// Given background (LOW) work is saturating its allowed slot,
|
||||
// When a foreground (NORMAL) task arrives,
|
||||
// Then it starts promptly using the reserved slot — it does NOT wait for a
|
||||
// background task to finish.
|
||||
test("a foreground task is not starved by a background flood", async () => {
|
||||
const start = Date.now();
|
||||
const log = [];
|
||||
const bg = (delay) => async () => {
|
||||
await new Promise((r) => setTimeout(r, delay));
|
||||
};
|
||||
const fg = () => async () => {
|
||||
log.push({ id: "fg", time: Date.now() - start });
|
||||
};
|
||||
|
||||
// Flood the queue with long background tasks first.
|
||||
const bgPromises = [];
|
||||
for (let i = 0; i < 4; i += 1) {
|
||||
bgPromises.push(runNip07WithRetry(bg(200), { priority: NIP07_PRIORITY.LOW }));
|
||||
}
|
||||
// Now a foreground task arrives while background is running.
|
||||
const fgPromise = runNip07WithRetry(fg(), { priority: NIP07_PRIORITY.NORMAL });
|
||||
|
||||
await fgPromise;
|
||||
const fgStart = log.find((e) => e.id === "fg");
|
||||
assert.ok(fgStart, "foreground task should have run");
|
||||
assert.ok(
|
||||
fgStart.time < 150,
|
||||
`foreground should start in the reserved slot (<150ms), got ${fgStart.time}ms`,
|
||||
);
|
||||
|
||||
await Promise.all(bgPromises);
|
||||
});
|
||||
|
||||
// SCN-nip07-foreground-uses-all-slots:
|
||||
// Given only foreground (NORMAL) work,
|
||||
// When two tasks are enqueued,
|
||||
// Then both run concurrently — the reservation does not throttle foreground.
|
||||
test("foreground work can still use every slot", async () => {
|
||||
let active = 0;
|
||||
let maxActive = 0;
|
||||
const task = (delay) => async () => {
|
||||
active += 1;
|
||||
maxActive = Math.max(maxActive, active);
|
||||
await new Promise((r) => setTimeout(r, delay));
|
||||
active -= 1;
|
||||
};
|
||||
|
||||
await Promise.all([
|
||||
runNip07WithRetry(task(80), { priority: NIP07_PRIORITY.NORMAL }),
|
||||
runNip07WithRetry(task(80), { priority: NIP07_PRIORITY.NORMAL }),
|
||||
]);
|
||||
|
||||
assert.equal(maxActive, 2, "two foreground tasks should run concurrently");
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user