From 69210a85864273ea7945e02c8b9f9141cc05f5e4 Mon Sep 17 00:00:00 2001 From: AskaEth Date: Tue, 13 Oct 2026 21:46:24 +0800 Subject: [PATCH] =?UTF-8?q?docs(memory):=20=E6=94=B9=E6=AD=A3=20Start=20?= =?UTF-8?q?=E9=87=8C=E9=82=A3=E7=AC=94=20Add=20=E7=9A=84=E5=B9=B6=E5=8F=91?= =?UTF-8?q?=E6=B3=A8=E9=87=8A=EF=BC=8C=E5=B9=B6=E8=A1=A5=20Enabled=3Dfalse?= =?UTF-8?q?=20=E5=9B=9E=E5=BD=92=E6=B5=8B=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit review r1(p2-1items)指出:`tidy_service.go` 新加的那段注释(以及 2ba0a40 的提交 信息)写着「计数在这把锁里加 …… `Stop` 也持这把锁,这样 Add 与 Stop 的 `wg.Wait` 不可能撞在一起」。**这句不成立**:`Stop` 在 `tidy_service.go:214` 就放锁,到 `:219` (另一个 goroutine 里)才 `wg.Wait`。锁只串行化 `running` 翻转与两笔 Add, **不覆盖 Wait**。reviewer 用交叉 Start/Stop 探针在 -race 下复现了 `panic: sync: WaitGroup is reused before previous Wait has returned`,并实测该 panic 在 M3 之前的基线(20051f0)上同样可复现 ⇒ 既有性质,本切片不改行为,只改说法: - 照真实理由改写注释:锁内加是为了让 `Stop` 的 `running` 判定与这两笔 Add **原子化**(杜绝「Stop 已看到 running=false 早退」与「Wait 漏掉一笔尚未 Add 的 goroutine」两个窗口); - 明确写出这把锁**不**覆盖 `Stop` 的 `wg.Wait`,WaitGroup 的复用纪律靠 「`Start`/`Stop` 在进程生命周期各只调一次」维持; - 并写明那个交叉探针的**当前调用点不可达**(`Start` 只在 `cmd/cyrene/main.go:97` 调、`Stop` 只在 `cmd/cyrene/wire_services.go:212` 调),免得后人看到「有窗口但 不可达」以为是漏改。 同时补上 reviewer 点出的真实缺口:`Enabled=false ⇒ 清扫不跑` 此前只有注释、没有 已提交的回归测试。新增 TestTidyServiceStart_整理关掉时清扫不跑,真跑 Start 后断言 「没有删除发生 + 没有审计 + running 保持 false」三件一起(不是只断言 cfg.Enabled 字段那种假牙)。两条断言各有牙:把 Enabled 闸改成恒 false ⇒ 红在 running 那一条; 在 Enabled 早退分支里偷偷起一个清扫(running 仍为 false)⇒ 红在「到期行却被删了」。 --- internal/runtime/memory/expire_loop_test.go | 60 +++++++++++++++++++++ internal/runtime/memory/tidy_service.go | 18 +++++-- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/internal/runtime/memory/expire_loop_test.go b/internal/runtime/memory/expire_loop_test.go index 53ed47c..f0267ed 100644 --- a/internal/runtime/memory/expire_loop_test.go +++ b/internal/runtime/memory/expire_loop_test.go @@ -116,6 +116,66 @@ func TestTidyServiceStart_清扫随服务起停(t *testing.T) { } } +// TestTidyServiceStart_整理关掉时清扫不跑 钉住那条**行为契约**(不是断言字段那种 +// 假牙):`MEMORY_TIDY_ENABLED=false` ⇒ 到期清扫**不跑**,过期行**一条都不删**。 +// +// 为什么值得一条专门的用例:清扫那行 `go s.expirySweepLoop(cctx)` 就排在 +// `if !s.cfg.Enabled { … return }` **之后**。将来谁把 Enabled 检查挪到后面、 +// 或让 Start 在关掉时也走完整条路,过期行就会在「整理被关掉」时被物理删除 —— +// 那是与「后台件随宿主启停」(`preinsert_gc` 随审查服务启停,design/23 §19.3-2) +// 相反的行为,而且**不会红任何别的用例**(读侧闸照样挡着,表面上一切正常)。 +// +// 断言是「没有删除发生 + 没有审计 + running 保持 false」三件一起 —— 只看 +// `cfg.Enabled` 字段是假牙:那证明不了 Start 真的按关掉处理。 +func TestTidyServiceStart_整理关掉时清扫不跑(t *testing.T) { + t.Parallel() + ctx := context.Background() + store := newTestStore(t) + now := tidyTestTime(t) + row := addItem(t, store, MemoryItem{ + PersonaID: "persona.a", Scope: ScopePersona, Kind: KindRelationship, + Content: "整理关掉时不该被删的到期行", ExpiresAt: now.Add(-time.Minute), + }) + + var ( + mu sync.Mutex + audits []string + ) + cfg := tidyTestCfg(0.80) + cfg.Enabled = false + svc := newTidyServiceForTest(t, store, cfg, nil, fixedTidyClock(now), + func(_, what, result string) { + mu.Lock() + defer mu.Unlock() + audits = append(audits, what+" "+result) + }) + // 假等待:万一清扫/心跳被起起来了,它会停在这里而不是空转(不影响断言)。 + svc.wait = func(ctx context.Context, _ time.Duration) bool { + <-ctx.Done() + return false + } + + svc.Start(ctx) + + // ① 确定性断言:Start 走的是「关掉 ⇒ 直接返回」那条路。 + if svc.running { + t.Fatal("Enabled=false 时 Start 应当直接返回,running 必须保持 false") + } + // ② 让「清扫其实被起起来了」有机会显形:清扫是启动即扫一轮,中间只有 + // 「取行 + 删行」一次 SQLite 往返(同一台机上实测在微秒级,远比这个窗口小); + // 对照组 TestTidyServiceStart_清扫随服务起停 用的就是同一个语料与同一个时钟, + // 它在毫秒级内把行删掉。给同一个数量级的窗口再断言「什么也没发生」。 + time.Sleep(200 * time.Millisecond) + if !memoryRowExists(t, store, row.ID) { + t.Fatal("整理被关掉了,到期行却被删了 —— 清扫跑在了 Enabled 闸之外") + } + mu.Lock() + defer mu.Unlock() + if len(audits) != 0 { + t.Fatalf("整理被关掉了,却写出了到期审计:%v", audits) + } +} + // TestSweepExpiredMemories_同步清掉向量 是切片 M1 的 reviewer 交上来的那条 finding // 的断言:过期行被物理删除时,它的**向量也必须一起清掉** —— 否则向量留在索引里 // 占掉 idx.Search 的 top-K 预算,召回质量静默下降(行本身已被读侧闸挡住,不会泄漏)。 diff --git a/internal/runtime/memory/tidy_service.go b/internal/runtime/memory/tidy_service.go index 4aa7947..cd31663 100644 --- a/internal/runtime/memory/tidy_service.go +++ b/internal/runtime/memory/tidy_service.go @@ -134,9 +134,21 @@ func (s *TidyService) Start(ctx context.Context) { s.cancel = cancel s.wg.Add(1) // 到期清扫(design/23 §19 的第 ② / ③ 步)与心跳**同生共死**:同一个 cctx、 - // 同一个 wg,因此 Stop 会一并取消并 join 它(§19.3-3 的「可停」)。计数在 - // 这把锁里加(与上面那一笔同处)是刻意的:`Stop` 也持这把锁,这样 Add 与 - // Stop 的 `wg.Wait` 不可能撞在一起。 + // 同一个 wg,因此 Stop 会一并取消并 join 它(§19.3-3 的「可停」)。 + // + // 🔴 为什么这笔 Add 也在锁内(与上面那一笔同处):让 `Stop` 的 `running` 判定 + // 与这两笔 Add **原子化** —— 于是不可能出现「Stop 已看到 `running=false` 早退」 + // 或「Wait 漏掉一笔尚未 Add 的 goroutine(那会当场泄漏一个清扫 goroutine)」 + // 这两种窗口。 + // + // ⚠️ 这把锁**不**覆盖 `Stop` 的 `wg.Wait`:`Stop` 在 `tidy_service.go:214` + // 就放锁,到 `:219`(另一个 goroutine 里)才 Wait。因此 WaitGroup 的复用纪律 + // (「上一轮 Wait 返回前不得再 Add」)**不是**由这把锁给的,而是靠 + // 「`Start` / `Stop` 在整个进程生命周期各只调用一次」维持:`Start` 只在 + // `cmd/cyrene/main.go:97` 调、`Stop` 只在 `cmd/cyrene/wire_services.go:212` 调。 + // 交叉调用(`Start` 与在飞的 `Stop` 并发)在 `-race` 下会 `panic: sync: + // WaitGroup is reused before previous Wait has returned` —— 实测该 panic 在 + // M3 之前的基线上同样可复现(既有性质,非本切片引入),只是当前调用点**不可达**。 s.wg.Add(1) s.mu.Unlock()