본문으로 건너뛰기

Code Review: 5a27732f — Reduce put_many lock count from 4N to 2N

커밋: 5a27732fbfec47b0a89d85383cea2356d4957b59 제목: [Perf][RawBlock] Reduce put_many lock count from 4N to 2N 작성자: Nayeon Kim nayeoni.kim@samsung.com 일자: 2026-05-29 리뷰 모드: /code-review --xhigh (9-angle finder × 1-vote verify × sweep, extra-high recall) 관련 변경 문서: private/change/L1-put_many-lock-coalesce.md

변경 요약​

RawBlockCore.put_many 가 키당 4회 잡던 self._lock 을 2회로 줄임. 핵심 방식은 _write_one 안에서 별도로 잡던 _inflight_io_count lock 두 개를 없애고, put_many 의 기존 allocate-lock / commit-lock 구간으로 흡수.

  • _write_one (lmcache/v1/storage_backend/raw_block/core.py:898–953): 내부 with self._lock: _inflight_io_count ±1 제거. 순수 I/O 함수가 됨. docstring Note: 블록으로 caller 책임 명시.
  • put_many (lmcache/v1/storage_backend/raw_block/core.py:434–528): allocate-lock 안에서 _inflight_io_count += 1. 이후 try: success = self._write_one(...) finally: with self._lock: ... 의 finally 가 _inflight_io_count -= 1, _last_io_ts = time.monotonic(), _inflight.pop(...), 그리고 commit/rollback 분기를 모두 수행.
  • 테스트: tests/v1/storage_backend/test_rust_raw_block_backend.py 끝에 _CountingLock 헬퍼 + 4개 테스트 추가 — 락 횟수 2N 계약, 예외 안전, inflight 가시성, checkpoint idle gate.

정합성 (Correctness) — 부분 통과​

핵심 의도 (4N → 2N) 는 측정 (_CountingLock 으로 N=4 시 acquire 8 회) 과 회귀 테스트 (raw_block 관련 9 passed, 26 skipped — extension 미설치) 로 검증됨. 그러나 _last_io_ts 갱신 시점이 바뀌면서 변경 전과 다르게 동작하는 경로가 있음 (F1, 아래).

Findings​

총 11건 (CONFIRMED 5, PLAUSIBLE 6). 분류 표:

#finding파일:라인verdictseverity권장 액션
F1_last_io_ts 가 payload-validation 실패에도 갱신 → checkpoint idle 회귀core.py:514CONFIRMEDhigh본 PR 내 수정 권고
F2테스트의 core._lock = counting SLF 위반test:2342CONFIRMEDmedium본 PR 내 정리 (테스트 한정)
F3inflight_io_count() docstring 이 실제 ON window 와 불일치core.py:399CONFIRMEDmedium본 PR 내 docstring 수정
F4_read_slot_header 가 구 패턴 유지 → 비대칭core.py:975PLAUSIBLElow별도 cleanup PR
F5_CountingLock.__enter__ 가 self._inner.__enter__() 반환test:2302CONFIRMEDlow본 PR 내 수정 (1줄)
F6zip(..., strict=False) — 길이 검증 직후인데 strict 안 씀core.py:470PLAUSIBLElow별도 cleanup PR
F7load_many_into 1/배치 vs put_many N/배치 — 카운터 의미 분기core.py:597PLAUSIBLEinfo별도 cleanup PR 또는 의도 시 docstring 명시
F8테스트 N=4 hardcode → fake device slot 한계와 implicit 결합test:2336PLAUSIBLElow별도 cleanup PR (헬퍼 보강)
F9_write_one Note 블록 docstring 스타일 비일관core.py:919PLAUSIBLEinfo본 PR 내 정리 가능 (선택)
F10release.set() 중복 호출 (idempotent)test:2428PLAUSIBLEinfo본 PR 내 정리 가능 (선택)
F11counter ON window 가 prep/lock-wait 포함 (F3 와 동일 root)core.py:506CONFIRMEDmediumF3 와 함께 처리

severity 기준: high = 회귀 (이전 동작과 다름), medium = 계약/표준 위반, low = 정리/일관성, info = 코멘트 수준.

즉시 조치 권고 (본 PR 내 보강)​

F1 — _last_io_ts 회귀 (high) [CONFIRMED]​

  • 현상: payload-validation 실패 (예: O_DIRECT 에서 payload 가 slot capacity 초과 → _prepare_write_payload 가 RuntimeError 던짐) 시, 변경 전에는 _last_io_ts 갱신 없음 → checkpoint idle gate 정상 동작. 변경 후에는 put_many finally 가 무조건 갱신 → meta_idle_quiet_ms 만큼 idle 판단이 추가로 지연됨.
  • 근거:
    • 변경 전 _write_one (HEAD~1): _last_io_ts 갱신은 inner try/finally (pwrite 직후) 안에서만 발생. _prepare_write_payload 가 던지면 outer except Exception: return False 로 빠져나가므로 갱신 안 됨.
    • 변경 후 core.py:511-514: try: success = self._write_one(...) 의 finally 가 allocate 가 성공한 모든 키 에 대해 무조건 _last_io_ts = time.monotonic() 호출.
  • 트리거: use_odirect=True 환경에서 caller 가 slot capacity 를 초과하는 payload 를 던졌을 때, 또는 _encode_header / O_DIRECT buffer alignment 단계에서 어떤 식으로든 raise 가 일어났을 때.
  • 결과: 정상적으로 buggy payload 만 reject 되고 끝나야 할 호출이 뒷따르는 checkpoint 시점을 지연시킴. crash 는 아니지만 §3.3 의 "비효과: checkpoint 빈도 (동일 또는 약간 보수적)" 주장은 이 워크로드에서 부분 false.
  • 제안 픽스 (택1):
    • (A) _write_one 의 반환을 tuple[bool, bool] (success, did_io) 로 확장하여 caller 가 actual pwrite 여부를 알 수 있게 함.
    • (B) finally 안에서 if success: self._last_io_ts = ... 로 조건부 갱신. 변경 전과 정확히 동일한 시맨틱 (pwrite 이 모두 끝났을 때만 갱신). → (B) 권장. 변경 표면 작음, semantic 가 정확히 변경 전 일치.
  • 테스트 보강: _prepare_write_payload 가 RuntimeError 를 던지는 케이스에서 _last_io_ts 가 사전 값 그대로 유지됨을 검증.

F3 + F11 — inflight_io_count() docstring 정정 (medium) [CONFIRMED]​

  • 현상: 변경 후 카운터 ON window 는 allocate-lock 안 += 1 → lock 해제 → _encode_header → _prepare_write_payload → pwrite × 2 → finally lock 대기 → finally lock 안 -= 1. docstring (core.py:399) 의 "currently active raw-device I/O operations" 보다 넓음.

  • 근거: 변경 문서 §2.2 의 시점별 표에서 ON window 가 넓어지는 점은 의도된 변경으로 명시되어 있으나, 그 결과로 docstring 자체가 stale.

  • 제안 픽스: docstring 을 다음과 유사하게 수정.

    Return the number of put/load operations whose inflight window
    (allocate -> device I/O -> commit) is currently open. Used by the
    checkpoint idle gate.

  • 이 PR 에 포함: docstring 한 줄 수정 — scope 작음.

F2 — 테스트 SLF 위반 정리 (medium) [CONFIRMED]​

  • 현상: tests/v1/storage_backend/test_rust_raw_block_backend.py:2342 의 core._lock = counting 직접 대입.
  • AGENTS.md §SLF: "all new code should follow SLF discipline regardless of location". CI 의 SLF gate 는 multiprocess/, distributed/ 한정이라 통과하지만 표준 위반.
  • 제안 픽스 (택1):
    • (A) RawBlockCore 에 test-only hook 추가 — production 코드에 test hook 추가는 비권장.
    • (B) 테스트가 _lock 직접 대입 시 코드 주석에 정당화 한 줄 추가. 이미 변경 문서 §4.4.1 에서 정당화되어 있음. → (B) 권장 — 본 PR 에서 테스트 코드 inline 주석 보강.

F5 — _CountingLock.__enter__ 반환값 정정 (low) [CONFIRMED]​

  • 현상: tests/v1/storage_backend/test_rust_raw_block_backend.py:2302 return self._inner.__enter__() → with as 바인딩이 inner Lock 결과로 새어나감.
  • 현 영향: core.py 어느 곳도 with self._lock as ...: 를 쓰지 않으므로 실제 영향 없음 (latent).
  • 제안 픽스: return self.
  • 이 PR 에 포함: 1 줄 수정, 위험 없음.

F9, F10 — 선택적 정리 (info)​

  • F9 core.py:919 _write_one 의 새 Note: 블록을 기존 docstring 의 Args/Returns/Raises 스타일과 일관되도록 다듬기 (선택).
  • F10 tests/...:2428 outer-finally release.set() 중복 제거 또는 "defensive" 주석 추가 (선택). Event.set() idempotent 라 기능 영향 없음.
  • 본 PR 분량을 고려해 선택적.

후속 PR 후보 (본 PR scope 외)​

F4 — _read_slot_header 통합 (low) [PLAUSIBLE]​

  • 현상: write 측 (_write_one) 은 caller 책임으로 바뀌었으나 read 측 _read_slot_header (core.py:975-988) 는 여전히 inline 2-lock 패턴 유지.
  • 영향: 실 동작 영향 없음. 향후 maintainer 가 _write_one 의 새 contract 를 보고 read 측도 같다고 가정 시 혼란.
  • 제안: 동일 패턴으로 통합하려면 _read_slot_header 호출자 (_load_checkpoint_from_device 등) 도 같이 손대야 함 → 별도 PR.

F6 — zip(..., strict=False) → strict=True (low) [PLAUSIBLE]​

  • 현상: core.py:470. 직전 line 462-465 에서 길이 검증을 이미 함 → strict=True 가 명백히 정합.
  • 제안: 동일 파일 내 다른 strict=False 사용처도 일괄 정리하는 별도 cleanup PR.

F7 — load/put 카운터 의미 분기 (info) [PLAUSIBLE]​

  • 현상: load_many_into 는 배치당 1회, put_many 는 키당 N회.
  • 제안: 의도를 명확히 한 뒤 (a) load 도 per-key 통일, (b) 카운터를 두 개로 분리, (c) docstring 에 명시 — 설계 결정 필요. 본 PR 외.

F8 — fake device 헬퍼 슬롯 capacity 노출 (low) [PLAUSIBLE]​

  • 현상: tests/...:2336 n = 4 hardcode 가 _make_raw_block_core 의 암묵적 6-slot ceiling 에 결합되어 있음.
  • 제안: _make_raw_block_core(slots=N) 처럼 capacity 자동 산정 옵션 추가 — 다른 테스트 fragility 도 같이 해소.

위험 / 호환성 평가​

  • on-disk format: 미변경.
  • public API: put_many 시그니처/리턴 미변경, _write_one 은 private.
  • 스레드 안전성: 카운터 ±1 모두 lock 안에서 처리, 여전히 thread-safe.
  • 롤백: 단일 commit revert 로 충분, 추가 마이그레이션 불필요.
  • 회귀 영향: F1 (payload-validation 실패 시 idle gate 지연) 한정. 픽스 적용 시 변경 전과 정확히 동일한 행동으로 복귀.

리뷰 메타​

  • 도구: /code-review --xhigh (9-angle finder × 1-vote verify × sweep).
  • 단계 요약:
    • Phase 0: git diff HEAD~1 로 diff 확보.
    • Phase 1: 9 angles 병렬 (A 라인스캔 / B 제거동작 / C 호출자 / D Python pitfall / E proxy + Reuse / Simplification / Efficiency / Altitude).
    • Phase 2: 1-vote verifier 가 후보를 CONFIRMED / PLAUSIBLE / REFUTED 로 판정. REFUTED 는 drop.
    • Phase 3: 동일 verifier 가 sweep 으로 추가 누락 후보 탐색.
  • 입력 commit: 5a27732f
  • verdict 분포: CONFIRMED 5 / PLAUSIBLE 6 / REFUTED 0 retained.
  • 본 리뷰는 read-only — 코드 변경 없음.

후속 조치 (2026-06-01)​

리뷰 권고 중 본 PR 내 보강 항목 4건을 적용. 회귀 테스트 1건 추가. 5 + 10 (raw_block 회귀 풀세트) 테스트 모두 PASS, ruff check/format 통과.

적용된 변경​

#파일:라인변경
F1lmcache/v1/storage_backend/raw_block/core.py:514-522put_many 의 finally 안 _last_io_ts = time.monotonic() 을 if success: 가드로 감쌈. prep-stage 실패 시 변경 전과 동일하게 idle gate 타임스탬프가 advance 되지 않음. 가드 옆에 의도 설명 코멘트 추가.
F3+F11lmcache/v1/storage_backend/raw_block/core.py:398-405inflight_io_count() docstring 을 변경 후 ON window (allocate -> device I/O -> commit) 와 일치하도록 갱신. checkpoint idle gate 와의 관계 명시.
F5tests/v1/storage_backend/test_rust_raw_block_backend.py:2300-2303_CountingLock.__enter__ 가 self 를 반환하도록 정정. inner Lock 의 __enter__() 결과를 새어나가게 하던 latent bug 해소.
신규 테스트tests/v1/storage_backend/test_rust_raw_block_backend.py::test_put_many_does_not_stamp_last_io_ts_on_prep_failure_prepare_write_payload 가 RuntimeError 를 던지는 케이스에서 _last_io_ts 가 사전 값 그대로 유지됨을 검증. F1 회귀 가드. monkeypatch 로 prep 실패를 직접 주입 — O_DIRECT capacity-overflow 의 가장 직접적인 재현.

보류 (의도적 미적용)​

#사유
F2_CountingLock docstring (tests/...:2287-2294) + inline 코멘트 (tests/...:2340-2341) 가 이미 SLF 정당화 충족. AGENTS.md §SLF 명시 한 줄 보강은 분량 대비 효과 낮음.
F9_write_one Note: 블록 스타일 — info 레벨, 본 PR scope 와 무관.
F10release.set() 중복 — Event.set() idempotent, 기능 영향 없음.
F4, F6, F7, F8별도 PR 후보 그대로 유지.

검증 명령​

source .venv/bin/activate
python3 -m pytest -v \
  tests/v1/storage_backend/test_rust_raw_block_backend.py \
  tests/v1/storage_backend/test_raw_block_key_codec.py
# => 10 passed, 26 skipped (extension 미설치 skip)
ruff check  lmcache/v1/storage_backend/raw_block/core.py \
            tests/v1/storage_backend/test_rust_raw_block_backend.py
ruff format --check \
            lmcache/v1/storage_backend/raw_block/core.py \
            tests/v1/storage_backend/test_rust_raw_block_backend.py
# => All checks passed! / 2 files already formatted

변경 문서 §3.3 의 주장 재확인​

F1 픽스 후, put_many 의 _last_io_ts 갱신 시점은 _write_one 이 True 를 반환한 경우 (= 두 pwrite 가 모두 완료된 경우) 로 한정 → 변경 전과 완전히 동일한 시맨틱. §3.3 의 "checkpoint 빈도 (동일 또는 약간 보수적)" 주장이 다시 모든 워크로드에서 성립.