Repository navigation
fix(plugin): close dirty-closed rescan gaps: cursor starvation, cancelled-drain settlement, mid-drain re-emission - #2588
Conversation
…ncelled-drain settlement Issue MemTensor#2459 reported two reliability gaps in the dirty-closed rescan: Gap 1 — cursor-lap starvation. collectDirtyClosedEpisodes() paged only downward (listClosedPage({ before: cursor })), so an episode that became dirty after the cursor paged past its position was not revisited until the walk reached the end of the closed set and the cursor reset — a full lap of ceil(closed/500) x 10 min (~70 min on the reporter's DB, hours on the 30k+-episode DBs of MemTensor#1787). Each scan now takes one bounded newest-first page from the head (before: null) before continuing the cursor walk, deduplicated by episode id, so freshly closed episodes and freshly re-marked recent rows are seen on the very next tick while the cursor still drains the older backlog a page at a time. Gap 2 — interrupted drains lost phase-2 accounting for the whole batch. recoverDirtyClosedEpisodes() returned right after await handle.flush() when startupRecoveryCancelled was set, skipping settlement for every queued episode: successes kept their rewardDirty markers (re-collected and re-run later — duplicate LLM spend) and failures lost their failedAttempts backoff counters. Settlement is synchronous bookkeeping on rows whose chains already ran, so it now runs even when cancelled and even if the flush throws mid-teardown, best-effort with one try/catch per row (init.dirty_closed_episodes.settle_failed). Also raises init.orphan_scan.failed from debug to warn: a skipped startup classification used to be invisible at default log levels (the reporter only found it by diffing kv cursor writes). Base: dev-v2.0.36. Fixes MemTensor#2459 Signed-off-by: akushonkamen <akushonkamen@163.com>
…-drain Review finding on MemTensor#2459: the head-page pass from gap 1 re-reads the newest page on every 10-minute tick, and episodes whose drain is still in flight keep their rewardDirty markers until phase-2 settles — so a tick racing an open drain re-collected and re-emitted the whole batch, re-running the full reward chain per episode every tick (~5-6 duplicate runs per episode for the 300+-episode / 1h+-drain scenario the issue describes; neither the 30s scan dedup nor the capture/reward subscribers dedupe by episodeId). Guard: recoverDirtyClosedEpisodes records each emitted episode id in a closure-level in-flight set before emitting and clears it as soon as that row's settlement runs (success, backoff bump, or un-settleable); both auto collection sites (init classification + periodic rescan) filter in-flight ids out. Per-episode, not a whole-scan mutex, so freshly dirtied rows are still collected on the very same tick while a large drain is open. The set is process-local: after a hard kill the markers on disk still drive a normal rescan on reboot. Base: dev-v2.0.36. Refs MemTensor#2459 Signed-off-by: akushonkamen <akushonkamen@163.com>
Memtensor-AI
added
area:plugin
🤖 Open Code ReviewTarget: PR #2588 🔍 OpenCodeReview found 5 issue(s) in this PR. 1.
|
|
Restores the change proposed in #2482 for review against
mainafter its former base branch,dev-v2.0.36, was deleted. The source commits and authors are preserved.Source head:
c70019cf99b938f1b66726f03b6d7dafff7ecbe4This PR is open for review. Review and CI against the current base are pending; validation claims in the original description below refer to the earlier snapshot. The original discussion and evidence remain in #2482.
Original change description
Description
Fixes #2459 — two reliability gaps in the memos-local-plugin dirty-closed rescan, plus one follow-up guard found in review. All changes are in
apps/memos-local-plugin/core/pipeline/memory-core.tswith new unit tests; both commits are based ondev-v2.0.36(8511502).Root cause
collectDirtyClosedEpisodes()paged only downward vialistClosedPage({ before: cursor })and only reset the cursor on a short page, so an episode that became dirty after the cursor paged past its position was never revisited until the walk reached the end of the closed set and the cursor reset — a full lap ofceil(closed/500) × 10 min(~70 min on the reporter's DB, hours on the 30k+-episode DBs of v2.0.2+ regression: bootstrapMemoryCoreFull() hangs with 100% CPU on databases >500MB #1787).recoverDirtyClosedEpisodes()returned right afterawait handle.flush()whenstartupRecoveryCancelledwas set, skipping settlement for every queued episode: successes kept theirrewardDirtymarkers (re-collected and re-run later — duplicate LLM spend) and failures lost theirfailedAttemptsbackoff counters.Fix
before: null) merged into an id-deduplicatedcollectedmap, then continues the cursor walk (core/pipeline/memory-core.ts:1788-1831). Cursor advancement still derives from the walk page only (core/pipeline/memory-core.ts:1823-1831), so head-page rows do not disturb cursor semantics; freshly closed / freshly re-marked rows are now seen on the very next tick.try { flush } finallywith one try/catch per episode (core/pipeline/memory-core.ts:1700-1775) — cancel or a flush throw mid-teardown still settles each row (clearrewardDirtyon success, bumpfailedAttempts+lastFailureAton failure); a per-row error logsinit.dirty_closed_episodes.settle_failedand cannot drop the rest of the batch.recoverDirtyClosedEpisodesrecords each emitted episode id in a process-local in-flight set before emitting and clears it as soon as that row settles; both auto-collection sites (init classificationcore/pipeline/memory-core.ts:811-825and the 10-min periodic rescancore/pipeline/memory-core.ts:1215-1227) filter in-flight ids, so a tick racing an open drain can no longer re-collect and re-emit the whole batch (duplicate LLM spend). Freshly dirtied rows are not blocked — per-episode guard, not a whole-scan mutex.init.orphan_scan.failedraised from debug to warn (core/pipeline/memory-core.ts:1219-1226): a skipped startup classification used to be invisible at default log levels.AI assistance disclosure: this change was produced with AI assistance in an automated contributor workflow; every command and test result below was executed locally as recorded.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Environment: macOS (arm64), Node + vitest 2.1.9, run from
apps/memos-local-plugin.Reproduce:
Results (final verification at this branch head):
recoveryReasonundefined on the row dirtied above the cursor) and Gap 2 (rewardDirtymarkers unsettled after cancelled drain).npx tsc -p tsconfig.json --noEmit): exit 0.Note: the ticket's
vitest … -qflag does not exist in vitest 2.1.9 (CACError: Unknown option -q), so--reporter=dotwas used instead. CI is expected red on this scope for external PRs due to the known environment issue tracked in #2476 (autotest environment preparation fails); local green evidence is provided above.Checklist
Reviewer Checklist