feat: Redis distributed lock for high concurrency#661
Closed
jlin53882 wants to merge 13 commits intoCortexReach:masterfrom
Closed
feat: Redis distributed lock for high concurrency#661jlin53882 wants to merge 13 commits intoCortexReach:masterfrom
jlin53882 wants to merge 13 commits intoCortexReach:masterfrom
Conversation
Tests verify: 1. Current implementation: lock acquired once per entry (not per batch) 2. Two-phase approach: lock acquired once per batch (N -> 1) 3. Concurrent writes: read-modify-write has data consistency boundaries 4. Plugin vs Upgrader: updates different fields, no direct overwriting Ref: Issue CortexReach#632
Two-Phase Processing: - Phase 1: LLM enrichment (no lock) - Phase 2: Single lock per batch for DB writes Reduces lock contention from N locks (one per entry) to 1 lock per batch. Tests: - Lock count verification: 10 entries = 1 lock (was 10) - LLM failure graceful degradation - Batch boundary handling - 100 entries stress test
- Added REFACTORING NOTE at class level explaining why upgradeEntry was split - Documented the key difference: OLD = lock per entry, NEW = 1 lock per batch - Added comments in prepareEntry explaining it contains the SAME logic as old upgradeEntry - Added comments in writeEnrichedBatch explaining the single lock approach - Added detailed comments in upgrade() showing before/after example - Added inline comments in the batch loop explaining why Phase 1 doesn't hold lock
The old test was designed to verify the BUGGY behavior (1 lock per entry). This update changes it to verify the FIXED behavior (1 lock per batch). This aligns with Issue CortexReach#632 fix - the test now confirms: - 3 entries = 1 lock (was 3 locks before)
…fixes) [FIX F2] 移除 writeEnrichedBatch 的 outer runWithFileLock - store.update() 內部已有 runWithFileLock，巢狀會造成 deadlock - proper-lockfile 的 O_EXCL 不支援遞迴 lock [FIX MR2] 每個 entry 寫入前重新讀取最新狀態 - Phase 1 讀取的 entry 是 snapshot - plugin 在 enrichment window 寫入的資料會被 shallow merge 覆蓋 - 改用 getById() 重新讀取最新資料再 merge
- F1: 修正硬編碼 /opt/homebrew/ 路徑，改用動態路徑 - F3: 修復 EnrichedEntry.error 未設置問題（LLM fallback 時設置 error） - F5: 新增 yield-every-5-writes 防止 plugin 長期飢餓 - 測試檔同步更新 F1 動態路徑
- Test 2: 實際呼叫 upgrader.upgrade() 觀察 lock 次數 - Test 3: 實際測試 Plugin + Upgrader 並發寫入 - Test 5: 實際驗證不同欄位互不覆蓋
- Return no-op lock instead of throwing Error when Redis is down - Log Redis errors instead of silent swallow - Fix duplicate ioredis entry in package.json
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
jlin53882
force-pushed
the
feat/redis-distributed-lock
branch
from
2a64448 to
e22cd64
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a Redis-based distributed lock to solve the high concurrency lock contention problem identified in Issue #643.
Problem
When captureAssistant=true and sessionMemory.enabled=true:
Solution
Add src/redis-lock.ts with:
Test Results
15x improvement!
Tests Added
Required
\
npm install ioredis
\\
Related