리뷰가 잡은 것은 전부 계획이었습니다: 구현 실수는 한 건도 없었습니다

전이 9곳을 공통 장치로 옮기는 동안 계획이 여섯 번 틀렸고, 그중 하나는 제가 만든 검증 단계가 틀린 질문을 하고 있어서 통과했습니다

앞 편에서 저는 캐시 갱신을 각 결제 흐름이 알아서 챙기는 구조 때문에 같은 장애를 두 번 냈고, 세 번째를 막으려고 전이와 발행을 분리할 수 없게 묶은 공통 장치를 만들었습니다. 그리고 그 글 마지막에 이렇게 적었습니다.

공통 장치를 만드는 것과 그것을 쓰게 만드는 것은 다른 문제입니다.

그때 그 장치를 실제로 쓰는 흐름은 두 개뿐이었습니다. 나머지는 여전히 각자 상태를 바꾸고 있었고, 그중 아홉 곳은 단말이 화면을 갱신하려고 폴링하는 바로 그 구간 안에 있었습니다. 이번 작업이 그 아홉 곳을 옮기는 일이었습니다.

작업 방식은 이랬습니다. 계획을 9개 태스크로 쪼개고, 태스크마다 앞뒤 맥락이 없는 새 구현자에게 맡기고, 끝날 때마다 별도 리뷰어를 붙였습니다. 구현자는 자기 태스크만 보고, 리뷰어는 그 diff만 봅니다.

그렇게 아홉 번을 돌렸는데, 리뷰가 잡아낸 것 중 구현자가 잘못 짠 것은 하나도 없었습니다. 틀린 것은 계획이었습니다. 제가 쓴 문장이거나, 제가 의심 없이 물려받은 규칙이거나. 이 글은 그 여섯 건에 대한 기록입니다.

어려운 건 옮기는 일이 아니라 트랜잭션 경계였습니다

먼저 무엇을 옮기는지 짧게. 결제 원장의 상태를 바꾸는 코드가 아홉 군데 흩어져 있었고, 그것들을 전부 하나의 함수로 통과시키는 게 목표였습니다. 그 함수가 저장과 캐시 갱신 이벤트 발행을 같이 하니까, 통과시키기만 하면 까먹을 수가 없습니다.

여기서 걸리는 게 하나 있습니다. 캐시를 실제로 갱신하는 리스너가 이렇게 선언돼 있습니다.

@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void updateState(StateUpdatedEvent event) { ... }

커밋이 끝난 뒤에만 캐시를 씁니다. 롤백되면 아예 실행되지 않으니 정합성은 맞습니다. 그런데 이 애노테이션에는 함정이 있습니다. 이벤트를 발행하는 시점에 활성 트랜잭션이 없으면, 그 이벤트는 예외도 로그도 없이 버려집니다.

아홉 곳 중 네 곳이 활성 트랜잭션 밖이었습니다. 콜백이 롤백된 뒤의 복구 전이, 단말이 보낸 취소 요청 처리, 2단계 QR 흐름. 이 네 곳에서 그냥 공통 함수를 부르면 캐시가 조용히 안 갱신됩니다. 이 프로젝트가 없애려는 그 증상이 다른 경로로 돌아오는 겁니다.

그래서 전이와 저장만 짧은 트랜잭션으로 감싸는 실행기를 하나 만들기로 했습니다. 메서드 전체를 트랜잭션으로 감쌀 수는 없습니다. 그 흐름들은 결제사 HTTP 호출과 전이가 뒤섞여 있어서, 감싸면 DB 커넥션을 네트워크 대기 시간만큼 붙잡게 됩니다. 여기까지가 계획이었고, 첫 번째 결함은 그 실행기의 의사코드에 있었습니다.

1. 커밋을 롤백 블록 안에 두면 진짜 원인이 사라집니다

제가 계획에 적어 넣은 코드입니다.

try {
    Payment saved = transition(payment, state, stage);
    transactionManager.commit(status);   // ← try 안
    return saved;
} catch (RuntimeException e) {
    transactionManager.rollback(status);
    throw e;
}

구현자는 이대로 짰고, 테스트도 통과했습니다. 그리고 리뷰어가 이걸 잡았습니다. Spring의 AbstractPlatformTransactionManager.commit()은 커밋 자체가 실패하면 내부적으로 롤백을 수행하고 TransactionStatus를 완료 상태로 표시합니다. 그 뒤에 rollback(status)를 부르면 어떻게 될까요.

IllegalTransactionStateException: Transaction is already completed

원래의 진짜 원인은 이 예외로 대체되어 사라집니다. 중복키였는지 락 타임아웃이었는지 알 수 없게 됩니다.

그리고 이건 드문 경로가 아닙니다. Hibernate는 UPDATE flush를 커밋 직전으로 미루기 때문에, 원장 저장의 제약 위반은 대개 바로 이 커밋 시점에 표면화됩니다. 결제 원장이 저장에 실패하는 상황에서 원인을 못 찾는 코드를, 제가 계획에 적어 배포한 셈입니다.

Spring의 TransactionTemplate.execute()가 왜 커밋을 try 밖에 두는지, 이때 처음 이유를 알았습니다.

Payment saved;
try {
    saved = transition(payment, state, stage);
} catch (RuntimeException | Error e) {
    transactionManager.rollback(status);
    throw e;
}
transactionManager.commit(status);   // ← try 밖
return saved;

추가로 얻은 것이 하나 있습니다. 리뷰어가 회귀 테스트를 요구했는데, 그 테스트를 짤 때 판별력을 따져야 했습니다. "던져진 예외가 커밋이 던진 그 인스턴스인가"만 확인하면 고친 코드와 안 고친 코드 양쪽에서 통과합니다. 목이 진짜 IllegalTransactionStateException을 재현하지는 않으니까요. 실제 판별자는 rollback이 한 번도 안 불렸는가였습니다.

이 클래스는 이후 세 개 태스크가 전부 그 위에 얹히는 토대였습니다. 여기서 안 잡혔으면 네 곳이 그 위에 쌓였습니다.

2. 조건 밖에 있던 문장이 조건 안으로 들어갔습니다

콜백 처리 코드의 원본은 이랬습니다.

} else {
    payment.updateState(FAILED);              // ← step 조건 밖. 무조건 실행
    if (STEP1.equals(step))      payment.updateLatestStage(Q1FAL);
    else if (STEP2.equals(step)) payment.updateLatestStage(Q2FAL);
}

updateState(FAILED)는 조건 밖에 있고 updateLatestStage만 조건 안에 있습니다. 그래서 step이 둘 다 아니면 상태는 FAILED로 바뀌고 stage만 그대로 남습니다. 실무에서 그 경우는 step이 null일 때입니다.

제 계획은 이 코드를 네 갈래 표로 정리했습니다. 성공/실패 × 1단계/2단계. 그리고 구현자는 그 표대로 옮겼습니다.

표에 없던 다섯 번째 갈래가 사라졌습니다. step이 없으면 아무 전이도 일어나지 않게 됐습니다.

확률은 낮습니다. 그 enum은 상수가 둘뿐이라 null일 때만 닿습니다. 그런데 닿으면:

실패한 결제가 진행중으로 남습니다. 단말은 그 거래를 계속 폴링하다 타임아웃하고, 취소를 걸고, 그게 나중에 망취소로 처리됩니다.

앞 편 전체가 없애려던 그 장애 시나리오입니다. 다른 문으로 들어온 겁니다.

이건 개별 실수가 아니라 패턴이라고 판단해서, 계획의 전역 제약에 규칙 두 줄을 추가했습니다.

태스크의 표에 있는 갈래가 테스트 스켈레톤보다 많으면 표가 기준이다. 스켈레톤이 틀린 것이다.
else가 없는 if / else if를 옮길 때는 어느 갈래에도 걸리지 않는 경우의 종전 동작을 먼저 확인하라.

두 번째 규칙은 뒤에서 한 번 더 값을 했습니다. 2단계 QR 흐름에도 같은 구조가 있었는데, 그때는 구현자가 먼저 확인하고 넘어갔습니다.

3. 규칙이 감시할 수 없는 것을 감시하고 있었습니다

앞 편에서 만든 것 중에 아키텍처 테스트가 있습니다. 공통 장치 밖에서 상태를 바꾸면 빌드가 깨지고, 아직 못 옮긴 곳은 화이트리스트에 이름을 올려 예외로 둡니다. 그 목록이 비는 것이 다음 단계 착수 조건이었습니다.

망취소 태스크를 맡은 구현자가 막혔다고 보고했습니다. 편입을 마쳤는데 화이트리스트에서 그 메서드를 뺄 수가 없다는 겁니다. 빼면 다른 두 호출 때문에 테스트가 깨진다고요.

그 두 호출을 열어봤습니다.

public void systemCancellationSucceeded() {
    this.isSystemCancelled = SUCCESS;
    this.updateTime = Instant.now();
}

state도 latestStage도 바꾸지 않습니다. 그리고 캐시에 저장되는 레코드는 이렇습니다.

record TransactionState(String posId, String transactionNo,
                        String state, String stage, String transactionTime)

isSystemCancelled는 캐시에 없습니다. 즉 이 메서드들은 캐시를 어긋나게 만들 수가 없습니다. 규칙이 스스로 밝힌 목적은 "상태 전이는 공통 장치를 거쳐야 캐시 갱신 이벤트가 함께 발행된다"였습니다. 규칙은 그 목적과 무관한 대상을 감시하고 있었던 겁니다.

결정적인 증거는 목록 자체의 비일관성이었습니다. 같은 파일에 systemCancellationNoneTarget()이 있는데, 똑같이 isSystemCancelled만 세팅하면서 목록에는 없습니다. 원칙에 따라 고른 게 아니라 이름이 비슷해서 쓸어담은 흔적입니다.

그대로 뒀으면 어떻게 됐을까요. 이 두 메서드의 운영 호출부가 여덟 곳입니다. 화이트리스트 키가 메서드 단위라, 그 여덟 곳은 편입을 마쳐도 목록에서 뺄 수 없습니다. 목록이 영원히 비지 않고, 그 목록이 비는 것이 착수 조건이었습니다.

더 나쁜 건 따로 있습니다. 항목을 남겨두면 그 메서드 안에서 누가 나중에 다시 상태를 직접 바꿔도 규칙이 못 잡습니다. 방금 얻어낸 보호막이 통째로 가려집니다.

그래서 감시 목록에서 두 이름을 뺐습니다. 다만 규칙을 좁히는 건 보호막을 줄이는 일이라, 조건을 하나 걸었습니다. 일부러 위반을 넣어 실패하는지 확인하고 되돌릴 것. 구현자는 임시로 직접 호출 한 줄을 심었고, 테스트는 그 줄을 정확히 가리키며 실패했고, 되돌린 뒤 다시 초록이 됐습니다.

앞 편에서 배운 것이 여기서 규범이 됐습니다. 가드는 통과가 아니라 실패로 검증해야 합니다.

4. 게이트가 틀린 질문을 했습니다

여기가 이 글의 본론입니다. 이번 작업에는 계획이 명시적으로 선언한 동작 변경이 하나 있었습니다. 종료 시각 필드가 결제 수단마다 있기도 없기도 했습니다. 대량 갱신 경로는 채우는데 엔티티 경로는 안 채워서, 같은 종류의 종료인데 값이 갈렸습니다. 이참에 맞추기로 했습니다.

규칙은 이렇게 세웠습니다.

if (state == COMPLETED || state == FAILED)
    payment.updateFinishedAt();

근거로 삼은 건 원본 도메인 메서드들이었습니다. 1차 QR 승인 완료, 1차 QR 실패, 환불 완료 — 전부 종료 시각을 채웁니다. 그러니 COMPLETED나 FAILED면 채우는 게 맞다, 이렇게 정리했습니다.

그리고 이건 값을 새로 채우는 변경이니까, 그 값을 읽는 쪽이 있는지 확인해야 한다고 판단했습니다. 그래서 계획에 게이트를 하나 넣었습니다. 착수 전에 이걸 확인하고, 걸리면 멈추라고요.

finishedAt == null로 분기하는 코드가 있는지 전 모듈에서 확인하라. 있으면 BLOCKED로 보고하라.

구현자는 확인했습니다. 0건이었습니다. 리뷰어가 독립적으로 재검증했고, 제가 지정하지 않은 모듈까지 추가로 훑었습니다. 역시 0건이었습니다. 게이트를 통과했고, 변경이 들어갔습니다.

아홉 태스크가 다 끝나고 마지막 전체 리뷰에서 이게 나왔습니다. 이 도메인에는 종료가 아닌 FAILED가 있습니다.

// 현재 진행 중인 거래인 경우 — 배치로 별도 처리될 예정입니다
else if (isProgressing(payment)) {
    transition(payment, FAILED, ABORT);
}

주석이 스스로 말하고 있습니다. "배치로 별도 처리될 예정"입니다. 단말이 취소를 요청했고, 실제 취소는 나중에 배치가 수행합니다. 이건 종료가 아니라 요청 접수입니다.

배치가 나중에 거래를 실제로 끝낼 때 쓰는 메서드는 종료 시각을 건드리지 않습니다. 그래서 요청 시각이 종료 시각으로 영구히 고착됩니다. 그리고 그 값은 통합 거래내역 페이로드에 실려 다른 시스템으로 나갑니다.

이제 게이트를 다시 봅시다.

finishedAt == null로 분기하는 코드가 있는지 확인하라.

아무도 분기하지 않습니다. 그냥 값을 읽어서 페이로드에 싣습니다. 게이트는 정직하게 0건을 반환했고, 정직하게 통과했습니다.

제 게이트는 "값이 새로 생기는가"를 물었습니다. "생기는 값이 맞는가"는 묻지 않았습니다. 그리고 이번 사고는 후자였습니다.

고치는 것 자체는 간단했습니다. 키를 state 하나에서 (state, stage) 쌍으로 바꾸고 ABORT를 제외하면 됩니다. 어려운 건 그런 조합이 더 없는지 확인하는 일이었고, 편입 지점이 넘기는 조합을 전수로 뽑아 대조하게 했습니다.

한 가지 더. 규칙을 좁히면서 반대 방향 회귀가 생길 수 있었습니다. 너무 좁히면 앞 편에서 고친 원래 문제가 되살아납니다. 엔티티 경로만 종료 시각이 빠지던 것 말입니다. 재리뷰에게 그걸 명시적으로 확인시켰고, 나머지 여덟 개 조합은 여전히 채우는 것으로 확인됐습니다.

제가 "살아 있는 버그"라고 보고한 것도 틀렸습니다

여섯 번째는 성격이 다릅니다. 이건 제가 없는 버그를 있다고 보고한 것입니다.

간편결제사 태스크를 준비하면서 그 패키지에서 캐시를 쓰는 곳을 찾았습니다. 리스너 하나뿐이었고, 그 리스너가 듣는 이벤트 중 하나는 저장소 어디에서도 발행되지 않고 있었습니다. 승인 성공 경로에는 이벤트 발행이 없었습니다.

그래서 결론을 냈습니다. 승인이 성공해도 캐시가 갱신되지 않는다, 운영에 나가 있는 살아 있는 버그다. 계획 문서에 근거표까지 붙여 넣었고, 사용자에게도 그렇게 보고했습니다.

구현자가 착수해서 코드를 읽다가 이걸 잡았습니다. 승인 경로는 게이트웨이로 캐시를 갱신하고 있었습니다.

제 검색어가 문제였습니다. publishEvent와 캐시 저장소 이름만 훑었고, 게이트웨이 추상화를 통과하는 경로를 넣지 않았습니다. 그 한 겹 때문에 있는 코드를 못 봤습니다.

사실로 남은 건 이겁니다. 캐시 미갱신이 아니라 발행처가 승인과 실패로 갈라져 있고 둘 다 공통 장치 밖이라 아키텍처 테스트가 감시하지 못한다는 것. 발행처 없이 리스너만 남아 있던 이벤트가 그 취약성이 이미 한 번 발현된 흔적이었고요.

계획 문서의 그 블록을 정정문으로 바꿨습니다. 무엇이 틀렸고 왜 틀렸는지까지 남겼습니다. 이 문서는 폐쇄망으로 옮겨져 거기서 그대로 구현되기 때문에, 틀린 근거를 남기는 것이 코드 버그만큼 비쌉니다.

무엇이 남았나

아홉 곳을 전부 옮겼습니다. 커밋 30개, 테스트 274개, 아키텍처 규칙 18개가 여섯 모듈에서 돌고, 전체 빌드가 통과합니다. 감시 목록의 구멍 하나도 막았습니다. 조건부 전이 포트를 직접 호출하는 경로가 규칙 밖에 있었습니다.

리뷰가 잡은 것을 세어보면 이렇습니다.

#	무엇이 틀렸나	출처	누가 찾았나
1	커밋을 롤백 블록 안에 두라고 지시	내 의사코드	태스크 리뷰
2	전이표에 다섯째 갈래 누락	내 전이표	태스크 리뷰
3	감시할 수 없는 것이 감시 목록에	앞 편의 규칙, 검증 없이 물려받음	구현자가 막혀서
4	게이트가 틀린 질문을 함	내 검증 기준	전체 리뷰
5	없는 버그를 있다고 보고	내 조사	구현자가 코드 읽다가
6	착수 조건 문장이 두 가지를 뒤섞음	내 계획 문장	실행 결과를 세다가

구현자가 잘못 짠 것은 이 목록에 없습니다. 한 건이 리뷰에서 "선언되지 않은 동작 변경"으로 걸렸는데, 파고들어 보니 공통 장치의 설계 계약상 피할 수 없는 귀결이었습니다. 호출자가 발행을 선택할 수 없게 만든 이상, 파트너별로 억제하는 코드는 표현 자체가 불가능합니다.

여섯 건이 나온 시점이 흩어져 있는 것도 눈에 띕니다. 4번은 앞의 다섯 종류를 다 통과한 뒤 마지막에 나왔습니다. 태스크별 리뷰는 자기 diff만 보기 때문에 "규칙이 도메인과 안 맞는다"는 종류를 구조적으로 볼 수 없습니다.

남은 것도 있습니다. 화이트리스트에 스무 건이 남았습니다. 처음에는 이걸 미완으로 봤는데, 세어보니 아니었습니다. 이번 작업은 폴링 구간 안의 아홉 곳만 옮기는 부분 작업이었고, 남은 스무 건은 QR 발급·환불·정산처럼 그 구간 밖의 흐름입니다. 착수 조건 문장이 틀렸던 겁니다. "목록이 빌 것"이 아니라 "목록에 폴링 구간 안의 지점이 없을 것"이어야 했습니다. 위 표의 6번입니다.

다만 스무 건 중 넷은 아직 판단이 안 섰습니다. 망취소 배치는 단말이 타임아웃한 뒤에 도는 것이라 구간 밖으로 보이는데, 배치가 도는 시점과 아직 폴링 중인 구간이 겹칠 여지를 실제로 배제한 적이 없습니다. 다음 단계 착수 전 첫 항목으로 적어뒀습니다.

그리고 이번 작업이 처음으로 드러낸 약점이 하나 있습니다. 캐시 갱신 리스너가 비동기이고 스레드 풀이 2~4개입니다. 2단계 QR 흐름은 상태가 셋 다 진행중이고 stage만 다른데, 캐시 쪽 가드는 "종결을 진행중으로 덮지 않는다"만 막습니다. 진행중 → 진행중은 그냥 통과합니다. 커밋 순서는 맞는데 쓰기 순서가 뒤집힐 수 있습니다.

이번 작업이 만든 결함은 아닙니다. 이전에는 stage 전이가 캐시로 나가지 않았으니까요. 이번 작업이 그걸 실사용 경로로 끌어올렸습니다. 별도 항목을 만드는 대신, 근본 해법이 같은 기존 백로그 항목에 사례로 보강했습니다.

작업하면서 남은 생각 세 가지.

계획은 코드와 같은 강도로 리뷰받아야 합니다. 이번에 리뷰가 잡은 여섯 건이 전부 계획에서 나왔는데, 정작 계획 자체를 리뷰하는 단계는 프로세스에 없었습니다. 구현자에게 계획을 주고 "이대로 짜라"고 하면, 계획의 결함은 코드가 되어서야 발견됩니다. 그때는 이미 그 위에 다른 태스크가 얹혀 있습니다.

게이트를 세울 때는 "무엇을 통과시킬 것인가"가 아니라 "무엇을 못 잡는가"를 물어야 합니다. 제 게이트는 잘 작동했습니다. 정직하게 훑었고 정직하게 0건을 반환했습니다. 문제는 그 질문이 사고의 형태와 안 맞았다는 것이고, 그건 게이트를 만들 때만 알 수 있는 것이었습니다. 앞으로 "새 필드가 채워진다" 류의 예외를 선언할 때는 (a) 누가 읽는가와 (b) 채워지는 값이 의미상 맞는가를 둘 다 묻게 할 생각입니다. (a)만 물으면 이번 같은 구멍이 또 납니다.

신선한 눈은 맥락이 없어서 값진 게 아니라, 맥락이 없어서 계획을 안 믿기 때문에 값집니다. 구현자들은 제 브리프에 있는 문장을 여러 번 의심했습니다. "캐시가 아예 갱신되지 않는다"는 제 서술을 코드로 확인해 반박했고, 만족 불가능한 단언을 짚었고, 공용 클래스에 새 API를 추가하는 결정 앞에서는 혼자 정하지 않고 멈췄습니다. 계획을 그대로 믿는 구현자였으면 여섯 건 중 몇 건은 그대로 나갔을 겁니다.

"까먹지 말자"로는 세 번째도 까먹습니다 — 이 글의 1편. 공통 전이 장치를 만든 이야기
고친 걸 되돌려도 테스트는 초록이었다 — 아무것도 지키지 않는 테스트
리뷰가 컴파일러였습니다 — 빌드를 못 돌리는 환경에서 리뷰가 무엇을 대신했나
내가 확인했다고 말한 세 가지 — 확인했다는 말과 확인한 것 사이