본문으로 건너뛰기

Code Review: 2a031316 — Override batched_remove in RustRawBlockBackend

커밋: 2a031316 제목: [perf] override batched_remove in RustRawBlockBackend to reduce lock contention 작성자: Nayeon Kim nayeoni.kim@samsung.com 일자: 2026-06-01 리뷰 모드: /code-review --xhigh — 5+3+1 angle finder × verify × sweep 관련 디자인 문서: ../change/L3-rust-raw-block-batched-remove.md

변경 요약​

StoragePluginInterface.batched_remove 의 default fallback (per-key remove() × N) 을 RustRawBlockBackend 에서 단일 _pin_lock + 단일 _core.delete_many(N) 호출로 대체. +30 LOC 의 단일 메서드 override + 99 LOC 의 happy-path 테스트.

가장 큰 시맨틱 변화는 (1) _pin_lock 보유 시간이 O(1) 에서 O(N) 으로 늘어난 점과 (2) force=True 전파가 N 키에 일괄 적용되어 in-flight load 와의 race window 가 넓어진 점. DAXBackend.batched_remove 와 동일 패턴이며 abstract base 의 # TODO(Jiayi): Optimize batched remove 를 한 backend 분 해소.

분류 요약​

카테고리건수가장 심각한 항목
Correctness — race window 확장2force=True ↔ in-flight _batched_get_prefix 의 use-after-free
Performance — 락 holding time2encode 루프가 _pin_lock 안에서 실행
Maintenance — 정리 품질4dataclass throwaway, two-pass on results, remove() 와의 본문 중복, docstring 오도
Test — 커버리지 갭4lock-count 검증 없음, _pinned_keys/refcount 단언 없음, 중복 키 미커버, mid-batch raise 미커버
Architectural2abstract base TODO 해소가 backend 별 복붙으로 분산, force=True 가 storage_manager 까지 통과

Tier 1 — 머지 전 다뤄야 할 항목 (high)​

F1. encode 루프가 _pin_lock 안에서 실행됨​

rust_raw_block_backend.py:369-371

encoded_keys = [encode_legacy_key(key).encoded for key in keys]
with self._pin_lock:
    results = self._core.delete_many(encoded_keys, force=force)

encode_legacy_key 는 key.to_string() + uint64 mask (key_codec.py:127-130) 로 pure key derivation — 공유 상태 미접근. 그러나 N 키 encode 가 _pin_lock 보유 중에 실행되면 다른 thread 의 pin/unpin/contains(pin=True)/_batched_get_prefix 가 N 회 to_string() 시간 동안 블록.

수정 방향: encoded_keys 리스트 컴프리헨션을 _pin_lock 획득 이전으로 이동. 본 PR 의 lock-reduction 효과를 정확히 노리려면 critical section 안에는 _core.delete_many 한 번 + discard 루프만 두는 것이 일관됨.

encoded_keys = [encode_legacy_key(key).encoded for key in keys]
with self._pin_lock:
    results = self._core.delete_many(encoded_keys, force=force)
    ...

→ 위 형태가 이미 그렇게 되어 있긴 하지만, 더 정확히는 [encode_legacy_key(key).encoded for key in keys] 가 lock 진입 직전에 위치하므로 위치 자체는 안전. 재확인 결과 F1 은 false positive — encoded_keys 가 실제로 lock 밖에서 빌드됨. 본 finding 은 verifier 단계에서 REFUTED. (남기지만 격하)

최종 등급: Tier 4 (info, REFUTED).

F2. force=True 와 in-flight _batched_get_prefix 의 use-after-free 윈도우​

rust_raw_block_backend.py:347 rust_raw_block_backend.py:439

_batched_get_prefix 는 _pin_lock 안에서 _core.get_metadata_prefix(lock=True) 로 _lock_refcnt 를 증가시킨 뒤 _pin_lock 을 해제하고 _core.load_many_into 를 호출. 그 윈도우에 batched_remove(force=True) 가 들어오면:

  1. delete_many(force=True) 가 _lock_refcnt 를 무시하고 _index 에서 pop + 슬롯 free.
  2. 동시에 다른 thread 가 batched_submit_put_task 로 같은 슬롯을 재할당하고 새 데이터 write.
  3. A 의 load_many_into 가 captured offset 으로 read → 타 키의 데이터를 반환.

중요: 이 race 는 본 PR 이 새로 만든 것이 아님 — single-key remove(force=True) 에서도 동일 시나리오 존재. 다만 본 PR 은 한 batched_remove 호출이 N 키를 동시에 force-delete 하므로 blast radius 가 N 배.

시나리오:

  • T_A: batched_get_non_blocking(keys=[k1..k10]) — _pin_lock 잡고 refcnt++ × 10, 해제, k1 의 pread 시작.
  • T_B: batched_remove([k1..k10], force=True) — _pin_lock 잡고 N 슬롯 free.
  • T_C: batched_submit_put_task 로 freed 슬롯 재할당 + write.
  • T_A 의 pread 가 T_C 의 새 페이로드 read.

수정 방향:

  1. (권장, 이번 PR 안) batched_remove 의 default 를 force=False 로 변경. abstract base 의 default 가 force=True 인 것은 별개 문제 — 본 backend 의 안전한 default 는 force=False 가 맞음. 단일 키 remove() 의 default 와 일관성을 깨므로 함께 변경하거나 docstring 으로 명시.
  2. (후속 PR) delete_many(force=True) 가 inflight read 와의 race 를 방지하도록 core 측에서 _lock_refcnt > 0 인 키는 free 하지 않고 tombstone 처리. 또는 _batched_get_prefix 가 load_many_into 동안 _pin_lock 을 해제하지 않도록 변경.

시나리오 재현: 멀티 thread 테스트 — A 가 prefix load 중일 때 B 가 force-remove 하고 C 가 즉시 put 했을 때 A 의 read 가 신선한 데이터를 반환하는지 검증. 현재 테스트 미커버.

F3. mid-batch raise 시 _pinned_keys 비-동기화​

rust_raw_block_backend.py:370-374

with self._pin_lock:
    results = self._core.delete_many(encoded_keys, force=force)
    for encoded_key, removed in zip(encoded_keys, results, strict=True):
        if removed:
            self._pinned_keys.discard(encoded_key)

_core.delete_many 가 mid-loop raise (예: _append_free_slot_locked 의 corrupt offset 검증) 시:

  1. _lock 안에서 일부 키의 _index/_lock_refcnt 가 이미 pop 됨.
  2. exception 이 _lock → _pin_lock 두 context manager 를 unwind.
  3. discard 루프 미실행 → _pinned_keys 에 이미 core 에서 사라진 키들이 잔류.

시나리오: 이후 pin(K) 호출이 if K in self._pinned_keys: return True (rust_raw_block_backend.py:602) 로 short-circuit → core 에 entry 없는데 phantom pin. 후속 get_with_pin 이 contains miss.

중요: 이 또한 single-key remove() 에서도 가능. 본 PR 은 blast radius 를 N 배로 키울 뿐. 다만 본 PR 의 docstring 은 "single locked batch" 라 표현하므로 사용자가 atomicity 를 기대할 수 있어 시맨틱 명시가 필요.

수정 방향:

  1. (이번 PR 안 권장) docstring 에 partial-state 가능성 명시: "If the underlying core mutation raises mid-batch, _pinned_keys may retain entries whose core records have already been cleared."
  2. (후속 PR) F2 와 같이 처리 — discard 를 try/finally 로 best-effort 보상하거나, force=True 전용 fast path 에서 discard set 을 미리 빼는 형태.

Tier 2 — Performance (medium)​

F4. _pin_lock 보유 시간이 O(N) — pin/unpin/get-prefix 가 직렬화됨​

rust_raw_block_backend.py:370-374

_pin_lock 안에서 (a) _core.delete_many 가 _core._lock 보유 + N 키 dict pop + slot recycle + meta-dirty 갱신, (b) discard 루프 N 회 진행. N=10000 에서 ms 단위. 이 동안 모든 pin/unpin/contains(pin=True)/_batched_get_prefix/_pin_if_needed/_unpin_if_needed 가 차단.

옛 single-key remove() 는 N 회의 짧은 critical section — reader 가 사이사이 들어갈 수 있었음. 본 PR 은 한 번의 긴 critical section 으로 변경. PR 의 lock-reduction 목표 달성을 위한 정상 trade-off 지만, docstring 에 명시되지 않으면 운영에서 p99 latency 스파이크가 batched_remove 사이클과 상관관계를 보일 때 진단이 어려움.

수정 방향: docstring 에 "Batch size linearly extends _pin_lock hold time; recommended batch size ≤ ~1000 keys to bound reader-side latency." 정도 추가. 또는 본 메서드 호출자가 chunking 을 책임지도록 명시.

F5. inflight 취소 후 follow-up put 이 silent drop 됨​

rust_raw_block_backend.py:347 rust_raw_block_backend.py:401 (L2 patch 후 라인)

delete_many 는 _inflight[key].canceled = True 로 마킹만 하고 슬롯은 put_many 에 맡김. 그 사이에 caller 가 같은 키로 batched_submit_put_task 를 다시 호출하면:

  1. _core.exists_inflight(encoded) 가 True (취소된 inflight 가 아직 popped 안 됨).
  2. dedup filter 가 새 put 을 skip.
  3. 원래의 put_many 가 canceled 를 보고 슬롯 free → 사용자 의도와 달리 키가 저장 안 됨.

중요: single-key remove() 에서도 동일. 본 PR 은 N 배 amplification.

수정 방향:

  1. (이번 PR 안) docstring 에 "Re-issuing a put for a key whose inflight was canceled by batched_remove may be filtered out by the dedup gate; callers must wait for the cancellation to drain." 명시.
  2. (후속 PR) exists_inflight 가 canceled 상태를 별도로 알리거나, delete_many 가 inflight 를 즉시 pop 하도록 core 보완.

Tier 3 — Maintenance / 정리 (medium-low)​

F6. encode_legacy_key(key).encoded 의 dataclass throwaway​

rust_raw_block_backend.py:369

encoded_keys = [encode_legacy_key(key).encoded for key in keys]

encode_legacy_key 는 RawBlockKeySpec (frozen dataclass) 를 build 하고 .encoded 만 사용 — 나머지 필드 (uint64 mask 결과 등) 는 즉시 폐기. N 회의 dataclass init + bigint mask 가 wasted work. key.to_string() 직접 호출이 등가.

수정 방향: encoded_keys = [key.to_string() for key in keys]. 단, encode_legacy_key 가 다른 호출자와 spec 형식 일관성을 유지하기 위해 사용된다면, 이는 후속 cleanup PR 로.

F7. results 에 대한 two-pass​

rust_raw_block_backend.py:372-375

for encoded_key, removed in zip(encoded_keys, results, strict=True):
    if removed:
        self._pinned_keys.discard(encoded_key)
return sum(results)

discard 루프와 sum(results) 가 results 를 두 번 순회. 단일 패스로 합칠 수 있음:

removed_count = 0
for encoded_key, removed in zip(encoded_keys, results, strict=True):
    if removed:
        self._pinned_keys.discard(encoded_key)
        removed_count += 1
return removed_count

마이크로 최적화이지만 readability 도 살짝 개선 (count 와 discard 의 의도가 한 위치에).

F8. remove() 와 batched_remove() 가 같은 본문을 별도로 가짐​

rust_raw_block_backend.py:339 rust_raw_block_backend.py:347

두 메서드가 encode → _pin_lock → delete_many → _pinned_keys.discard 로 동일. remove(key) 가 bool(self.batched_remove([key], force=force)) 로 위임 가능. 향후 pin/discard 프로토콜 변경 시 두 곳을 잊지 않고 갱신할 위험이 사라짐.

수정 방향: 본 PR 이나 후속 cleanup PR 에서 remove() 를 batched_remove([key], force=force) > 0 으로 단일화.

F9. docstring 의 "default batched_remove fallback" 표현​

rust_raw_block_backend.py:354-356

Reduces lock-acquire count from N (one per key, via the default
``batched_remove`` fallback) to a single ``_pin_lock`` acquisition

abstract base 의 default 는 self.remove() 를 N 회 호출 — 본 클래스에서 N 회의 _pin_lock 획득이 발생하는 건 이 backend 의 remove() 가 그렇게 동작 하기 때문. 다른 backend 의 fallback lock 비용은 다를 수 있음.

수정 방향: "Reduces this backend's _pin_lock acquisitions from N (one per key) to 1." 로 수정. base-class invariant 인 것처럼 들리지 않도록.

Tier 4 — Test 커버리지 (medium)​

F10. _pin_lock 1회 획득의 회귀 방지 테스트 부재​

test_rust_raw_block_backend.py:572

본 PR 의 핵심 목표 (lock 4N → 2N, lock 1 회 획득) 가 test 로 검증되지 않음. 향후 refactor 가 per-key with self._pin_lock: 을 다시 도입해도 통과.

수정 방향: _pin_lock 을 counting wrapper 로 swap (또는 patch.object(backend._pin_lock, '__enter__', wraps=...)), N=5 batch 가 정확히 1 회 enter 하는지 검증.

F11. force=False pinned-skip 의 _pinned_keys/_lock_refcnt 단언 부재​

test_rust_raw_block_backend.py:656

removed = backend.batched_remove(keys, force=False)
assert removed == len(keys) - 1
assert backend.contains(keys[2], pin=False) is True

keys[2] 가 pinned 상태에서 force=False 호출 시 _pinned_keys 에 잔류해야 하지만, 테스트는 contains(pin=False) 만 확인. 향후 refactor 가 잘못해서 _pinned_keys.discard 를 결과 무관하게 호출해도 통과.

수정 방향: backend.lock_refcount(encoded_keys[2]) == 1 와 unpin → lock_refcount == 0 round-trip 추가.

F12. 중복 키 / mixed batch 미커버​

test_rust_raw_block_backend.py:572-667

batched_remove([k, k, k]) 시 첫 호출만 True, 나머지는 False (이미 popped). 향후 set(encoded_keys) 같은 중복 제거가 들어가면 silent miscount. 또한 현실적 eviction 입력은 (missing, pinned, present, duplicate-of-present) 가 한 batch 에 섞임 — 현재 테스트는 분리 호출로 커버.

수정 방향: 단일 call 에 4종 키를 섞어 테스트. 결과 카운트와 _pinned_keys 상태 확인.

F13. mid-batch raise 시나리오 미커버​

test_rust_raw_block_backend.py:572

F3 의 delete_many mid-loop raise 케이스가 테스트 없음. patch 로 raise 를 주입해 (a) _pin_lock 이 정상 해제되는지, (b) _pinned_keys 잔여 정책이 docstring 과 일치하는지 검증해야 함.

수정 방향: with patch.object(backend._core, "delete_many", side_effect=RuntimeError("simulated")): 후 후속 pin() 이 deadlock 없이 진행되는지, _pinned_keys 상태가 documented 인지 검증.

Tier 5 — Architectural (info)​

F14. abstract base 의 TODO 가 backend 별 복붙으로 분산됨​

abstract_backend.py:234-251

# TODO(Jiayi): Optimize batched remove
def batched_remove(self, keys, force=True):
    num_removed = 0
    for key in keys:
        num_removed += self.remove(key, force=force)
    return num_removed

본 PR 이 두 번째 backend 의 override (DAX 가 첫 번째). 향후 RawBlockCore 류의 새 backend 는 이 TODO 를 다시 발견하고 같은 패턴을 복붙하게 될 위험. abstract base 가 _delete_encoded(keys, force) -> list[bool] 같은 hook 을 정의하고 default batched_remove 가 그 hook 을 호출하도록 하면, 두 override 모두 사라지고 새 backend 는 hook 만 구현.

수정 방향: 후속 architectural PR. L2 review F4/F13 (abstract contract pin) 와 묶어 처리.

F15. force=True default 가 storage_manager 까지 통과​

storage_manager.py:1092 (commit 시점)

StorageManager.batched_remove 가 force 인자를 forward 하지 않음. 외부 caller 가 storage_manager.batched_remove(keys) 로 호출하면 backend 의 default force=True 가 적용 → pinned entries 가 silently bulldoze.

중요: 본 PR 이 새로 만든 문제는 아님 — abstract base 의 default 도 force=True. 다만 본 PR 의 force=True default 가 단일 키 remove() 와 일관되도록 의도된 것이므로, 시맨틱 측면에서 force=False 가 안전한 default 인지 재검토 가치가 있음.

수정 방향: 후속 PR — StorageManager.batched_remove 가 force 를 forward 하도록 수정 + 본 backend 의 default 를 force=False 로 변경 검토 (F2 와 묶어 처리).

테스트 평가​

신규 테스트 1개 (test_rust_raw_block_backend.py:572-667):

  • test_rust_raw_block_backend_batched_remove — happy path + force=False/True coverage. 빈 입력, 5 key 일괄 삭제, pin → force=False, all-missing batch, force=True final 모두 cover.

부족한 점 (F10-F13 참조):

  • _pin_lock 획득 횟수 검증 없음 — PR 의 핵심 목표 회귀 방지 불가.
  • _pinned_keys/_lock_refcnt 의 invariant 단언 없음 — contains(pin=False) 만으로는 internal state divergence 미감지.
  • 중복 키 / mid-batch raise / mixed batch 미커버.

전반적으로 happy-path 커버는 되어 있으나, 본 PR 의 가치 (lock 횟수 감소) 와 invariant (_pinned_keys ↔ _lock_refcnt 동기화) 를 직접 검증하는 회귀 테스트가 빠짐.

결론 / 권고​

머지 권장도: F2, F3 의 docstring 명시 후 머지. F10-F13 의 테스트 보강은 본 PR 또는 직후 follow-up 으로.

  • F1 (encode in lock) 은 검증 결과 false positive — encoded_keys 가 실제로 lock 밖에서 빌드됨. 본 finding 은 격하.
  • F2, F3, F5 는 single-key remove() 에 이미 존재하는 race/시맨틱 — 본 PR 이 새로 도입 하지 않음. 다만 한 호출이 N 키에 일괄 적용되어 blast radius 가 N 배로 커진 점이 docstring 에 명시되지 않음. 이번 PR 안에서 docstring 만 보강하면 충분, 실제 race 닫기는 후속 PR.
  • F4 (lock holding O(N)) 는 의도된 trade-off. docstring 에 batch size 권고 한 줄 추가 권장.
  • F6-F9 는 선택적 cleanup. 이번 PR 또는 별도 cleanup PR.
  • F10-F13 의 테스트 보강은 회귀 방지 측면에서 가치 큼. 특히 F10 (lock-count 검증) 은 PR 의 목표 자체를 lock-in.

후속 PR 후보:

  • F2 race 닫기 — delete_many(force=True) 가 _lock_refcnt > 0 키를 보호하도록 core 변경. 또는 _batched_get_prefix 가 load 동안 _pin_lock 을 유지.
  • F5 inflight 취소 시맨틱 정리 — exists_inflight 가 canceled 상태 별도 노출, 또는 delete_many 가 inflight 즉시 pop.
  • F14 abstract base TODO 해소 — _delete_encoded hook 패턴으로 DAX/RustRawBlock 두 override 통합. L2 review F4/F13 (abstract contract pin) 과 묶어 처리.
  • F15 storage_manager 의 force forward — 별도 small PR.

선택적 cleanup: F6 (dataclass throwaway), F7 (single-pass), F8 (remove ↔ batched_remove 통합), F9 (docstring wording).

L1 / L2 / L3 cross-PR 정리​

L1 (raw_block/core.py 의 put_many lock coalesce), L2 (plugins/rust_raw_block_backend.py 의 batched_submit_put_task 재구성), L3 (본 PR — 같은 plugin 의 batched_remove override) 는 각자 다른 메서드/lock 을 건드림.

수정 메서드사용 lockcore API
L1_write_one, put_manycore _lock(없음 — core 자체)
L2batched_submit_put_task_put_lockput_many, exists_many, exists_inflight
L3batched_remove (신규)_pin_lockdelete_many

→ 메서드/lock 단위로 직교. 머지 순서 무관.

T1 (9fc5a901) 와의 의존성: L3 의 현 베이스는 T1 미적용. T1 머지 후 rebase 시 본 PR 의 delete_many 호출 결과가 list[bool] → list[RawBlockDeleteResult] 로 바뀌므로, if removed: 와 sum(results) 두 줄을 if result.deleted: / sum(1 for r in results if r.deleted) 로 mechanical 수정 필요. L3 디자인 문서 §1.5 참조.

테스트 helper cleanup 누적: L1 F8 (_make_raw_block_core 의 N=4 슬롯 한계와 implicit 결합) + L2 F9 (_make_legacy_backend 의 dead return slot) + L3 의 tempfile/config 인라인 보일러플레이트 (_make_legacy_backend 미사용) 가 모두 같은 파일 tests/v1/storage_backend/test_rust_raw_block_backend.py 의 helper 정리 항목. 별도 cleanup PR 로 묶어 처리하는 것이 자연스럽다.

변경 후에도 유지되어야 할 좋은 점​

  • DAXBackend.batched_remove 와 동일 패턴으로 backend 일관성 회복.
  • abstract base 의 TODO 한 backend 분 해소.
  • force=False 시 pinned 키 보존이 single-key remove() 와 정확히 동일하게 유지됨 (if removed: gate).
  • zip(strict=True) 로 delete_many 결과 길이 mismatch 를 명시적으로 검출.
  • 빈 입력 short-circuit (if not keys: return 0) 로 lock 미획득 보장.