Skip to content

🐯 [并发] acquireCredit 在 StartFlush 和 Acquire 间无竞争保护 · memtable.go #127

@github-actions

Description

@github-actions

来自 #125 的架构级评审建议。不阻塞合入,仅供参考是否有更好的架构解法。

⚠️ [重要 · 并发] acquireCredit 在 StartFlush 和 Acquire 间无竞争保护 storage/zstorage/memtable.go:181

问题根因acquireCredit 调用流程是:TryAcquire 失败 → StartFlushAcquire(可能阻塞)。问题在于 StartFlush 只是尝试向 FlushChan 投递信号——如果 FlushWorker 已经在刷盘、但刷的是旧 dirty 且尚未 Release,那么 StartFlushFlushWorker 不会重入,而刚被 StartFlush 唤醒的 FlushWorker 刷的可能是旧数据(dirty 非 nil 时重用),Acquire 的等待者可能迟迟得不到释放。这种情况发生的精确场景:

  1. TryAcquire(50) 失败(池满)
  2. StartFlush() 被调用,但 FlushChan 已有信号(default 分支直接返回)
  3. Acquire(50) 进入等待
  4. FlushWorker 当前正在执行刷盘,但刷的是上一轮的 dirty,Release 的数据量可能不足以让这次 Acquire 通过 fits 检查
  5. 没有新的 Flush 被触发,acquireCredit 永久阻塞

为什么低级解法不够:通用评审者可能会建议在 acquireCredit 中加 for 循环重试 TryAcquire+Flush,或把 FlushChan 换成 channel close/reset 模式。但这些仍然只是打补丁——根因是背压信号和刷盘触发之间的耦合过松,没有「需要多少信用、什么时候释放」的明确契约。

架构级方案方案 A(短期):让 acquireCredit 告知 FlushWorker 需求——把信用需求传递下去,确保 FlushWorker 知道该释放多少。可以改为:

  • acquireCredit(n) 的等待需求记录到一个共享的 needBytes 字段(atomic),FlushWorker 在 Release 后检查是否满足当前最大等待者的需求,不满足则立即触发下一次 Flush。

方案 B(更优):改为信用借贷模式——acquireCredit 不是被动等待,而是参与 on-demand flush 的调度:当信用不足时,acquireCredit 直接参与刷盘(而不是仅触发),由它驱动 Flush 直到信用足够。但这和当前双表设计(Flush 是后台协程)有冲突。

当前 PR 的 acquireCredit 在 TryAcquire 失败后调了 StartFlushAcquire——如果 Flush 已有 inflight 但释放量不够,这条路确实会死锁。建议在 Flush 完成后多一步:判断是否仍有阻塞的 Acquire 等待者,若有则重入触发 Flush,避免一次 Flush 释放不够的问题。

代价/收益:方案 A 代价:需要少量额外状态跟踪(needBytes atomic + 条件判断),实现复杂度低。方案 B 更彻底但改动了 Flush 协程模型,不适合本 PR 增量引入。建议选方案 A 作为防御性改进。


💡 [建议 · 并发] Put/Delete 错误路径可能 double-release 信用 storage/zstorage/memtable.go:178

问题根因:在 Put(和类似的 Delete)中,如果 m.active 为 nil/空,当前代码 m.credits.Release(full) 归还信用——这是正确的。但如果 m.active.insert(key, value) 返回的 delta负数(覆盖写时新 value 比旧 value 短),则 full - delta > full,会 Release 多于 full,从而导致 used 降为负数——虽然 Release 内部有 p.used = 0 的钳位,但多归还会导致信用池膨胀(释放的信用多于占用的),背压的上限约束被稀释。

为什么低级解法不够:通用评审者可能会指出「insert 返回负数 delta 时信用对账不对」,但不会追问这个负 delta 场景是否真的会出现——以及如果出现,信用池的 used 钳位到 0 是否就安全了。

架构级方案:这个问题本质是信用占用和实际内存变动的解耦——写入前按 full 预占,写入后按 delta 归还。当 delta 为负数时(覆盖写缩短 value),归还的逻辑仍然正确(多还了预占但未占用的部分),但 Release 的钳位 p.used = 0 是安全的——因为它最多退回已经释放的状态。

更关键的改进是:覆盖写场景下,byteSize 维护是否完全正确? 当前覆盖写只调整了 byteSize += delta(新旧 value 的差值),但没有考虑 key 的长度变化。如果 key 被覆盖且长度不变(keys compare equal),这个没问题。但如果 insert 逻辑改了 key 的比较规则,这里就有 bug。当前实现是安全的,但建议加一个断言(invariant check)来验证 byteSize >= 0 且与预占信用对账。

代价/收益:当前行为是正确的(钳位到 0 防负),只是多归还少量信用在覆盖写缩短场景下不会破坏正确性。建议加一个 sl.byteSize >= 0 的检查作为防御性断言,代价极低。

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions