본문으로 건너뛰기

Code Review: 93403c0f — Replace per-key fan-out with batched put_many

커밋: 93403c0f 제목: [Fix][RawBlock][Perf] Replace per-key fan-out with batched put_many 작성자: Nayeon Kim nayeoni.kim@samsung.com 일자: 2026-05-29 리뷰 모드: /code-review --xhigh — 9-angle finder × verify × sweep 관련 디자인 문서: ../change/L2-batched_submit_put_task-batching.md

변경 요약​

RustRawBlockBackend.batched_submit_put_task를 N개의 독립 코루틴 fan-out에서 단일 put_many(specs, objs) 호출로 치환. _submit_put_one을 제거하고 _submit_put_batch를 도입하여 dedup 필터링 → 일괄 ref_count_up → 단일 future dispatch → 단일 put_many 워커 디스패치 흐름으로 정리.

가장 큰 구조적 변화는 (1) accepted key 전체가 하나의 Future 를 공유하는 점과 (2) _put_lock 보호 영역이 _put_tasks 멤버십 검사 + bitmap 필터 + exists_inflight 검사를 모두 포함하도록 확장된 점. 옛 per-key 코드가 갖고 있던 "한 key 의 실패는 한 key 만 영향" 이라는 격리 특성이 일부 깨졌다.

분류 요약​

카테고리건수가장 심각한 항목
Correctness — 자원 누수/회계2dispatch raise 시 N개 ref_count + _put_tasks 잔류
Correctness — TOCTOU/시맨틱2dedup bitmap 이 _put_lock 밖에서 페치됨
Performance — 락 경쟁/낭비 작업3_put_lock 안에서 per-key exists_inflight 호출
Contract — 시맨틱 변화1shared Future 가 peer backends 와 불일치
Maintenance — 정리 품질5dead branch, 세 평행 리스트, error 메시지 등
Architectural3abstract contract 미정의, dedup 중복 구현, double-hop dispatch

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

F1. dispatch 실패 경로의 N-key 자원 누수​

rust_raw_block_backend.py:411

for obj in accepted_objs:
    obj.ref_count_up()

fut = asyncio.run_coroutine_threadsafe(
    self._submit_put_batch(
        accepted_keys, accepted_specs, accepted_objs, on_complete_callback
    ),
    loop,
)

asyncio.run_coroutine_threadsafe() 는 loop 가 closed/stopped 상태에서 raise. 411-412 의 ref_count_up 루프가 끝난 직후 414 의 dispatch 가 raise 하면 (1) accepted_objs N 개의 ref_count 가 +1 인 채로 남고, (2) _put_tasks 에 추가된 N 개 key 도 영구 잔류. _submit_put_batch 의 finally 가 호출 자체가 안 되므로 cleanup 경로 부재.

시나리오: backend close() 와 동시 put 의 race. 411 (ref_count_up 루프 완료) 와 414 (dispatch) 사이에 다른 thread 가 loop 를 stop 하는 경우, dispatch 가 raise 하면서 _put_tasks 와 N 개 obj 의 elevated ref_count 를 그대로 둠. close() 의 10s 폴링은 그 leak 된 entries 가 사라지지 않으므로 timeout 으로 진행. 옛 per-key 코드는 한 key 의 dispatch 만 영향을 받았으나, 이번 PR 은 한 batch 의 N 키가 동시에 leak.

수정 방향: 411-419 를 try/except 로 감싸 except 블록에서 ref_count_down + _put_tasks.discard 를 수행한 뒤 re-raise. ref_count_up 루프 자체와 dispatch 호출을 같은 보호 범위로 묶어 부분 진행 상태에 대한 보상 코드를 한 곳에 집중.

F2. _put_lock 밖의 exists_many 가 부르는 dedup TOCTOU​

rust_raw_block_backend.py:390

specs = [encode_legacy_key(key) for key in keys]
encoded_keys = [spec.encoded for spec in specs]
indexed_bitmap = self._core.exists_many(encoded_keys, lock=False)

accepted_keys: list[CacheEngineKey] = []
accepted_specs: list[RawBlockKeySpec] = []
accepted_objs: list[MemoryObj] = []
with self._put_lock:
    for i, key in enumerate(keys):
        if key in self._put_tasks:
            continue
        if indexed_bitmap[i]:
            continue
        if self._core.exists_inflight(encoded_keys[i]):
            continue

exists_many(..., lock=False) 가 _put_lock 바깥에서 평가되므로, bitmap 페치와 401-407 의 필터 루프 사이에 다른 thread 가 동일 key K 를 commit 하면 stale False 값이 K 를 accepted_keys 로 진입시킴.

시나리오: Thread A, B 가 동시에 K 를 put. 둘 다 exists_many 에서 K=False 를 관찰. A 가 _put_lock 을 먼저 잡고 K 를 accept → put_many → _index 에 등록. 이어서 B 가 _put_lock 진입 → indexed_bitmap[i]=False (stale), exists_inflight=False (A 가 inflight 단계를 이미 통과 완료) → K 를 accept. B 의 put_many 는 core.py:468 부근의 _index 재검사로 short-circuit 되어 results[i]=True 가 돌아오고 B 의 on_complete_callback(K) 이 발화한다. 콜백 자체는 "key 가 persist 됐다" 는 약한 계약을 충족하지만, 호출자가 "콜백 발화 = 이번 submission 이 persist 됐다" 로 해석하면 어긋남 — 실제로 디스크에 쓰여진 것은 A 의 데이터다.

수정 방향: exists_many 호출을 _put_lock 안으로 이동시키거나 lock=True 옵션을 사용해 bitmap 과 _put_lock 보호 검사가 동일 critical section 에서 보이도록 한다. Performance 측면에서는 F5 와 합쳐 별도 batched API (exists_inflight_many) 도입과 동시에 정리하는 것이 자연스럽다.

L1 과의 상호작용 (cross-PR note): 같은 시기에 머지 예정인 L1 (cr-5a27732f) 가 RawBlockCore.put_many 의 lock 획득 횟수를 4N → 2N 으로 줄이지만, A 의 _index 등록과 inflight 해제는 여전히 같은 commit-lock 안에서 함께 일어나므로 본 race 의 윈도우는 L1 머지 전후 모두 동일하게 존재한다. L1 이 본 race 를 fix 하지도 deepen 하지도 않음 — 본 PR 안에서 독립적으로 처리해야 함을 명시.

디자인 문서 6.1 위험표와의 차이 (재검토): 디자인 문서 6.1 위험표는 race 항목을 "exists_many ↔ exists_inflight 간 race / 영향: 중복 put 시도 / 완화: core.put_many idempotency" 로 마감했으나, 본 finding 이 가리키는 race 는 그것과 다른 종류 — exists_many(lock 밖) ↔ _put_lock-안 검사(다른 thread 의 _index commit 이후) 간의 더 넓은 윈도우다. 데이터 자체는 idempotency 로 안전하지만, B 의 callback 발화 시점이 "이번 submission 이 persist 됐다" 로 해석될 수 있는 시맨틱 측면은 디자인 문서가 다루지 않은 영역. 디자인 문서 6.1 의 race 평가는 이 측면을 추가하여 사후 갱신이 필요하다.

Tier 2 — Correctness/Performance (medium)​

F3. ref_count_up 루프 자체가 보호되지 않음​

rust_raw_block_backend.py:411

for obj in accepted_objs:
    obj.ref_count_up()

대부분의 MemoryObj 구현에서 ref_count_up() 은 raise 하지 않지만, 디버깅 hook / assertion / 향후 backend 별 구현 변경 등으로 mid-loop 에서 raise 할 가능성은 열려 있다. j 번째에서 raise 하면 0..j-1 만 +1, j..N-1 미증가, N 개 key 모두 _put_tasks 에 잔류한 채 예외가 cleanup 없이 escape.

옛 per-key 코드는 단일 key 의 코루틴 안에서 ref_count 를 처리했으므로 단일 key 만 영향. 이번 PR 은 batch 전체로 blast radius 확장.

수정 방향: F1 과 동일한 try/except 보호 범위 안에 ref_count_up 루프를 포함시키고, 부분 증가분을 0..j-1 까지만 되돌리도록 인덱스를 추적한다. 또는 ref_count_up 의 계약상 raise 가 없음을 docstring 에 명시하고 정적으로 비-throwing 임을 보장.

F5. _put_lock 안의 per-key exists_inflight 호출​

rust_raw_block_backend.py:401

with self._put_lock:
    for i, key in enumerate(keys):
        if key in self._put_tasks:
            continue
        if indexed_bitmap[i]:
            continue
        if self._core.exists_inflight(encoded_keys[i]):
            continue

exists_inflight() 가 _put_lock 을 보유한 채 N 회 호출되며 호출마다 core 내부 _lock 을 잡는다. L2 디자인 문서의 "per-key core lock 빈도 감소" 목표와 정면으로 어긋나며, 한 batched_submit_put_task 가 _put_lock 을 보유하는 동안 다른 동시 호출자가 직렬화된다.

시나리오: N=100 batch 두 개가 동시에 들어오면, 한쪽이 _put_lock 을 보유하는 ~100 × (core._lock acquire + dict lookup) μs 동안 다른 쪽은 _put_lock 자체에서 대기. 옛 코드는 inflight 검사를 _put_lock 밖에서 수행했기 때문에 이런 직렬화가 없었다.

수정 방향: 별도 PR 로 exists_inflight_many 배치 API 를 신설해 _put_lock 안에서 한 번의 core lock 획득으로 N 키를 검사하거나, 검사 자체를 _put_lock 밖에서 사전 필터로 수행하고 안에서 한 번 더 cheap 한 dict 검사만 하는 2-단계 구조로 분리.

추기 (디자인 문서 가정 오류): L2 디자인 문서 2.5 절은 exists_inflight 를 "Python 측 dict membership 검사 1회 → 매우 저렴" 으로 평가하고 옵션 (a) exists_inflight_many 도입을 보류했다. 그러나 실제 코드 경로는 호출마다 core 내부 _lock 을 획득하므로 디자인 문서 평가보다 비용이 크다. F5 를 후속 PR 로 미루는 결정 자체는 합리적이지만, 디자인 문서 2.5 의 비용 근거는 사후 갱신이 필요하다.

F6. dead branch + 부분 실패의 ambiguous contract​

rust_raw_block_backend.py:436

results = put_result.results
if not results or not any(results):
    raise RuntimeError(
        f"Failed to persist raw-block batch of {len(keys)} keys "
        f"(first encoded: {specs[0].encoded})"
    )

not results 분기는 dead. accepted_keys 가 비었으면 408 의 early-return 으로 이미 빠져나갔고, put_many 는 len(results) == len(specs) 를 보장한다. 또한 docstring 은 "every accepted key 가 fail 했을 때" raise 라 명시되어 있어 부분 실패는 silent 처리된다. 옛 _submit_put_one 은 per-key 단위로 raise/log 가 분리되었으나 이번 PR 은 contract 가 좁아져, 부분 실패 시 future 는 success 로 종료되고 caller 는 실패 key 에 대해 알 길이 로그뿐.

수정 방향: 조건을 if not any(results): 로 단순화하고, partial failure 의 정의와 "future 는 모든 key 실패 시에만 fail" 라는 결정을 docstring 에 명확히 박는다. caller 가 부분 실패를 인지해야 한다면 on_complete_callback 의 보완 channel (예: on_failure) 도입을 별도로 검토.

F7. dedup 이전의 불필요한 encode 작업​

rust_raw_block_backend.py:388

specs = [encode_legacy_key(key) for key in keys]
encoded_keys = [spec.encoded for spec in specs]
indexed_bitmap = self._core.exists_many(encoded_keys, lock=False)

모든 input key 에 대해 dedup 수행 전에 encode_legacy_key 가 실행된다. 90% 의 key 가 이미 indexed 거나 inflight 인 batch 에서 90 개의 encode 작업이 폐기됨. 옛 per-key 코드는 dedup 이후에만 encode 를 수행했다.

수정 방향: cheap 한 key in self._put_tasks 검사를 먼저 통과한 key 에 한해 encode 를 수행. _put_lock 밖의 dedup 사전 필터는 race 가능하므로, 락 안에서 final decision 을 내리되 encode 는 lazy 하게 수행하는 형태로 재배치.

Tier 3 — Contract 시맨틱 변화 (medium)​

F4. shared-Future 의 시맨틱이 peer backends 와 불일치​

rust_raw_block_backend.py:420

fut = asyncio.run_coroutine_threadsafe(
    self._submit_put_batch(
        accepted_keys, accepted_specs, accepted_objs, on_complete_callback
    ),
    loop,
)
return [fut] * len(accepted_keys)

반환 리스트의 N 개 슬롯이 동일한 Future 객체. peer backends (gds, local_disk, nixl) 는 per-key distinct Future 를 반환하므로 시맨틱이 불일치한다. 호출자가 futures[2].cancel() 로 한 key 만 드롭하려 하면 Future 객체가 동일하므로 batch 전체가 취소된다.

abstract_backend.py:72-101 의 StoragePluginInterface.batched_submit_put_task 가 이 contract 를 명시하지 않아 backend 마다 선택이 달라진 결과. 현재 storage_manager.batched_put 가 반환값을 무시하고 있어 latent 한 상태로 남아 있다.

수정 방향: 본 PR 안에서는 "옵션 A — shared Future 를 의도한 결정" 임을 docstring 에 명시하고, abstract interface 에 contract 를 못 박는 작업은 후속 PR 로 분리. SharedBatchFuture wrapper 도입으로 cancel/result 시맨틱을 명시적으로 표현하는 방향도 검토 가치 있음.

추기 (재검토): docstring 명시 부분은 이미 충족됨. rust_raw_block_backend.py:369-377 의 Returns 절이 "A list of length equal to the number of accepted keys, where every entry is the same Future representing the batched submission." 로 옵션 A 시맨틱을 이미 박아 두었다. F4 의 본 PR 안 잔여 작업은 사실상 없으며, 남는 본질은 abstract interface 와의 contract 불일치 (= F13). 두 항목을 묶어 후속 PR 로 처리하는 것이 정합적.

Tier 4 — 정리/품질 (low)​

F8. partial-failure 테스트의 시맨틱 혼동​

test_rust_raw_block_backend.py:2519

def patched_put_many(specs, objs_):
    real = original_put_many(specs, objs_)
    # Force the middle entry to look like a failure.
    real.results[1] = False
    return real

patched_put_many 가 real put_many 를 호출한 뒤 results[1] = False 로 변조하므로 실제 데이터는 디스크에 persist 된다. 주석 "Force the middle entry to look like a failure" 은 데이터가 진짜로 실패한 것 같은 인상을 주지만 실은 callback 분기만 False 로 보이도록 만든 것. 회귀 시 "왜 실패한 key 가 디스크에 있지?" 라는 디버깅 misdirection 이 발생할 수 있다.

수정 방향: fully-mocked put_many 로 교체하여 디스크에 쓰지 않게 만들거나, 주석을 "mark as failure for callback dispatch testing only; payload is still persisted by the real call" 처럼 의도를 명확히 한다.

F9. _make_legacy_backend 의 dead return slot​

test_rust_raw_block_backend.py:2274

def _make_legacy_backend(
    monkeypatch,
    *,
    memory_allocator,
    loop_in_thread,
    instance_id: str,
    dev_path: str,
) -> "tuple[RustRawBlockBackend, LocalCPUBackend]":

(backend, local_cpu) 튜플을 반환하지만 모든 호출자가 backend, _ = _make_legacy_backend(...) 로 두 번째 값을 무시한다.

수정 방향: 두 번째 슬롯을 제거하고 RustRawBlockBackend 단일 객체를 반환. 어떤 호출자에서도 local_cpu 가 필요하면 그 caller 에서 별도로 만들면 된다.

F10. 세 개의 평행 리스트가 lockstep 으로 append​

rust_raw_block_backend.py:392

accepted_keys: list[CacheEngineKey] = []
accepted_specs: list[RawBlockKeySpec] = []
accepted_objs: list[MemoryObj] = []
with self._put_lock:
    for i, key in enumerate(keys):
        ...
        accepted_keys.append(key)
        accepted_specs.append(specs[i])
        accepted_objs.append(objs[i])

세 리스트가 lockstep 으로 append. 향후 refactor 가 한두 리스트에만 append 를 추가하면 인덱스가 어긋나며, 이런 종류의 버그는 unit test 가 잡기 어렵다.

수정 방향: 단일 리스트 accepted: list[tuple[CacheEngineKey, RawBlockKeySpec, MemoryObj]] = [] 로 합치거나 작은 dataclass 를 도입.

F11. 에러 메시지 입도​

rust_raw_block_backend.py:437

raise RuntimeError(
    f"Failed to persist raw-block batch of {len(keys)} keys "
    f"(first encoded: {specs[0].encoded})"
)

first encoded: {specs[0].encoded} 로 첫 key 만 메시지에 포함. 옛 per-key 코드는 실제 실패 key 를 적시했으므로, all-fail 케이스에서 첫 key 가 root cause 와 무관하면 운영자가 잘못된 key 를 조사하게 된다.

수정 방향: 모든 encoded key 를 메시지에 포함하되 길면 N 개 + ellipsis 로 truncate 하거나, count + first/last 만 표기.

F12. callback None + all-success 경로의 redundant iteration​

rust_raw_block_backend.py:441

for key, ok in zip(keys, results, strict=True):
    if not ok:
        logger.warning("RustRawBlockBackend: put failed for key %s", key)
        continue
    if on_complete_callback is not None:
        try:
            on_complete_callback(key)
        except Exception as e:
            logger.warning(
                "on_complete_callback failed for key %s: %s", key, e
            )

callback 이 None 이고 results 가 모두 True 인 common case 에서도 N-iteration zip 이 실행됨. 옛 코드는 이런 work 가 없었다.

수정 방향: 루프 진입 직전에 if on_complete_callback is None and all(results): return 로 hot-path 단축.

Tier 5 — Architectural (info)​

F13. abstract interface 의 미정의 contract​

abstract_backend.py:72-101

StoragePluginInterface.batched_submit_put_task() 의 시그니처가 Union[List[Future], None] 만 선언할 뿐 per-key vs shared Future 시맨틱을 명시하지 않는다. RustRawBlockBackend 는 shared Future, peer backends 는 per-key Future. 호출자가 두 backend 를 동일하게 취급하기 어렵다.

수정 방향: 별도 PR 로 abstract interface 에 contract 를 못 박거나, SharedBatchFuture 같은 wrapper 타입을 도입해 시맨틱을 타입 시스템으로 표현. F4 와 같이 다뤄지면 좋다.

F14. backend 별 dedup loop 중복 구현​

모든 backend (rust_raw_block, local_disk, gds, pd_backend_async, nixl) 가 자체적으로 _put_tasks 멤버십 필터링을 구현. 본 PR 의 L2 batched dedup loop 는 또 한 카피를 batched lock window 아래에 인라인했다.

수정 방향: 베이스 클래스 mixin 이 dedup loop 를 소유하고, subclass 는 persistence hook 만 구현하는 구조로 정리. 별도 architectural PR 후보.

F15. double-hop dispatch​

rust_raw_block_backend.py:414

fut = asyncio.run_coroutine_threadsafe(
    self._submit_put_batch(
        accepted_keys, accepted_specs, accepted_objs, on_complete_callback
    ),
    loop,
)

batched_submit_put_task 가 run_coroutine_threadsafe 로 asyncio loop 에 dispatch 하고, 그 loop 가 다시 to_thread 로 worker thread 에 dispatch. 그러나 storage_manager 는 이미 worker thread context 에서 호출 중이라, 직접 put_many 를 호출하면 두 hop 모두 생략 가능. batch 당 ~50-100μs latency 가 dispatch overhead 로 소비된다.

수정 방향: 별도 architectural PR 에서 dispatch 구조 자체를 재검토. 본 PR 의 범위는 아님.

테스트 평가​

신규 테스트 4개 (test_rust_raw_block_backend.py:2273-2625):

  1. test_batched_submit_put_returns_same_future_per_accepted_key — 옵션 A (shared Future) 시맨틱을 직접 검증. 좋은 테스트.
  2. test_batched_submit_put_filters_already_indexed_and_inflight — dedup 회귀 방지에 유효. 단, F2 의 race window 는 sequential 시나리오만 다루므로 잡지 못한다. 멀티 thread concurrent put 테스트가 추가되어야 race 회귀를 검출할 수 있다.
  3. test_batched_submit_put_partial_failure_logs_and_keeps_others — partial fail callback 분기를 검증하지만 F8 의 시맨틱 혼동 (real put 후 결과만 변조) 이 함께 존재.
  4. test_batched_submit_put_balances_ref_counts_on_exception — _submit_put_batch 의 finally 가 ref_count_down + _put_tasks.discard 를 수행하는지 검증. 다만 F1 (run_coroutine_threadsafe raise 경로) 와 F3 (411 ref_count_up 루프 raise 경로) 는 미커버 — 두 경로 모두 _submit_put_batch 가 시작되기 전에 발생하므로 현재 테스트로는 검출되지 않는다.

결론 / 권고​

머지 권장도: F1, F2 처리 후 머지.

  • F1 은 backend close() race 등 dispatch 실패 경로에서 N 개 key 가 영구 leak — 단일 key 였던 옛 코드 대비 blast radius 가 N 배. 본 PR 안에서 411-419 를 try/except 로 묶어 부분 진행 상태를 보상하는 코드를 추가하면 해결 가능.
  • F2 는 silent dedup miss 로 호출자의 callback 해석에 영향. _put_lock 안으로 exists_many 를 옮기거나 lock=True 로 바꾸면 본 PR 안에서 닫을 수 있다.

한 묶음 처리 권고: F1 + F3 + F2 는 모두 batched_submit_put_task 의 동일 함수 안 인접 라인 (388-419) 에 위치한다. F1 try/except 도입 시 F3 의 ref_count_up 루프도 같은 보호 범위에 자연히 포함되며, F2 의 exists_many 호출 위치 변경 또는 lock=True 전환은 같은 함수 안 다른 라인이라 한 PR 로 묶는 것이 효율적.

후속 PR 후보: F4 + F13 (abstract contract pin — F4 의 docstring 부분은 이미 충족, 잔여 작업이 F13 과 동일), F5 (exists_inflight_many 배치 API + 디자인 문서 2.5 근거 갱신), F14 (architectural cleanup), F15 (dispatch rethink).

선택적 cleanup: F6 (dead branch + docstring 명확화), F7 (lazy encode), F8-F12 (테스트 시맨틱/리스트 정리/메시지 입도/hot-path 단축).

L1 / L2 cross-PR 정리​

L1 (cr-5a27732f) 와 L2 가 다루는 코드 경로는 서로 다르므로 (L1: raw_block/core.py, L2: plugins/rust_raw_block_backend.py) finding 이 직접 겹치지는 않는다. 다만 두 PR 의 review 가 같이 비춰봐야 하는 약한 연결고리가 셋 있다.

  1. 테스트 helper cleanup 누적 — L1 F8 (_make_raw_block_core 가 N=4 슬롯 한계와 implicit 결합) 와 L2 F9 (_make_legacy_backend 의 dead return slot) 는 동일 파일 tests/v1/storage_backend/test_rust_raw_block_backend.py 의 helper 정리 항목. 별도 cleanup PR 로 묶어 처리하는 것이 자연스럽다.
  2. 공개 인터페이스 contract 명세 갭 — L1 F3 (inflight_io_count() docstring 이 실제 ON window 와 불일치) 와 L2 F4 (shared-Future 시맨틱이 abstract contract 에 미명시) 는 같은 패턴 — "공개 함수 시그니처가 갖는 계약이 코드와 다른 곳에 있거나 비어있음". 두 항목을 묶어 contract documentation 보강 PR 로 처리하는 안을 검토.
  3. L2 F2 race 의 L1 의존성 없음 — F2 의 dedup TOCTOU 는 L1 의 lock 윈도우 변화에 영향받지 않는다. 즉 L1 이 머지되어도 본 race 는 그대로 남으며, L1 이 race 를 생성하지도 않는다. 본 PR 에서 독립적으로 closing 해야 함.

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

  • 단일 put_many 호출로 core-lock 획득 횟수가 N → 1 로 줄어든 점은 디자인 문서 목표에 부합.
  • _submit_put_batch 의 finally 가 ref_count_down 과 _put_tasks.discard 를 모두 포함하도록 정리된 점은 이전 per-key 코드 대비 cleanup 일관성이 높다.
  • shared Future 결정 자체는 (시맨틱만 contract 화하면) 호출자에 batched 호출을 명시적으로 표현할 수 있어 합리적.