docs(memory): 改正 Start 里那笔 Add 的并发注释,并补 Enabled=false 回归测试
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)⇒ 红在「到期行却被删了」。
This commit is contained in:
1 parent
44c25eb03b
commit
69210a8586
2 files changed
+75
-3
No files matched your search
@@ -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 预算,召回质量静默下降(行本身已被读侧闸挡住,不会泄漏)。
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in new issue
Block a user